drivers/usb/gadget/function/f_phonet.c | 34 +++++++++++++------------- drivers/usb/gadget/function/u_phonet.h | 10 ++++++++ 2 files changed, 27 insertions(+), 17 deletions(-)
pn_bind() and phonet_free_inst() race on opts->bound and opts->net.
If configfs removes the function instance while pn_bind() is between
the !bound check and setting bound = true, free_inst() frees opts->net
and pn_bind() then writes to net->dev.parent via gphonet_set_gadget().
The existing "no race condition" comment was wrong: configfs_rmdir()
can run independently of the composite bind sequence.
Add a mutex to f_phonet_opts and use scoped_guard(mutex) in both
pn_bind() and phonet_free_inst() to serialize access to ->bound and
->net. Add a kernel-doc comment describing what the lock protects,
and destroy the mutex before freeing opts.
Fixes: 00a2430ff07d ("usb: gadget: Gadget directory cleanup - group usb functions")
Reported-by: syzbot+098999e05b6b877c01b3@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=098999e05b6b877c01b3
Signed-off-by: Nguyen Quang Le Kien <khiemtranzo532001@gmail.com>
---
v2:
- use scoped_guard(mutex) instead of open-coded lock/unlock
- add kernel-doc comment on struct f_phonet_opts describing what
@lock protects
- add explicit #include <linux/mutex.h>
- call mutex_destroy() before kfree(opts)
- remove stale "no race condition" comment; explain why it was wrong
in the commit message
---
drivers/usb/gadget/function/f_phonet.c | 34 +++++++++++++-------------
drivers/usb/gadget/function/u_phonet.h | 10 ++++++++
2 files changed, 27 insertions(+), 17 deletions(-)
diff --git a/drivers/usb/gadget/function/f_phonet.c b/drivers/usb/gadget/function/f_phonet.c
index b1ee9a7c2..350579747 100644
--- a/drivers/usb/gadget/function/f_phonet.c
+++ b/drivers/usb/gadget/function/f_phonet.c
@@ -12,6 +12,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/device.h>
+#include <linux/mutex.h>
#include <linux/netdevice.h>
#include <linux/if_ether.h>
@@ -499,19 +500,14 @@ static int pn_bind(struct usb_configuration *c, struct usb_function *f)
phonet_opts = container_of(f->fi, struct f_phonet_opts, func_inst);
- /*
- * in drivers/usb/gadget/configfs.c:configfs_composite_bind()
- * configurations are bound in sequence with list_for_each_entry,
- * in each configuration its functions are bound in sequence
- * with list_for_each_entry, so we assume no race condition
- * with regard to phonet_opts->bound access
- */
- if (!phonet_opts->bound) {
- gphonet_set_gadget(phonet_opts->net, gadget);
- status = gphonet_register_netdev(phonet_opts->net);
- if (status)
- return status;
- phonet_opts->bound = true;
+ scoped_guard(mutex, &phonet_opts->lock) {
+ if (!phonet_opts->bound) {
+ gphonet_set_gadget(phonet_opts->net, gadget);
+ status = gphonet_register_netdev(phonet_opts->net);
+ if (status)
+ return status;
+ phonet_opts->bound = true;
+ }
}
/* Reserve interface IDs */
@@ -621,10 +617,13 @@ static void phonet_free_inst(struct usb_function_instance *f)
struct f_phonet_opts *opts;
opts = container_of(f, struct f_phonet_opts, func_inst);
- if (opts->bound)
- gphonet_cleanup(opts->net);
- else
- free_netdev(opts->net);
+ scoped_guard(mutex, &opts->lock) {
+ if (opts->bound)
+ gphonet_cleanup(opts->net);
+ else
+ free_netdev(opts->net);
+ }
+ mutex_destroy(&opts->lock);
kfree(opts);
}
@@ -636,6 +635,7 @@ static struct usb_function_instance *phonet_alloc_inst(void)
if (!opts)
return ERR_PTR(-ENOMEM);
+ mutex_init(&opts->lock);
opts->func_inst.free_func_inst = phonet_free_inst;
opts->net = gphonet_setup_default();
if (IS_ERR(opts->net)) {
diff --git a/drivers/usb/gadget/function/u_phonet.h b/drivers/usb/gadget/function/u_phonet.h
index ff62ca22c..54fadfe64 100644
--- a/drivers/usb/gadget/function/u_phonet.h
+++ b/drivers/usb/gadget/function/u_phonet.h
@@ -8,11 +8,21 @@
#ifndef __U_PHONET_H
#define __U_PHONET_H
+#include <linux/mutex.h>
#include <linux/usb/composite.h>
#include <linux/usb/cdc.h>
+/**
+ * struct f_phonet_opts - Phonet function instance options
+ * @func_inst: USB function instance
+ * @lock: protects @bound and @net against concurrent access from
+ * pn_bind() vs phonet_free_inst() during configfs teardown
+ * @bound: true once pn_bind() has successfully registered @net
+ * @net: net_device owned by this function instance
+ */
struct f_phonet_opts {
struct usb_function_instance func_inst;
+ struct mutex lock;
bool bound;
struct net_device *net;
};
--
2.34.1
On Tue, Aug 04, 2026 at 03:44:32PM +0800, Nguyen Quang Le Kien wrote:
> pn_bind() and phonet_free_inst() race on opts->bound and opts->net.
> If configfs removes the function instance while pn_bind() is between
> the !bound check and setting bound = true, free_inst() frees opts->net
> and pn_bind() then writes to net->dev.parent via gphonet_set_gadget().
>
> The existing "no race condition" comment was wrong: configfs_rmdir()
> can run independently of the composite bind sequence.
>
> Add a mutex to f_phonet_opts and use scoped_guard(mutex) in both
> pn_bind() and phonet_free_inst() to serialize access to ->bound and
> ->net. Add a kernel-doc comment describing what the lock protects,
> and destroy the mutex before freeing opts.
>
> Fixes: 00a2430ff07d ("usb: gadget: Gadget directory cleanup - group usb functions")
> Reported-by: syzbot+098999e05b6b877c01b3@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=098999e05b6b877c01b3
Did this new version properly run through syzbot and it reported it
succeeded?
> Signed-off-by: Nguyen Quang Le Kien <khiemtranzo532001@gmail.com>
> ---
> v2:
> - use scoped_guard(mutex) instead of open-coded lock/unlock
> - add kernel-doc comment on struct f_phonet_opts describing what
> @lock protects
> - add explicit #include <linux/mutex.h>
> - call mutex_destroy() before kfree(opts)
> - remove stale "no race condition" comment; explain why it was wrong
> in the commit message
> ---
> drivers/usb/gadget/function/f_phonet.c | 34 +++++++++++++-------------
> drivers/usb/gadget/function/u_phonet.h | 10 ++++++++
> 2 files changed, 27 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_phonet.c b/drivers/usb/gadget/function/f_phonet.c
> index b1ee9a7c2..350579747 100644
> --- a/drivers/usb/gadget/function/f_phonet.c
> +++ b/drivers/usb/gadget/function/f_phonet.c
> @@ -12,6 +12,7 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/device.h>
> +#include <linux/mutex.h>
This isn't needed as you added it to the .h file, right?
And you didn't answer my question about LLM use.
thanks,
greg k-h
#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> #syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master This crash does not have a reproducer. I cannot test it.
On Tue, Aug 04, 2026 at 09:52:53AM +0200, Greg KH wrote: > Did this new version properly run through syzbot and it reported it succeeded? > > This isn't needed as you added it to the .h file, right? > > And you didn't answer my question about LLM use. Not yet - I sent v2 about 30 minutes ago and syzbot hasn't picked it up. I'll wait for the test result before sending v3. You're right about the redundant include; that will be gone in v3. On the LLM question: I used it as a drafting assistant for the v1 changelog and comment wording, but the race analysis and the actual fix are my own, and I build-tested the patch. The over-formulaic changelog and leaving the old stale "no race" comment in v1 were on me. I wrote v2 and the upcoming v3 directly. Will send v3 once syzbot reports back.
On Tue, Aug 04, 2026 at 04:18:53PM +0800, Nguyen Quang Le Kien wrote: > On Tue, Aug 04, 2026 at 09:52:53AM +0200, Greg KH wrote: > > Did this new version properly run through syzbot and it reported it succeeded? > > > > This isn't needed as you added it to the .h file, right? > > > > And you didn't answer my question about LLM use. > > Not yet - I sent v2 about 30 minutes ago and syzbot hasn't picked it up. > I'll wait for the test result before sending v3. Please always do that before asking a human to review it. > You're right about the redundant include; that will be gone in v3. > > On the LLM question: I used it as a drafting assistant for the v1 > changelog and comment wording, but the race analysis and the actual fix > are my own, and I build-tested the patch. The over-formulaic changelog > and leaving the old stale "no race" comment in v1 were on me. I wrote v2 > and the upcoming v3 directly. Will send v3 once syzbot reports back. Please always document your LLM usage, as our rules require you to. thanks, greg k-h
On Tue, Aug 04, 2026 at 08:21:00AM +0200, Greg KH wrote: > Please always do that before asking a human to review it. syzbot actually already replied to my test request in this thread: "This crash does not have a reproducer. I cannot test it." So there is nothing to wait for - it simply can't test this one. v3 (redundant include removed) coming up. Thanks, Nguyen Quang Le Kien
On Wed, Aug 05, 2026 at 12:08:13PM +0800, Nguyen Quang Le Kien wrote: > On Tue, Aug 04, 2026 at 08:21:00AM +0200, Greg KH wrote: > > Please always do that before asking a human to review it. > > syzbot actually already replied to my test request in this thread: > "This crash does not have a reproducer. I cannot test it." So there > is nothing to wait for - it simply can't test this one. Then you do not know if this fixes the problem or not :(
On Wed, Aug 05, 2026 at XX:XX, Greg KH wrote: > Then you do not know if this fixes the problem or not :( I get that. Quick question - is there a way to actually verify this short of waiting for syzbot to get a reproducer? I was thinking maybe fault injection or something, but not sure if that's the right approach here. Thanks, Nguyen Quang Le Kien
On Wed, Aug 05, 2026 at 02:57:03PM +0800, Nguyen Quang Le Kien wrote: > On Wed, Aug 05, 2026 at XX:XX, Greg KH wrote: > > Then you do not know if this fixes the problem or not :( > > I get that. Quick question - is there a way to actually verify this > short of waiting for syzbot to get a reproducer? I was thinking > maybe fault injection or something, but not sure if that's the right > approach here. I do not know, but step back, why are you trying to fix this issue at all if you can not reproduce it and you do not have the hardware to test it for? thanks, greg k-h
On Wed, Aug 05, 2026 at XX:XX, Greg KH wrote: > I do not know, but step back, why are you trying to fix this issue at > all if you can not reproduce it and you do not have the hardware to test > it for? The race is clear from reading the code - the old comment was just wrong. syzbot hit it 13 times so I figured the fix was worth sending even without a local reproducer. But if you'd rather not merge it without one, that's fine too. Thanks, Nguyen Quang Le Kien
On Wed, Aug 05, 2026 at 04:24:02PM +0800, Nguyen Quang Le Kien wrote: > On Wed, Aug 05, 2026 at XX:XX, Greg KH wrote: > > I do not know, but step back, why are you trying to fix this issue at > > all if you can not reproduce it and you do not have the hardware to test > > it for? > > The race is clear from reading the code - the old comment was just > wrong. syzbot hit it 13 times so I figured the fix was worth sending > even without a local reproducer. Great, but what drew you to wanting to fix this specific syzbot issue? Do you have this hardware that you need to see this issue resolved for? thanks, greg k-h
On Wed, Aug 05, 2026 at XX:XX, Greg KH wrote: > Great, but what drew you to wanting to fix this specific syzbot issue? > Do you have this hardware that you need to see this issue resolved for? No, I don't have Phonet hardware. I work on USB kernel code at my job and was looking through open syzbot USB bugs to get practice on real issues. The race here was easy to spot so I sent a fix. Thanks, Nguyen Quang Le Kien
pn_bind() and phonet_free_inst() race on opts->bound and opts->net.
If configfs removes the function instance while pn_bind() is between
the !bound check and setting bound = true, free_inst() frees opts->net
and pn_bind() then writes to net->dev.parent via gphonet_set_gadget().
The old "no race condition" comment was wrong - configfs_rmdir() can
run in parallel with the composite bind path.
Add a mutex to f_phonet_opts, use scoped_guard(mutex) in both paths,
add kernel-doc on the struct, and destroy the mutex before freeing
opts.
Fixes: 00a2430ff07d ("usb: gadget: Gadget directory cleanup - group usb functions")
Reported-by: syzbot+098999e05b6b877c01b3@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=098999e05b6b877c01b3
Signed-off-by: Nguyen Quang Le Kien <khiemtranzo532001@gmail.com>
---
v3: drop the redundant #include <linux/mutex.h> in f_phonet.c -
u_phonet.h already includes it.
v2: scoped_guard(mutex) instead of open-coded lock/unlock; kernel-doc
on the struct; mutex_destroy(); remove the wrong comment.
---
drivers/usb/gadget/function/f_phonet.c | 33 +++++++++++++-------------
drivers/usb/gadget/function/u_phonet.h | 10 ++++++++
2 files changed, 26 insertions(+), 17 deletions(-)
diff --git a/drivers/usb/gadget/function/f_phonet.c b/drivers/usb/gadget/function/f_phonet.c
index b1ee9a7c2..e14ee91a8 100644
--- a/drivers/usb/gadget/function/f_phonet.c
+++ b/drivers/usb/gadget/function/f_phonet.c
@@ -499,19 +499,14 @@ static int pn_bind(struct usb_configuration *c, struct usb_function *f)
phonet_opts = container_of(f->fi, struct f_phonet_opts, func_inst);
- /*
- * in drivers/usb/gadget/configfs.c:configfs_composite_bind()
- * configurations are bound in sequence with list_for_each_entry,
- * in each configuration its functions are bound in sequence
- * with list_for_each_entry, so we assume no race condition
- * with regard to phonet_opts->bound access
- */
- if (!phonet_opts->bound) {
- gphonet_set_gadget(phonet_opts->net, gadget);
- status = gphonet_register_netdev(phonet_opts->net);
- if (status)
- return status;
- phonet_opts->bound = true;
+ scoped_guard(mutex, &phonet_opts->lock) {
+ if (!phonet_opts->bound) {
+ gphonet_set_gadget(phonet_opts->net, gadget);
+ status = gphonet_register_netdev(phonet_opts->net);
+ if (status)
+ return status;
+ phonet_opts->bound = true;
+ }
}
/* Reserve interface IDs */
@@ -621,10 +616,13 @@ static void phonet_free_inst(struct usb_function_instance *f)
struct f_phonet_opts *opts;
opts = container_of(f, struct f_phonet_opts, func_inst);
- if (opts->bound)
- gphonet_cleanup(opts->net);
- else
- free_netdev(opts->net);
+ scoped_guard(mutex, &opts->lock) {
+ if (opts->bound)
+ gphonet_cleanup(opts->net);
+ else
+ free_netdev(opts->net);
+ }
+ mutex_destroy(&opts->lock);
kfree(opts);
}
@@ -636,6 +634,7 @@ static struct usb_function_instance *phonet_alloc_inst(void)
if (!opts)
return ERR_PTR(-ENOMEM);
+ mutex_init(&opts->lock);
opts->func_inst.free_func_inst = phonet_free_inst;
opts->net = gphonet_setup_default();
if (IS_ERR(opts->net)) {
diff --git a/drivers/usb/gadget/function/u_phonet.h b/drivers/usb/gadget/function/u_phonet.h
index ff62ca22c..54fadfe64 100644
--- a/drivers/usb/gadget/function/u_phonet.h
+++ b/drivers/usb/gadget/function/u_phonet.h
@@ -8,11 +8,21 @@
#ifndef __U_PHONET_H
#define __U_PHONET_H
+#include <linux/mutex.h>
#include <linux/usb/composite.h>
#include <linux/usb/cdc.h>
+/**
+ * struct f_phonet_opts - Phonet function instance options
+ * @func_inst: USB function instance
+ * @lock: protects @bound and @net against concurrent access from
+ * pn_bind() vs phonet_free_inst() during configfs teardown
+ * @bound: true once pn_bind() has successfully registered @net
+ * @net: net_device owned by this function instance
+ */
struct f_phonet_opts {
struct usb_function_instance func_inst;
+ struct mutex lock;
bool bound;
struct net_device *net;
};
--
2.34.1
On Wed, Aug 05, 2026 at 12:13:19PM +0800, Nguyen Quang Le Kien wrote:
> pn_bind() and phonet_free_inst() race on opts->bound and opts->net.
> If configfs removes the function instance while pn_bind() is between
> the !bound check and setting bound = true, free_inst() frees opts->net
> and pn_bind() then writes to net->dev.parent via gphonet_set_gadget().
>
> The old "no race condition" comment was wrong - configfs_rmdir() can
> run in parallel with the composite bind path.
>
> Add a mutex to f_phonet_opts, use scoped_guard(mutex) in both paths,
> add kernel-doc on the struct, and destroy the mutex before freeing
> opts.
>
> Fixes: 00a2430ff07d ("usb: gadget: Gadget directory cleanup - group usb functions")
> Reported-by: syzbot+098999e05b6b877c01b3@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=098999e05b6b877c01b3
> Signed-off-by: Nguyen Quang Le Kien <khiemtranzo532001@gmail.com>
> ---
> v3: drop the redundant #include <linux/mutex.h> in f_phonet.c -
> u_phonet.h already includes it.
>
> v2: scoped_guard(mutex) instead of open-coded lock/unlock; kernel-doc
> on the struct; mutex_destroy(); remove the wrong comment.
You did not use the Assisted-by: tag as you should have :(
© 2016 - 2026 Red Hat, Inc.