The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] usb: gadget: f_phonet: fix use-after-free in pn_bind
@ 2026-08-04  3:33 Nguyen Quang Le Kien
  2026-08-04  6:30 ` Greg KH
  0 siblings, 1 reply; 16+ messages in thread
From: Nguyen Quang Le Kien @ 2026-08-04  3:33 UTC (permalink / raw)
  To: gregkh
  Cc: linux-usb, linux-kernel, syzbot+098999e05b6b877c01b3,
	Nguyen Quang Le Kien

phonet_free_inst() checks opts->bound to decide whether to call
gphonet_cleanup() or free_netdev(). If opts->bound is false,
free_netdev(opts->net) is called immediately.

However, pn_bind() can race with phonet_free_inst() when a configfs
entry is removed while binding is in progress. pn_bind() reads
opts->bound and calls gphonet_set_gadget(opts->net, ...) before
setting opts->bound = true. If phonet_free_inst() runs concurrently
after the !opts->bound check in pn_bind() but before opts->bound is
set, it will free opts->net, causing a use-after-free when pn_bind()
subsequently writes to net->dev.parent via gphonet_set_gadget().

Fix this by adding a mutex to f_phonet_opts and holding it in both
pn_bind() and phonet_free_inst() when accessing opts->bound and
opts->net. This is consistent with how other gadget functions such
as f_eem protect their opts->bound flag.

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>
---
 drivers/usb/gadget/function/f_phonet.c | 16 ++++++++--------
 drivers/usb/gadget/function/u_phonet.h |  1 +
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/gadget/function/f_phonet.c b/drivers/usb/gadget/function/f_phonet.c
index b1ee9a7c2..b7eae14e8 100644
--- a/drivers/usb/gadget/function/f_phonet.c
+++ b/drivers/usb/gadget/function/f_phonet.c
@@ -499,20 +499,17 @@ 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
-	 */
+	mutex_lock(&phonet_opts->lock);
 	if (!phonet_opts->bound) {
 		gphonet_set_gadget(phonet_opts->net, gadget);
 		status = gphonet_register_netdev(phonet_opts->net);
-		if (status)
+		if (status) {
+			mutex_unlock(&phonet_opts->lock);
 			return status;
+		}
 		phonet_opts->bound = true;
 	}
+	mutex_unlock(&phonet_opts->lock);
 
 	/* Reserve interface IDs */
 	status = usb_interface_id(c, f);
@@ -621,10 +618,12 @@ 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);
+	mutex_lock(&opts->lock);
 	if (opts->bound)
 		gphonet_cleanup(opts->net);
 	else
 		free_netdev(opts->net);
+	mutex_unlock(&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..4666413fc 100644
--- a/drivers/usb/gadget/function/u_phonet.h
+++ b/drivers/usb/gadget/function/u_phonet.h
@@ -13,6 +13,7 @@
 
 struct f_phonet_opts {
 	struct usb_function_instance func_inst;
+	struct mutex lock;
 	bool bound;
 	struct net_device *net;
 };
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] usb: gadget: f_phonet: fix use-after-free in pn_bind
  2026-08-04  3:33 [PATCH] usb: gadget: f_phonet: fix use-after-free in pn_bind Nguyen Quang Le Kien
@ 2026-08-04  6:30 ` Greg KH
  2026-08-04  7:44   ` [PATCH v2] " Nguyen Quang Le Kien
  0 siblings, 1 reply; 16+ messages in thread
From: Greg KH @ 2026-08-04  6:30 UTC (permalink / raw)
  To: Nguyen Quang Le Kien; +Cc: linux-usb, linux-kernel, syzbot+098999e05b6b877c01b3

On Tue, Aug 04, 2026 at 11:33:41AM +0800, Nguyen Quang Le Kien wrote:
> phonet_free_inst() checks opts->bound to decide whether to call
> gphonet_cleanup() or free_netdev(). If opts->bound is false,
> free_netdev(opts->net) is called immediately.
> 
> However, pn_bind() can race with phonet_free_inst() when a configfs
> entry is removed while binding is in progress. pn_bind() reads
> opts->bound and calls gphonet_set_gadget(opts->net, ...) before
> setting opts->bound = true. If phonet_free_inst() runs concurrently
> after the !opts->bound check in pn_bind() but before opts->bound is
> set, it will free opts->net, causing a use-after-free when pn_bind()
> subsequently writes to net->dev.parent via gphonet_set_gadget().
> 
> Fix this by adding a mutex to f_phonet_opts and holding it in both
> pn_bind() and phonet_free_inst() when accessing opts->bound and
> opts->net. This is consistent with how other gadget functions such
> as f_eem protect their opts->bound flag.
> 
> 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>
> ---
>  drivers/usb/gadget/function/f_phonet.c | 16 ++++++++--------
>  drivers/usb/gadget/function/u_phonet.h |  1 +
>  2 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/f_phonet.c b/drivers/usb/gadget/function/f_phonet.c
> index b1ee9a7c2..b7eae14e8 100644
> --- a/drivers/usb/gadget/function/f_phonet.c
> +++ b/drivers/usb/gadget/function/f_phonet.c
> @@ -499,20 +499,17 @@ 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
> -	 */
> +	mutex_lock(&phonet_opts->lock);

So the coomment lied?

And why not use guard()?  Was an LLM used to generate this changelog and
code change?


>  	if (!phonet_opts->bound) {
>  		gphonet_set_gadget(phonet_opts->net, gadget);
>  		status = gphonet_register_netdev(phonet_opts->net);
> -		if (status)
> +		if (status) {
> +			mutex_unlock(&phonet_opts->lock);
>  			return status;
> +		}
>  		phonet_opts->bound = true;
>  	}
> +	mutex_unlock(&phonet_opts->lock);
>  
>  	/* Reserve interface IDs */
>  	status = usb_interface_id(c, f);
> @@ -621,10 +618,12 @@ 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);
> +	mutex_lock(&opts->lock);
>  	if (opts->bound)
>  		gphonet_cleanup(opts->net);
>  	else
>  		free_netdev(opts->net);
> +	mutex_unlock(&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..4666413fc 100644
> --- a/drivers/usb/gadget/function/u_phonet.h
> +++ b/drivers/usb/gadget/function/u_phonet.h
> @@ -13,6 +13,7 @@
>  
>  struct f_phonet_opts {
>  	struct usb_function_instance func_inst;
> +	struct mutex lock;

No comment as to what this lock protects?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2] usb: gadget: f_phonet: fix use-after-free in pn_bind
  2026-08-04  6:30 ` Greg KH
