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

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? Is this generated code or did you
write it?

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?

Did you run tests to see if you can bind and unbind devices - does the
driver work correctly?

thanks,
-- Shuah