[PATCH RESEND] usb: gadget: f_midi: fix UAF race during IN request handling

[PATCH RESEND] usb: gadget: f_midi: fix UAF race during IN request handling

[Sonali Pradhan]

f_midi_disable() frees pre-allocated IN requests from in_req_fifo
without holding midi->transmit_lock. Concurrent f_midi_transmit() or
f_midi_complete() calls can then access these request buffers after they
are freed, leading to a use-after-free.

Fix this by acquiring midi->transmit_lock in f_midi_disable() while
releasing IN requests, and checking ep->enabled under midi->transmit_lock
in f_midi_transmit() and f_midi_complete() before modifying requests.

Fixes: e1e3d7ec5da3 ("usb: gadget: f_midi: pre-allocate IN requests")
Cc: stable@vger.kernel.org
Signed-off-by: Sonali Pradhan <sonalipradhan@google.com>
---
Resending and rebasing on latest linux-next as I haven't received
feedback on my initial submission:
https://lore.kernel.org/all/20260819102709.212646-1-sonalipradhan@google.com/

 drivers/usb/gadget/function/f_midi.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
index 63fb6ee70a3d..c26d04eb9209 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -280,9 +280,18 @@ f_midi_complete(struct usb_ep *ep, struct usb_request *req)
 			/* We received stuff. req is queued again, below */
 			f_midi_handle_out_data(ep, req);
 		} else if (ep == midi->in_ep) {
+			unsigned long flags;
+
 			/* Our transmit completed. See if there's more to go.
 			 * f_midi_transmit eats req, don't queue it again. */
+			spin_lock_irqsave(&midi->transmit_lock, flags);
+			if (!ep->enabled) {
+				spin_unlock_irqrestore(&midi->transmit_lock, flags);
+				return;
+			}
 			req->length = 0;
+			spin_unlock_irqrestore(&midi->transmit_lock, flags);
+
 			queue_work(system_highpri_wq, &midi->work);
 			return;
 		}
@@ -420,6 +429,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 +441,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);
 }
@@ -678,10 +690,11 @@ static void f_midi_transmit(struct f_midi *midi)
 	unsigned long flags;
 
 	/* We only care about USB requests if IN endpoint is enabled */
-	if (!ep || !ep->enabled)
-		goto drop_out;
-
 	spin_lock_irqsave(&midi->transmit_lock, flags);
+	if (!ep || !ep->enabled) {
+		spin_unlock_irqrestore(&midi->transmit_lock, flags);
+		goto drop_out;
+	}
 
 	do {
 		ret = f_midi_do_transmit(midi, ep);
-- 
2.56.0.176.g3575e65682-goog

Re: [PATCH RESEND] usb: gadget: f_midi: fix UAF race during IN request handling

[Takashi Iwai]

On Mon, 19 Oct 2026 15:57:01 +0200,
Sonali Pradhan wrote:
> 
> f_midi_disable() frees pre-allocated IN requests from in_req_fifo
> without holding midi->transmit_lock. Concurrent f_midi_transmit() or
> f_midi_complete() calls can then access these request buffers after they
> are freed, leading to a use-after-free.
> 
> Fix this by acquiring midi->transmit_lock in f_midi_disable() while
> releasing IN requests, and checking ep->enabled under midi->transmit_lock
> in f_midi_transmit() and f_midi_complete() before modifying requests.
> 
> Fixes: e1e3d7ec5da3 ("usb: gadget: f_midi: pre-allocate IN requests")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sonali Pradhan <sonalipradhan@google.com>
> ---
> Resending and rebasing on latest linux-next as I haven't received
> feedback on my initial submission:
> https://lore.kernel.org/all/20260819102709.212646-1-sonalipradhan@google.com/
> 
>  drivers/usb/gadget/function/f_midi.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
> index 63fb6ee70a3d..c26d04eb9209 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -280,9 +280,18 @@ f_midi_complete(struct usb_ep *ep, struct usb_request *req)
>  			/* We received stuff. req is queued again, below */
>  			f_midi_handle_out_data(ep, req);
>  		} else if (ep == midi->in_ep) {
> +			unsigned long flags;
> +
>  			/* Our transmit completed. See if there's more to go.
>  			 * f_midi_transmit eats req, don't queue it again. */
> +			spin_lock_irqsave(&midi->transmit_lock, flags);
> +			if (!ep->enabled) {
> +				spin_unlock_irqrestore(&midi->transmit_lock, flags);
> +				return;
> +			}
>  			req->length = 0;
> +			spin_unlock_irqrestore(&midi->transmit_lock, flags);
> +

Does this transmit_lock help here?  As far as I see, the ep
enable/disable itself is done outside the lock.

> @@ -678,10 +690,11 @@ static void f_midi_transmit(struct f_midi *midi)
>  	unsigned long flags;
>  
>  	/* We only care about USB requests if IN endpoint is enabled */
> -	if (!ep || !ep->enabled)
> -		goto drop_out;
> -
>  	spin_lock_irqsave(&midi->transmit_lock, flags);
> +	if (!ep || !ep->enabled) {
> +		spin_unlock_irqrestore(&midi->transmit_lock, flags);
> +		goto drop_out;
> +	}

Ditto.


thanks,

Takashi

Re: [PATCH RESEND] usb: gadget: f_midi: fix UAF race during IN request handling

[Sonali Pradhan]

On Tue, 20 Oct 2026 11:06:57 +0200,
Takashi Iwai wrote:
>
> On Mon, 19 Oct 2026 15:57:01 +0200,
> Sonali Pradhan wrote:
> >
> > diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
> > index 63fb6ee70a3d..c26d04eb9209 100644
> > --- a/drivers/usb/gadget/function/f_midi.c
> > +++ b/drivers/usb/gadget/function/f_midi.c
> > @@ -280,9 +280,18 @@ f_midi_complete(struct usb_ep *ep, struct usb_request *req)
> >                     /* We received stuff. req is queued again, below */
> >                     f_midi_handle_out_data(ep, req);
> >             } else if (ep == midi->in_ep) {
> > +                   unsigned long flags;
> > +
> >                     /* Our transmit completed. See if there's more to go.
> >                      * f_midi_transmit eats req, don't queue it again. */
> > +                   spin_lock_irqsave(&midi->transmit_lock, flags);
> > +                   if (!ep->enabled) {
> > +                           spin_unlock_irqrestore(&midi->transmit_lock, flags);
> > +                           return;
> > +                   }
> >                     req->length = 0;
> > +                   spin_unlock_irqrestore(&midi->transmit_lock, flags);
> > +
>
> Does this transmit_lock help here?  As far as I see, the ep
> enable/disable itself is done outside the lock.
>
> > @@ -678,10 +690,11 @@ static void f_midi_transmit(struct f_midi *midi)
> >     unsigned long flags;
> >
> >     /* We only care about USB requests if IN endpoint is enabled */
> > -   if (!ep || !ep->enabled)
> > -           goto drop_out;
> > -
> >     spin_lock_irqsave(&midi->transmit_lock, flags);
> > +   if (!ep || !ep->enabled) {
> > +           spin_unlock_irqrestore(&midi->transmit_lock, flags);
> > +           goto drop_out;
> > +   }
>
> Ditto.

You are right about this. I'll rework the patch along with the issues
noted in Sashiko's review and send a v2.

Thanks,
Sonali