Re: [PATCH v3 0/2] usbip: usbip_host: remove legacy rebind_store in favor of drivers_probe

[Jeffin Philip <jeffinphilip14@gmail.com>]

On Wed, 12 Aug 2026, at 13:31:50 -0600, Shuah Khan wrote:
>On 8/11/26 20:40, Jeffin Philip wrote:
>> 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 <jeffinphilip14@gmail.com>
>>>> ---
>>>> 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.
>
>Okay. But why is this change split into two patches? Does just the first patch
>work correctly?

The first patch is for people echoing the busid directly to rebind node, so it
just prints out a warning asking them to use drivers_probe. The second patch is 
for the usbip tool, as the old tool used rebind store for rebinding, we replaced
it already in the first patch and since 'usbip unbind' rebinds the device back to
the usb driver after unbinding it, we cannot use rebind_store like the old tool.
That is why I replaced rebind with drivers_probe in the second patch.

>> 
>>> 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.