[PATCH v2] usbip: usbip_host: fix null pointer dereference in

[PATCH v2] usbip: usbip_host: fix null pointer dereference in

[Jeffin Philip]

rebind_store calls do_rebind which dereferences udev without
checking if it is NULL. If busid is registered using match_busid
but the device is never bound to the driver or it is never present
in the first place, it triggers a null pointer dereference when we
attempt to rebind the device. Fix this by checking explicitly for
udev first and returning -ENODEV if udev is NULL. Add usb_get_dev
in addition to the null check to get a reference to udev to prevent
udev from becoming NULL after the check. Drop the reference after
using it in do_rebind().

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 v2:
 - Addressed concerns raised by the maintainer 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()
---
 drivers/usb/usbip/stub_main.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/usb/usbip/stub_main.c b/drivers/usb/usbip/stub_main.c
index 79110a69d697..c911626427dc 100644
--- a/drivers/usb/usbip/stub_main.c
+++ b/drivers/usb/usbip/stub_main.c
@@ -256,12 +256,21 @@ static ssize_t rebind_store(struct device_driver *dev, const char *buf,
 	if (!bid)
 		return -ENODEV;
 
+	if (!bid->udev) {
+		put_busid_priv(bid);
+		return -ENODEV;
+	}
+
+	/* get a reference to udev to prevent it from becoming NULL */
+	usb_get_dev(bid->udev);
 	/* 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);
+	/* we finished using udev and don't need the reference anymore, drop it */
+	usb_put_dev(bid->udev);
 	if (ret < 0)
 		return ret;
 
-- 
2.55.0

Re: [PATCH v2] usbip: usbip_host: fix null pointer dereference in

[Greg KH]

On Fri, Aug 07, 2026 at 10:07:59AM +0530, Jeffin Philip wrote:
> rebind_store calls do_rebind which dereferences udev without
> checking if it is NULL. If busid is registered using match_busid
> but the device is never bound to the driver or it is never present
> in the first place, it triggers a null pointer dereference when we
> attempt to rebind the device. Fix this by checking explicitly for
> udev first and returning -ENODEV if udev is NULL. Add usb_get_dev
> in addition to the null check to get a reference to udev to prevent
> udev from becoming NULL after the check. Drop the reference after
> using it in do_rebind().
> 
> 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 v2:
>  - Addressed concerns raised by the maintainer 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()
> ---
>  drivers/usb/usbip/stub_main.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/usb/usbip/stub_main.c b/drivers/usb/usbip/stub_main.c
> index 79110a69d697..c911626427dc 100644
> --- a/drivers/usb/usbip/stub_main.c
> +++ b/drivers/usb/usbip/stub_main.c
> @@ -256,12 +256,21 @@ static ssize_t rebind_store(struct device_driver *dev, const char *buf,
>  	if (!bid)
>  		return -ENODEV;
>  
> +	if (!bid->udev) {
> +		put_busid_priv(bid);
> +		return -ENODEV;
> +	}
> +
> +	/* get a reference to udev to prevent it from becoming NULL */
> +	usb_get_dev(bid->udev);

So what happens if udev becomes NULL after checking it and before
grabbing the reference?

This is not how to handle this at all.  Please step back and look at the
root problem here and address that.  This is just papering over the
issue.

thanks,

greg k-h

[PATCH v2] usbip: usbip_host: fix null pointer dereference in

[Jeffin Philip]

On Fri, Aug 07, 2026 at 08:00:31 +0200, Greg KH wrote:
>On Fri, Aug 07, 2026 at 10:07:59AM +0530, Jeffin Philip wrote:
>> rebind_store calls do_rebind which dereferences udev without
>> checking if it is NULL. If busid is registered using match_busid
>> but the device is never bound to the driver or it is never present
>> in the first place, it triggers a null pointer dereference when we
>> attempt to rebind the device. Fix this by checking explicitly for
>> udev first and returning -ENODEV if udev is NULL. Add usb_get_dev
>> in addition to the null check to get a reference to udev to prevent
>> udev from becoming NULL after the check. Drop the reference after
>> using it in do_rebind().
>> 
>> 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 v2:
>>  - Addressed concerns raised by the maintainer 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()
>> ---
>>  drivers/usb/usbip/stub_main.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>> 
>> diff --git a/drivers/usb/usbip/stub_main.c b/drivers/usb/usbip/stub_main.c
>> index 79110a69d697..c911626427dc 100644
>> --- a/drivers/usb/usbip/stub_main.c
>> +++ b/drivers/usb/usbip/stub_main.c
>> @@ -256,12 +256,21 @@ static ssize_t rebind_store(struct device_driver *dev, const char *buf,
>>  	if (!bid)
>>  		return -ENODEV;
>>  
>> +	if (!bid->udev) {
>> +		put_busid_priv(bid);
>> +		return -ENODEV;
>> +	}
>> +
>> +	/* get a reference to udev to prevent it from becoming NULL */
>> +	usb_get_dev(bid->udev);
>
>So what happens if udev becomes NULL after checking it and before
>grabbing the reference?

I looked through the driver and could not find where udev becomes NULL after
checking and before getting the reference since we do both of those under
busid lock. There is a small window between releasing the lock and
do_rebind, is that what what you are referring to? If so or otherwise, 
could you please advise me on how to move forward in correcting this patch?

Thanks,
Jeffin.

Re: [PATCH v2] usbip: usbip_host: fix null pointer dereference in

[Greg KH]

On Fri, Aug 07, 2026 at 03:48:22PM +0530, Jeffin Philip wrote:
> On Fri, Aug 07, 2026 at 08:00:31 +0200, Greg KH wrote:
> >On Fri, Aug 07, 2026 at 10:07:59AM +0530, Jeffin Philip wrote:
> >> rebind_store calls do_rebind which dereferences udev without
> >> checking if it is NULL. If busid is registered using match_busid
> >> but the device is never bound to the driver or it is never present
> >> in the first place, it triggers a null pointer dereference when we
> >> attempt to rebind the device. Fix this by checking explicitly for
> >> udev first and returning -ENODEV if udev is NULL. Add usb_get_dev
> >> in addition to the null check to get a reference to udev to prevent
> >> udev from becoming NULL after the check. Drop the reference after
> >> using it in do_rebind().
> >> 
> >> 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 v2:
> >>  - Addressed concerns raised by the maintainer 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()
> >> ---
> >>  drivers/usb/usbip/stub_main.c | 9 +++++++++
> >>  1 file changed, 9 insertions(+)
> >> 
> >> diff --git a/drivers/usb/usbip/stub_main.c b/drivers/usb/usbip/stub_main.c
> >> index 79110a69d697..c911626427dc 100644
> >> --- a/drivers/usb/usbip/stub_main.c
> >> +++ b/drivers/usb/usbip/stub_main.c
> >> @@ -256,12 +256,21 @@ static ssize_t rebind_store(struct device_driver *dev, const char *buf,
> >>  	if (!bid)
> >>  		return -ENODEV;
> >>  
> >> +	if (!bid->udev) {
> >> +		put_busid_priv(bid);
> >> +		return -ENODEV;
> >> +	}
> >> +
> >> +	/* get a reference to udev to prevent it from becoming NULL */
> >> +	usb_get_dev(bid->udev);
> >
> >So what happens if udev becomes NULL after checking it and before
> >grabbing the reference?
> 
> I looked through the driver and could not find where udev becomes NULL after
> checking and before getting the reference since we do both of those under
> busid lock. There is a small window between releasing the lock and
> do_rebind, is that what what you are referring to? If so or otherwise, 
> could you please advise me on how to move forward in correcting this patch?

If you do not hold a lock when testing and doing something based on a
field, it will race and is broken.

Again, step back and try to determine what you are trying to fix here,
and how that can be done in a race-free way.  If you don't know, that's
fine too, I sure don't!  :)

But we can't take a change that doesn't actually fix the issue, you
wouldn't want that, right?

Why are you looking at this issue anyway?  Did someone assign it to you?

thanks,

greg k-h

Re: [PATCH v2] usbip: usbip_host: fix null pointer dereference in

[Jeffin Philip]

On Fri, Aug 07, 2026 at 03:09:56PM +0200, Greg KH wrote:
>On Fri, Aug 07, 2026 at 03:48:22PM +0530, Jeffin Philip wrote:
>> On Fri, Aug 07, 2026 at 08:00:31 +0200, Greg KH wrote:
>> >On Fri, Aug 07, 2026 at 10:07:59AM +0530, Jeffin Philip wrote:
>> >> rebind_store calls do_rebind which dereferences udev without
>> >> checking if it is NULL. If busid is registered using match_busid
>> >> but the device is never bound to the driver or it is never present
>> >> in the first place, it triggers a null pointer dereference when we
>> >> attempt to rebind the device. Fix this by checking explicitly for
>> >> udev first and returning -ENODEV if udev is NULL. Add usb_get_dev
>> >> in addition to the null check to get a reference to udev to prevent
>> >> udev from becoming NULL after the check. Drop the reference after
>> >> using it in do_rebind().
>> >> 
>> >> 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 v2:
>> >>  - Addressed concerns raised by the maintainer 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()
>> >> ---
>> >>  drivers/usb/usbip/stub_main.c | 9 +++++++++
>> >>  1 file changed, 9 insertions(+)
>> >> 
>> >> diff --git a/drivers/usb/usbip/stub_main.c b/drivers/usb/usbip/stub_main.c
>> >> index 79110a69d697..c911626427dc 100644
>> >> --- a/drivers/usb/usbip/stub_main.c
>> >> +++ b/drivers/usb/usbip/stub_main.c
>> >> @@ -256,12 +256,21 @@ static ssize_t rebind_store(struct device_driver *dev, const char *buf,
>> >>  	if (!bid)
>> >>  		return -ENODEV;
>> >>  
>> >> +	if (!bid->udev) {
>> >> +		put_busid_priv(bid);
>> >> +		return -ENODEV;
>> >> +	}
>> >> +
>> >> +	/* get a reference to udev to prevent it from becoming NULL */
>> >> +	usb_get_dev(bid->udev);
>> >
>> >So what happens if udev becomes NULL after checking it and before
>> >grabbing the reference?
>> 
>> I looked through the driver and could not find where udev becomes NULL after
>> checking and before getting the reference since we do both of those under
>> busid lock. There is a small window between releasing the lock and
>> do_rebind, is that what what you are referring to? If so or otherwise, 
>> could you please advise me on how to move forward in correcting this patch?
>
>If you do not hold a lock when testing and doing something based on a
>field, it will race and is broken.
>
>Again, step back and try to determine what you are trying to fix here,
>and how that can be done in a race-free way.  If you don't know, that's
>fine too, I sure don't!  :)

We could add a new flag(in bus_id_priv->status) that would prevent probe from
scanning or any other function from nullifying udev and that would prevent race
conditions. But I think that would be an unnecessarily large change just for a
small issue like null udev. We could also use a local variable to store the udev
so it does not become null after dropping locks. Apart from these, I don't have 
other ideas.

>But we can't take a change that doesn't actually fix the issue, you
>wouldn't want that, right?

Yes, I don't want to send bad patches either. 

>Why are you looking at this issue anyway?  Did someone assign it to you?

I am relatively new to usb subsystem and have been wanting to learn it for
quite some time. So I got on syzbot where I found this issue, I wanted to
learn the subsystem in practice, that's all. Nobody assigned me this.

Thanks,
Jeffin.