Ticket #88 (accepted defect)

Opened 4 years ago

Last modified 23 months ago

Crash during cancellation of transfer when device disconnected

Reported by: www.google.com/accounts/o8/id?id=aitoawmz93igogrpknbehv21jdxrf37ft8ksncg Owned by: hjelmn
Milestone: libusb/libusbx 1.2.0 Component: libusb-1.0 Darwin backend
Keywords: Cc:
Blocked By: Blocks:

Description

Reproduced this on OSX. Haven't tried yet on Linux.

I set up and submit a control transfer, but in the middle I disconnect the device.

It crashes (with signal EXC_BAD_ACCESS) on line 1307 (in cancel_control_transfer of darwin_usb.c):

kresult = (*(dpriv->device))->USBDeviceAbortPipeZero (dpriv->device);

I think it's a race condition where dpriv->device is made NULL by whatever code detects that the device has disconnected. A simple patch for this particular code path is:

if (!dpriv->device)
  return LIBUSB_ERROR_NO_DEVICE;

kresult = (*(dpriv->device))->USBDeviceAbortPipeZero (dpriv->device);

There may be a more generic code path available (& more code paths where this needs to be done).

Change History

comment:1 Changed 4 years ago by www.google.com/accounts/o8/id?id=aitoawmz93igogrpknbehv21jdxrf37ft8ksncg

  • Summary changed from [PATCH] Crash cancelling transfer of disconnected device to [PATCH] Crash during cancellation of transfer when device disconnected

comment:2 Changed 4 years ago by stuge

  • Keywords darwin added
  • Summary changed from [PATCH] Crash during cancellation of transfer when device disconnected to Crash during cancellation of transfer when device disconnected

The suggested check of dpriv->device before dereferencing it does not completely eliminate the race condition, although it makes the race window much smaller.

Hopefully Nathan can have a look at this and find a good solution. (Does dpriv need a lock for ->device?)

comment:3 Changed 4 years ago by hjelmn

  • Owner set to hjelmn
  • Status changed from new to accepted

I will look into this problem and see if there is a way to completely eliminate the race condition.

comment:4 Changed 4 years ago by www.google.com/accounts/o8/id?id=aitoawkqduuilezm1tu1seo2uz1ndphzkn82lqc

Check out the patch set for ticket #82. It might fix this problem.

comment:5 Changed 4 years ago by hjelmn

Adding a check on dpriv->device is a good idea. Approved for 1.0.9.

comment:6 Changed 3 years ago by Vitali Lovich <vlovich@…>

In [0c5bf03eb829e51dcf19562fc4f745937235ea51/libusb]:

Darwin: Reduce race likelihood between cancellation and device disconnect

References #88. The race condition still remains, but this change
makes it less likely to trigger.

comment:7 follow-up: Changed 3 years ago by stuge

  • Component changed from libusb-1.0 to libusb-1.0 Darwin backend
  • Keywords darwin removed

comment:8 in reply to: ↑ 7 Changed 23 months ago by hjelmn

  • Milestone set to 1.0.16
Note: See TracTickets for help on using tickets.