@ 2026-08-04  7:44   ` Nguyen Quang Le Kien
  2026-08-04  7:52     ` Greg KH
  2026-08-05  4:13     ` [PATCH v3] " Nguyen Quang Le Kien
  0 siblings, 2 replies; 16+ messages in thread
From: Nguyen Quang Le Kien @ 2026-08-04  7:44 UTC (permalink / raw)
  To: gregkh
  Cc: linux-usb, linux-kernel, syzbot+098999e05b6b877c01b3,
	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 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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] usb: gadget: f_phonet: fix use-after-free in pn_bind
  2026-08-04  7:44   ` [PATCH v2] " Nguyen Quang Le Kien
@ 2026-08-04  7:52     ` Greg KH
  2026-08-04  8:18       ` Nguyen Quang Le Kien
  2026-08-04  8:25       ` [PATCH v2] " Nguyen Quang Le Kien
  2026-08-05  4:13     ` [PATCH v3] " Nguyen Quang Le Kien
  1 sibling, 2 replies; 16+ messages in thread
From: Greg KH @ 2026-08-04  7:52 UTC (permalink / raw)
  To: Nguyen Quang Le Kien; +Cc: linux-usb, linux-kernel, syzbot+098999e05b6b877c01b3

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] usb: gadget: f_phonet: fix use-after-free in pn_bind
  2026-08-04  7:52     ` Greg KH
@ 2026-08-04  8:18       ` Nguyen Quang Le Kien
  2026-08-04  8:21         ` Greg KH
  2026-08-04  8:25       ` [PATCH v2] " Nguyen Quang Le Kien
  1 sibling, 1 reply; 16+ messages in thread
From: Nguyen Quang Le Kien @ 2026-08-04  8:18 UTC (permalink / raw)
  To: gregkh
  Cc: linux-usb, linux-kernel, syzbot+098999e05b6b877c01b3,
	Nguyen Quang Le Kien

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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] usb: gadget: f_phonet: fix use-after-free in pn_bind
  2026-08-04  8:18       ` Nguyen Quang Le Kien
@ 2026-08-04  8:21         ` Greg KH
  2026-08-05  4:08           ` Nguyen Quang Le Kien
  0 siblings, 1 reply; 16+ messages in thread
From: Greg KH @ 2026-08-04  8:21 UTC (permalink / raw)
  To: Nguyen Quang Le Kien; +Cc: linux-usb, linux-kernel, syzbot+098999e05b6b877c01b3

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] usb: gadget: f_phonet: fix use-after-free in pn_bind
  2026-08-04  7:52     ` Greg KH
  2026-08-04  8:18       ` Nguyen Quang Le Kien
@ 2026-08-04  8:25       ` Nguyen Quang Le Kien
  2026-08-04  8:25         ` syzbot
  1 sibling, 1 reply; 16+ messages in thread
From: Nguyen Quang Le Kien @ 2026-08-04  8:25 UTC (permalink / raw)
  To: syzbot+098999e05b6b877c01b3
  Cc: gregkh, linux-usb, linux-kernel, Nguyen Quang Le Kien

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] usb: gadget: f_phonet: fix use-after-free in pn_bind
  2026-08-04  8:25       ` [PATCH v2] " Nguyen Quang Le Kien
@ 2026-08-04  8:25         ` syzbot
  0 siblings, 0 replies; 16+ messages in thread
From: syzbot @ 2026-08-04  8:25 UTC (permalink / raw)
  To: khiemtranzo532001
  Cc: gregkh, khiemtranzo532001, linux-kernel, linux-usb,
	syzkaller-bugs

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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] usb: gadget: f_phonet: fix use-after-free in pn_bind
  2026-08-04  8:21         ` Greg KH
