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

[Shuah Khan]

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@xxxxxxxxx>
> > > ---
> > > 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?

> > 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,
-- Shuah