From: Jeffin Philip <jeffinphilip14@gmail.com>
To: skhan@linuxfoundation.org
Cc: gregkh@linuxfoundation.org, i@zenithal.me,
jeffinphilip14@gmail.com, 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 v3 0/2] usbip: usbip_host: remove legacy rebind_store in favor of drivers_probe
Date: Wed, 12 Aug 2026 08:10:50 +0530 [thread overview]
Message-ID: <20260812024050.11267-1-jeffinphilip14@gmail.com> (raw)
In-Reply-To: <e3473010-e995-40b6-9040-e38f51d1a6c9@linuxfoundation.org>
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.
prev parent reply other threads:[~2026-08-12 2:41 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-11 16:05 [PATCH v3 0/2] usbip: usbip_host: remove legacy rebind_store in favor of drivers_probe Jeffin Philip
2026-08-11 16:05 ` [PATCH v3 1/2] usbip: usbip_host: fix null pointer dereference in rebind_store Jeffin Philip
2026-08-11 16:05 ` [PATCH v3 2/2] usbip: tools: replace faulty rebind_store with drivers_probe Jeffin Philip
2026-08-11 22:53 ` [PATCH v3 0/2] usbip: usbip_host: remove legacy rebind_store in favor of drivers_probe Shuah Khan
2026-08-12 2:40 ` Jeffin Philip [this message]
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=20260812024050.11267-1-jeffinphilip14@gmail.com \
--to=jeffinphilip14@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=i@zenithal.me \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=shuah@kernel.org \
--cc=skhan@linuxfoundation.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