@ 2026-08-05  4:08           ` Nguyen Quang Le Kien
  2026-08-05  4:57             ` Greg KH
  0 siblings, 1 reply; 16+ messages in thread
From: Nguyen Quang Le Kien @ 2026-08-05  4:08 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, linux-kernel, Nguyen Quang Le Kien

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v3] usb: gadget: f_phonet: fix use-after-free in pn_bind
  2026-08-04  7:44   ` [PATCH v2] " Nguyen Quang Le Kien
  2026-08-04  7:52     ` Greg KH
@ 2026-08-05  4:13     ` Nguyen Quang Le Kien
  1 sibling, 0 replies; 16+ messages in thread
From: Nguyen Quang Le Kien @ 2026-08-05  4:13 UTC (permalink / raw)
  To: gregkh
  Cc: linux-usb, linux-kernel, Nguyen Quang Le Kien,
	syzbot+098999e05b6b877c01b3

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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2] usb: gadget: f_phonet: fix use-after-free in pn_bind
  2026-08-05  4:08           ` Nguyen Quang Le Kien
@ 2026-08-05  4:57             ` Greg KH
  2026-08-05  6:57               ` [PATCH v3] " Nguyen Quang Le Kien
  0 siblings, 1 reply; 16+ messages in thread
From: Greg KH @ 2026-08-05  4:57 UTC (permalink / raw)
  To: Nguyen Quang Le Kien; +Cc: linux-usb, linux-kernel

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 :(

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] usb: gadget: f_phonet: fix use-after-free in pn_bind
  2026-08-05  4:57             ` Greg KH
@ 2026-08-05  6:57               ` Nguyen Quang Le Kien
  2026-08-05  7:51                 ` Greg KH
  0 siblings, 1 reply; 16+ messages in thread
From: Nguyen Quang Le Kien @ 2026-08-05  6:57 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, linux-kernel, Nguyen Quang Le Kien

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] usb: gadget: f_phonet: fix use-after-free in pn_bind
  2026-08-05  6:57               ` [PATCH v3] " Nguyen Quang Le Kien
@ 2026-08-05  7:51                 ` Greg KH
  2026-08-05  8:24                   ` Nguyen Quang Le Kien
  0 siblings, 1 reply; 16+ messages in thread
From: Greg KH @ 2026-08-05  7:51 UTC (permalink / raw)
  To: Nguyen Quang Le Kien; +Cc: linux-usb, linux-kernel

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] usb: gadget: f_phonet: fix use-after-free in pn_bind
  2026-08-05  7:51                 ` Greg KH
@ 2026-08-05  8:24                   ` Nguyen Quang Le Kien
  2026-08-05  8:37                     ` Greg KH
  0 siblings, 1 reply; 16+ messages in thread
From: Nguyen Quang Le Kien @ 2026-08-05  8:24 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, linux-kernel, Nguyen Quang Le Kien

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] usb: gadget: f_phonet: fix use-after-free in pn_bind
  2026-08-05  8:24                   ` Nguyen Quang Le Kien
@ 2026-08-05  8:37                     ` Greg KH
  2026-08-05  9:23                       ` Nguyen Quang Le Kien
  0 siblings, 1 reply; 16+ messages in thread
From: Greg KH @ 2026-08-05  8:37 UTC (permalink / raw)
  To: Nguyen Quang Le Kien; +Cc: linux-usb, linux-kernel

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3] usb: gadget: f_phonet: fix use-after-free in pn_bind
  2026-08-05  8:37                     ` Greg KH
@ 2026-08-05  9:23                       ` Nguyen Quang Le Kien
  0 siblings, 0 replies; 16+ messages in thread
From: Nguyen Quang Le Kien @ 2026-08-05  9:23 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, linux-kernel, Nguyen Quang Le Kien

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2026-08-05  9:23 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-04  3:33 [PATCH] usb: gadget: f_phonet: fix use-after-free in pn_bind Nguyen Quang Le Kien
2026-08-04  6:30 ` Greg KH
2026-08-04  7:44   ` [PATCH v2] " Nguyen Quang Le Kien
2026-08-04  7:52     ` Greg KH
2026-08-04  8:18       ` Nguyen Quang Le Kien
2026-08-04  8:21         ` Greg KH
2026-08-05  4:08           ` Nguyen Quang Le Kien
2026-08-05  4:57             ` Greg KH
2026-08-05  6:57               ` [PATCH v3] " Nguyen Quang Le Kien
2026-08-05  7:51                 ` Greg KH
2026-08-05  8:24                   ` Nguyen Quang Le Kien
2026-08-05  8:37                     ` Greg KH
2026-08-05  9:23                       ` Nguyen Quang Le Kien
2026-08-04  8:25       ` [PATCH v2] " Nguyen Quang Le Kien
2026-08-04  8:25         ` syzbot
2026-08-05  4:13     ` [PATCH v3] " Nguyen Quang Le Kien

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox