linux-kernel - [PATCH] usb: gadget: f_midi: added spinlock on transmit function

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:	Wed,  9 Mar 2016 16:17:41 +0000
From:	"Felipe F. Tonello" <eu@...ipetonello.com>
To:	linux-usb@...r.kernel.org
Cc:	linux-kernel@...r.kernel.org, Felipe Balbi <balbi@...nel.org>,
	"# v4 . 5+" <stable@...r.kernel.org>
Subject: [PATCH] usb: gadget: f_midi: added spinlock on transmit function

Since f_midi_transmit is called by both ALSA and USB sub-systems, it can
potentially cause a race condition between both calls because f_midi_transmit
is not reentrant nor thread-safe. This is due to an implementation detail that
the transmit function looks for the next available usb request from the fifo
and only enqueues it if there is data to send, otherwise just re-uses it. So,
if both ALSA and USB frameworks calls this function at the same time,
kfifo_seek() will return the same usb_request, which will cause a race
condition.

To solve this problem a syncronization mechanism is necessary. In this case it
is used a spinlock since f_midi_transmit is also called by usb_request->complete
callback in interrupt context.

Cc: <stable@...r.kernel.org> # v4.5+
Fixes: e1e3d7ec5da3 ("usb: gadget: f_midi: pre-allocate IN requests")
Signed-off-by: Felipe F. Tonello <eu@...ipetonello.com>
---
 drivers/usb/gadget/function/f_midi.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
index fb1fe96d3215..ecb46d6abf0e 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -24,6 +24,7 @@
 #include <linux/slab.h>
 #include <linux/device.h>
 #include <linux/kfifo.h>
+#include <linux/spinlock.h>
 
 #include <sound/core.h>
 #include <sound/initval.h>
@@ -92,6 +93,7 @@ struct f_midi {
 	/* This fifo is used as a buffer ring for pre-allocated IN usb_requests */
 	DECLARE_KFIFO_PTR(in_req_fifo, struct usb_request *);
 	unsigned int in_last_port;
+	spinlock_t transmit_lock;
 };
 
 static inline struct f_midi *func_to_midi(struct usb_function *f)
@@ -535,12 +537,15 @@ static void f_midi_drop_out_substreams(struct f_midi *midi)
 static void f_midi_transmit(struct f_midi *midi)
 {
 	struct usb_ep *ep = midi->in_ep;
+	unsigned long flags;
 	bool active;
 
 	/* 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);
+
 	do {
 		struct usb_request *req = NULL;
 		unsigned int len, i;
@@ -552,14 +557,17 @@ static void f_midi_transmit(struct f_midi *midi)
 		len = kfifo_peek(&midi->in_req_fifo, &req);
 		if (len != 1) {
 			ERROR(midi, "%s: Couldn't get usb request\n", __func__);
+			spin_unlock_irqrestore(&midi->transmit_lock, flags);
 			goto drop_out;
 		}
 
 		/* If buffer overrun, then we ignore this transmission.
 		 * IMPORTANT: This will cause the user-space rawmidi device to block until a) usb
 		 * requests have been completed or b) snd_rawmidi_write() times out. */
-		if (req->length > 0)
+		if (req->length > 0) {
+			spin_unlock_irqrestore(&midi->transmit_lock, flags);
 			return;
+		}
 
 		for (i = midi->in_last_port; i < MAX_PORTS; i++) {
 			struct gmidi_in_port *port = midi->in_port[i];
@@ -611,6 +619,8 @@ static void f_midi_transmit(struct f_midi *midi)
 		}
 	} while (active);
 
+	spin_unlock_irqrestore(&midi->transmit_lock, flags);
+
 	return;
 
 drop_out:
@@ -1216,6 +1226,8 @@ static struct usb_function *f_midi_alloc(struct usb_function_instance *fi)
 	if (status)
 		goto setup_fail;
 
+	spin_lock_init(&midi->transmit_lock);
+
 	++opts->refcnt;
 	mutex_unlock(&opts->lock);
 
-- 
2.7.2