mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2] usb: gadget: f_midi: fix UAF race during IN request handling
@ 2026-10-28  7:48 Sonali Pradhan
  2026-11-03 14:47 ` Takashi Iwai
  0 siblings, 1 reply; 2+ messages in thread
From: Sonali Pradhan @ 2026-10-28  7:48 UTC (permalink / raw)
  To: nkapron, gregkh
  Cc: tiwai, balbi, jeffinphilip14, victor.krawiec, fanwu01,
	christophe.jaillet, linux-usb, linux-kernel, stable,
	sonalipradhan

f_midi_disable() drains and frees pre-allocated IN requests from
in_req_fifo without holding midi->transmit_lock. If f_midi_transmit()
runs concurrently on the workqueue, f_midi_do_transmit() can peek a
request from in_req_fifo and write into its buffer while
f_midi_disable() frees it, leading to a use-after-free.

Additionally, when usb_ep_queue() fails in f_midi_do_transmit() (such
as with -ESHUTDOWN when the endpoint is disabled concurrently), it
resets req->length = 0 and returns active instead of the error code,
causing the do-while loop in f_midi_transmit() to keep spinning under
midi->transmit_lock until the substream is drained.

Fix this by acquiring midi->transmit_lock in f_midi_disable() while
releasing IN requests from in_req_fifo, and returning the error code
from f_midi_do_transmit() when usb_ep_queue() fails so f_midi_transmit()
aborts immediately and drops active substreams.

Fixes: e1e3d7ec5da3 ("usb: gadget: f_midi: pre-allocate IN requests")
Cc: stable@vger.kernel.org
Signed-off-by: Sonali Pradhan <sonalipradhan@google.com>
---
Changes in v2:
- Remove the transmit_lock and ep->enabled checks added in v1 from
  f_midi_complete() and f_midi_transmit().
- Return error code from f_midi_do_transmit() when usb_ep_queue() fails
  so f_midi_transmit() aborts the loop immediately.
- Link to v1: https://lore.kernel.org/all/20261019135701.2767078-1-sonalipradhan@google.com/

 drivers/usb/gadget/function/f_midi.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
index 63fb6ee70a3d..48cda34399e8 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -420,6 +420,7 @@ static void f_midi_disable(struct usb_function *f)
 	struct f_midi *midi = func_to_midi(f);
 	struct usb_composite_dev *cdev = f->config->cdev;
 	struct usb_request *req = NULL;
+	unsigned long flags;
 
 	DBG(cdev, "disable\n");
 
@@ -431,8 +432,10 @@ static void f_midi_disable(struct usb_function *f)
 	usb_ep_disable(midi->out_ep);
 
 	/* release IN requests */
+	spin_lock_irqsave(&midi->transmit_lock, flags);
 	while (kfifo_get(&midi->in_req_fifo, &req))
 		free_ep_req(midi->in_ep, req);
+	spin_unlock_irqrestore(&midi->transmit_lock, flags);
 
 	f_midi_drop_out_substreams(midi);
 }
@@ -661,6 +664,7 @@ static int f_midi_do_transmit(struct f_midi *midi, struct usb_ep *ep)
 		ERROR(midi, "%s failed to queue req: %d\n",
 		      midi->in_ep->name, err);
 		req->length = 0; /* Re-use request next time. */
+		return err;
 	} else {
 		/* Upon success, put request at the back of the queue. */
 		kfifo_skip(&midi->in_req_fifo);
-- 
2.56.0.307.g5fe7bd680c-goog

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

* Re: [PATCH v2] usb: gadget: f_midi: fix UAF race during IN request handling
  2026-10-28  7:48 [PATCH v2] usb: gadget: f_midi: fix UAF race during IN request handling Sonali Pradhan
@ 2026-11-03 14:47 ` Takashi Iwai
  0 siblings, 0 replies; 2+ messages in thread
From: Takashi Iwai @ 2026-11-03 14:47 UTC (permalink / raw)
  To: Sonali Pradhan
  Cc: nkapron, gregkh, tiwai, balbi, jeffinphilip14, victor.krawiec,
	fanwu01, christophe.jaillet, linux-usb, linux-kernel, stable

On Wed, 28 Oct 2026 08:48:21 +0100,
Sonali Pradhan wrote:
> 
> f_midi_disable() drains and frees pre-allocated IN requests from
> in_req_fifo without holding midi->transmit_lock. If f_midi_transmit()
> runs concurrently on the workqueue, f_midi_do_transmit() can peek a
> request from in_req_fifo and write into its buffer while
> f_midi_disable() frees it, leading to a use-after-free.
> 
> Additionally, when usb_ep_queue() fails in f_midi_do_transmit() (such
> as with -ESHUTDOWN when the endpoint is disabled concurrently), it
> resets req->length = 0 and returns active instead of the error code,
> causing the do-while loop in f_midi_transmit() to keep spinning under
> midi->transmit_lock until the substream is drained.
> 
> Fix this by acquiring midi->transmit_lock in f_midi_disable() while
> releasing IN requests from in_req_fifo, and returning the error code
> from f_midi_do_transmit() when usb_ep_queue() fails so f_midi_transmit()
> aborts immediately and drops active substreams.
> 
> Fixes: e1e3d7ec5da3 ("usb: gadget: f_midi: pre-allocate IN requests")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sonali Pradhan <sonalipradhan@google.com>

Reviewed-by: Takashi Iwai <tiwai@suse.de>


thanks,

Takashi


> ---
> Changes in v2:
> - Remove the transmit_lock and ep->enabled checks added in v1 from
>   f_midi_complete() and f_midi_transmit().
> - Return error code from f_midi_do_transmit() when usb_ep_queue() fails
>   so f_midi_transmit() aborts the loop immediately.
> - Link to v1: https://lore.kernel.org/all/20261019135701.2767078-1-sonalipradhan@google.com/
> 
>  drivers/usb/gadget/function/f_midi.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
> index 63fb6ee70a3d..48cda34399e8 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -420,6 +420,7 @@ static void f_midi_disable(struct usb_function *f)
>  	struct f_midi *midi = func_to_midi(f);
>  	struct usb_composite_dev *cdev = f->config->cdev;
>  	struct usb_request *req = NULL;
> +	unsigned long flags;
>  
>  	DBG(cdev, "disable\n");
>  
> @@ -431,8 +432,10 @@ static void f_midi_disable(struct usb_function *f)
>  	usb_ep_disable(midi->out_ep);
>  
>  	/* release IN requests */
> +	spin_lock_irqsave(&midi->transmit_lock, flags);
>  	while (kfifo_get(&midi->in_req_fifo, &req))
>  		free_ep_req(midi->in_ep, req);
> +	spin_unlock_irqrestore(&midi->transmit_lock, flags);
>  
>  	f_midi_drop_out_substreams(midi);
>  }
> @@ -661,6 +664,7 @@ static int f_midi_do_transmit(struct f_midi *midi, struct usb_ep *ep)
>  		ERROR(midi, "%s failed to queue req: %d\n",
>  		      midi->in_ep->name, err);
>  		req->length = 0; /* Re-use request next time. */
> +		return err;
>  	} else {
>  		/* Upon success, put request at the back of the queue. */
>  		kfifo_skip(&midi->in_req_fifo);
> -- 
> 2.56.0.307.g5fe7bd680c-goog

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

end of thread, other threads:[~2026-11-03 14:47 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-28  7:48 [PATCH v2] usb: gadget: f_midi: fix UAF race during IN request handling Sonali Pradhan
2026-11-03 14:47 ` Takashi Iwai

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

all inboxes | Powered by JetHome®