Re: [PATCH] usb: gadget: f_midi: added spinlock on transmit function

[Felipe Ferreri Tonello <eu@felipetonello.com>]

[-- Attachment #1: Type: text/plain, Size: 1881 bytes --]

Hi Balbi,

On 09/03/16 07:20, Felipe Balbi wrote:
> 
> Hi,
> 
> "Felipe F. Tonello" <eu@felipetonello.com> writes:
>> [ text/plain ]
>> 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.4+
> 
> this should be v4.5+
> 
> $ git describe e1e3d7ec5da3
> v4.4-rc5-59-ge1e3d7ec5da3
> 
>> @@ -651,17 +653,22 @@ static void f_midi_transmit(struct f_midi *midi)
>>  {
>>  	struct usb_ep *ep = midi->in_ep;
>>  	int ret;
>> +	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);
>> +
>>  	do {
>>  		ret = f_midi_do_transmit(midi, ep);
> 
> you wrote this commit on top of 'next' right ? I know that because of
> this call to f_midi_do_transmit() here which is introduced by commit
> aac887442d5e ("usb: gadget: f_midi: move some of f_midi_transmit to
> separate func") which is not in Linus' tree yet.
> 
> This prevents me from taking this patch during current -rc.

Yes, but then what should I do? Because if I patch on Linus' tree, then
the patch won't apply to your tree.

-- 
Felipe

[-- Attachment #2: 0x92698E6A.asc --]
[-- Type: application/pgp-keys, Size: 7195 bytes --]