The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Jeffin Philip <jeffinphilip14@gmail.com>
Cc: i@zenithal.me, linux-kernel@vger.kernel.org,
	linux-usb@vger.kernel.org, shuah@kernel.org,
	stable@vger.kernel.org,
	syzbot+af76b01c9a0f0ab60fb0@syzkaller.appspotmail.com,
	valentina.manea.m@gmail.com
Subject: Re: [PATCH v2] usbip: usbip_host: fix null pointer dereference in
Date: Fri, 7 Aug 2026 15:09:56 +0200	[thread overview]
Message-ID: <2026080737-drank-trodden-16e4@gregkh> (raw)
In-Reply-To: <20260807101822.20922-1-jeffinphilip14@gmail.com>

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

  reply	other threads:[~2026-08-07 13:10 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-07  4:37 [PATCH v2] usbip: usbip_host: fix null pointer dereference in Jeffin Philip
2026-08-07  6:00 ` Greg KH
2026-08-07 10:18   ` Jeffin Philip
2026-08-07 13:09     ` Greg KH [this message]
2026-08-07 15:36       ` Jeffin Philip

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2026080737-drank-trodden-16e4@gregkh \
    --to=gregkh@linuxfoundation.org \
    --cc=i@zenithal.me \
    --cc=jeffinphilip14@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=shuah@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=syzbot+af76b01c9a0f0ab60fb0@syzkaller.appspotmail.com \
    --cc=valentina.manea.m@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox