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

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

[Jeffin Philip]

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
Changes in v2:
 - Addressed concerns raised by the Greg KH in v1 discussion
   - Added usb_get_dev() to get a reference to udev preventing
     it from becoming null after the null check. Drop the reference
     after using it in do_rebind(). Did not fix the race.
v1:
 - Initial patch with a udev null check that returns -ENODEV if udev
   is null.
---

Jeffin Philip (2):
  usbip: usbip_host: fix null pointer dereference in rebind_store
  usbip: tools: replace faulty rebind_store with drivers_probe

 drivers/usb/usbip/stub_main.c      | 27 +--------------------------
 tools/usb/usbip/src/usbip_unbind.c |  7 +++----
 2 files changed, 4 insertions(+), 30 deletions(-)

--
2.55.0

[PATCH v3 1/2] usbip: usbip_host: fix null pointer dereference in rebind_store

[Jeffin Philip]

rebind_store drops locks to execute do_rebind which sleeps
during which time udev may become NULL due to physical disconnect.
Since this cannot be prevented and spinlocks cannot be obtained
in do_rebind, we turn towards the function that performs the
same action, drivers_probe, safely. Remove rebind_store and print a
warning to the user to use drivers_probe instead as a safer
alternative.

Reported-by: syzbot+af76b01c9a0f0ab60fb0@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=af76b01c9a0f0ab60fb0
Fixes: 4bfb141bc013 ("usbip: usbip_host: fix to hold parent lock for device_attach() calls")
Cc: stable@vger.kernel.org
Signed-off-by: Jeffin Philip <jeffinphilip14@gmail.com>
---
Changes in v3:
 - Addressed the race condition proposed by Greg KH in v2
   discussion.
Changes in v2:
 - Addressed concerns raised by the Greg KH in v1 discussion
   - Added usb_get_dev() to get a reference to udev preventing
     it from becoming null after the null check. Drop the reference
     after using it in do_rebind()
v1:
 - Initial patch with a udev null check that returns -ENODEV if udev
   is null.
---
 drivers/usb/usbip/stub_main.c | 27 +--------------------------
 1 file changed, 1 insertion(+), 26 deletions(-)

diff --git a/drivers/usb/usbip/stub_main.c b/drivers/usb/usbip/stub_main.c
index 79110a69d697..013f1563b1e9 100644
--- a/drivers/usb/usbip/stub_main.c
+++ b/drivers/usb/usbip/stub_main.c
@@ -242,32 +242,7 @@ static void stub_device_rebind(void)
 static ssize_t rebind_store(struct device_driver *dev, const char *buf,
 				 size_t count)
 {
-	int ret;
-	int len;
-	struct bus_id_priv *bid;
-
-	/* buf length should be less that BUSID_SIZE */
-	len = strnlen(buf, BUSID_SIZE);
-
-	if (!(len < BUSID_SIZE))
-		return -EINVAL;
-
-	bid = get_busid_priv(buf);
-	if (!bid)
-		return -ENODEV;
-
-	/* mark the device for deletion so probe ignores it during rescan */
-	bid->status = STUB_BUSID_OTHER;
-	/* release the busid lock */
-	put_busid_priv(bid);
-
-	ret = do_rebind((char *) buf, bid);
-	if (ret < 0)
-		return ret;
-
-	/* delete device from busid_table */
-	del_match_busid((char *) buf);
-
+	pr_warn("rebind node is deprecated, consider using drivers_probe instead\n");
 	return count;
 }

--
2.55.0

[PATCH v3 2/2] usbip: tools: replace faulty rebind_store with drivers_probe

[Jeffin Philip]

unbind_device() currently unbinds and then uses rebind_store
to rebind the device back to the usb driver. This is already
possible with the safer drivers_probe rather than the faulty
rebind_store. Fix this by referencing drivers_probe rather
than rebind_store.

Fixes: a46034ca57ed ("staging: usbip: trigger driver probing after unbinding from usbip-host")
Signed-off-by: Jeffin Philip <jeffinphilip14@gmail.com>
---
 tools/usb/usbip/src/usbip_unbind.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/tools/usb/usbip/src/usbip_unbind.c b/tools/usb/usbip/src/usbip_unbind.c
index 66a44d4a0d56..242280cec452 100644
--- a/tools/usb/usbip/src/usbip_unbind.c
+++ b/tools/usb/usbip/src/usbip_unbind.c
@@ -34,7 +34,7 @@ static int unbind_device(char *busid)
 
 	char unbind_attr_name[] = "unbind";
 	char unbind_attr_path[SYSFS_PATH_MAX];
-	char rebind_attr_name[] = "rebind";
+	char rebind_attr_name[] = "drivers_probe";
 	char rebind_attr_path[SYSFS_PATH_MAX];
 
 	struct udev *udev;
@@ -77,9 +77,8 @@ static int unbind_device(char *busid)
 	}
 
 	/* Trigger new probing. */
-	snprintf(rebind_attr_path, sizeof(unbind_attr_path), "%s/%s/%s/%s/%s/%s",
-			SYSFS_MNT_PATH, SYSFS_BUS_NAME, bus_type, SYSFS_DRIVERS_NAME,
-			USBIP_HOST_DRV_NAME, rebind_attr_name);
+	snprintf(rebind_attr_path, sizeof(rebind_attr_path), "%s/%s/%s/%s",
+			SYSFS_MNT_PATH, SYSFS_BUS_NAME, bus_type, rebind_attr_name);
 
 	rc = write_sysfs_attribute(rebind_attr_path, busid, strlen(busid));
 	if (rc < 0) {
--
2.55.0

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

[Shuah Khan]

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

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

[Jeffin Philip]

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.

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