Messages in this thread | | | Subject | Re: [PATCH 0/6] usbip fixes to crashes found by syzbot | From | Shuah Khan <> | Date | Thu, 18 Mar 2021 07:13:37 -0600 |
| |
On 3/17/21 9:06 AM, Shuah Khan wrote: > On 3/17/21 12:21 AM, Tetsuo Handa wrote: >> Shuah, this driver is getting more and more cryptic and buggy. >> Please explain the strategy for serialization before you write patches. >> >>> - Fix attach_store() to check usbip_event_happened() before >>> waking up threads. >> >> No, this helps nothing. >> >>> diff --git a/drivers/usb/usbip/vhci_sysfs.c >>> b/drivers/usb/usbip/vhci_sysfs.c >>> index c4b4256e5dad3..f0a770adebd97 100644 >>> --- a/drivers/usb/usbip/vhci_sysfs.c >>> +++ b/drivers/usb/usbip/vhci_sysfs.c >>> @@ -418,6 +418,15 @@ static ssize_t attach_store(struct device *dev, >>> struct device_attribute *attr, >>> spin_unlock_irqrestore(&vhci->lock, flags); >>> /* end the lock */ >>> + if (usbip_event_happened(&vdev->ud)) { >>> + /* >>> + * something went wrong - event handler shutting >>> + * the connection and doing reset - bail out >>> + */ >>> + dev_err(dev, "Event happended - handler is active\n"); >>> + return -EAGAIN; >>> + } >>> + >> >> detach_store() can queue shutdown event as soon as reaching "/* end >> the lock */" line >> but attach_store() might be preempted immediately after verifying that >> usbip_event_happened() was false (i.e. at this location) in order to >> wait for >> shutdown event posted by detach_store() to be processed. >> >
Please don't review code that isn't sent upstream. This repo you are looking at is a private branch created just to verify fixes on syzbot.
I understand the race window you are talking about. I have my way of working to resolve it.
thanks, -- Shuah
| |