Commit 9acdf4df authored by Felipe F. Tonello's avatar Felipe F. Tonello Committed by Felipe Balbi

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@vger.kernel.org> # v4.5+
Fixes: e1e3d7ec ("usb: gadget: f_midi: pre-allocate IN requests")
Signed-off-by: default avatarFelipe F. Tonello <eu@felipetonello.com>
Signed-off-by: default avatarFelipe Balbi <balbi@kernel.org>
parent f55532a0
...@@ -24,6 +24,7 @@ ...@@ -24,6 +24,7 @@
#include <linux/slab.h> #include <linux/slab.h>
#include <linux/device.h> #include <linux/device.h>
#include <linux/kfifo.h> #include <linux/kfifo.h>
#include <linux/spinlock.h>
#include <sound/core.h> #include <sound/core.h>
#include <sound/initval.h> #include <sound/initval.h>
...@@ -89,6 +90,7 @@ struct f_midi { ...@@ -89,6 +90,7 @@ struct f_midi {
unsigned int buflen, qlen; unsigned int buflen, qlen;
/* This fifo is used as a buffer ring for pre-allocated IN usb_requests */ /* This fifo is used as a buffer ring for pre-allocated IN usb_requests */
DECLARE_KFIFO_PTR(in_req_fifo, struct usb_request *); DECLARE_KFIFO_PTR(in_req_fifo, struct usb_request *);
spinlock_t transmit_lock;
unsigned int in_last_port; unsigned int in_last_port;
struct gmidi_in_port in_ports_array[/* in_ports */]; struct gmidi_in_port in_ports_array[/* in_ports */];
...@@ -597,17 +599,22 @@ static void f_midi_transmit(struct f_midi *midi) ...@@ -597,17 +599,22 @@ static void f_midi_transmit(struct f_midi *midi)
{ {
struct usb_ep *ep = midi->in_ep; struct usb_ep *ep = midi->in_ep;
int ret; int ret;
unsigned long flags;
/* We only care about USB requests if IN endpoint is enabled */ /* We only care about USB requests if IN endpoint is enabled */
if (!ep || !ep->enabled) if (!ep || !ep->enabled)
goto drop_out; goto drop_out;
spin_lock_irqsave(&midi->transmit_lock, flags);
do { do {
ret = f_midi_do_transmit(midi, ep); ret = f_midi_do_transmit(midi, ep);
if (ret < 0) if (ret < 0)
goto drop_out; goto drop_out;
} while (ret); } while (ret);
spin_unlock_irqrestore(&midi->transmit_lock, flags);
return; return;
drop_out: drop_out:
...@@ -1201,6 +1208,8 @@ static struct usb_function *f_midi_alloc(struct usb_function_instance *fi) ...@@ -1201,6 +1208,8 @@ static struct usb_function *f_midi_alloc(struct usb_function_instance *fi)
if (status) if (status)
goto setup_fail; goto setup_fail;
spin_lock_init(&midi->transmit_lock);
++opts->refcnt; ++opts->refcnt;
mutex_unlock(&opts->lock); mutex_unlock(&opts->lock);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment