Re: [PATCH v3 0/2] usbip: usbip_host: remove legacy rebind_store in favor of drivers_probe
Jeffin Philip <[email protected]>
| Newsgroups | org.kernel.vger.stable,org.kernel.vger.linux-kernel,org.kernel.vger.linux-usb |
|---|---|
| Message-ID | <[email protected]> |
On Tue, Aug 11 2026, at 16:53:23 -0600, Shuah Khan wrote: >On 8/11/26 10:05, Jeffin Philip wrote: >> do_rebind, which sleeps normally gets a mutex lock. However, it does not >> or should I say, cannot check for null udev between spin lock dropped in >> rebind_store and entering do_rebind. This is a potential race window >> already. So, even if we check for null udev under spinlock, we cannot do >> it outside. Regarding do_rebind, it is called during stub_device_rebind, >> but that function is called during module exit when all files are removed. >> So, do_rebind is not designed to work in a concurrent environment in the >> first place. >> >> We have a safer function that can already do what do_rebind does, >> drivers_probe. So, we use drivers_probe to rebind the device rather than >> use rebind_store. >> >> usbip tool references this function immediately after the device is unbound, >> which is safe for the tool itself but since we opted for drivers_probe, fix >> it by using drivers_probe rather than rebind_store after unbinding device >> which is more safer. >> >> Tested and working in both userspace via the tool and manually echoing >> the busid in the related nodes. rebind node is still left active with a >> warning to use drivers_probe upon encountering rebind_store. >> >> Thanks, >> Jeffin. >> >> Signed-off-by: Jeffin Philip <[email protected]> >> --- >> Changes in v3: >> - Removed rebind_store in favor of drivers_probe to eliminate race >> condition > >How did you find this problem? I found the problem on syzbot and had to reproduce it using the following commands: link to issue: https://syzkaller.appspot.com/bug?extid=af76b01c9a0f0ab60fb0 echo 'add 1-1' > /sys/bus/usb/drivers/ubsip-host/match_busid echo '1-1' > /sys/bus/usb/drivers/usbip-host/rebind >Is this generated code or did you write it? No, I wrote the code myself. >Also, the first patch removes code in rebind_store(), replacing it >with a pr_warn()? The second patch points it driver_probe() - what >happens with just the first patch? It just prints out a warning. I did get a -Wunused function warning while building the kernel for testing and considered removing it. I ultimately didn't as scripts running on newer kernels(if this was merged) would break as there is no rebind node. Should I remove it? and is pr_warn not the right way to deal with this? If so, please advise. >Did you run tests to see if you can bind and unbind devices - does the >driver work correctly? Yes the tool works correctly after switching to drivers_probe. Devices are bound and unbound correctly. The function also caused an invalid opcode while testing in an unpatched kernel with a different sequence: write to match_busid, then write to bind and rebind, followed by writing to unbind. Thanks, Jeffin.