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

Sonali Pradhan <[email protected]>
Newsgroups org.kernel.vger.linux-usb,org.kernel.vger.linux-kernel,org.kernel.vger.stable
Message-ID <[email protected]>
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: [email protected]
Signed-off-by: Sonali Pradhan <[email protected]>
---
 drivers/usb/gadget/function/f_midi.c | 22 +++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
index fba8cf787d6c..ee64147b12f1 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -280,9 +280,20 @@ 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 +431,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 +437,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 +686,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);
--
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.