From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com ([134.134.136.24]:6792 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932182AbcCIKe2 (ORCPT ); Wed, 9 Mar 2016 05:34:28 -0500 From: Felipe Balbi To: Felipe Ferreri Tonello , linux-usb@vger.kernel.org Cc: "# v4 . 4+" Subject: Re: [PATCH] usb: gadget: f_midi: added spinlock on transmit function In-Reply-To: <56DFEE5D.2070609@felipetonello.com> References: <1457468507-25622-1-git-send-email-eu@felipetonello.com> <874mcgglsh.fsf@intel.com> <56DFEE5D.2070609@felipetonello.com> Date: Wed, 09 Mar 2016 12:33:36 +0200 Message-ID: <87si00ey9r.fsf@intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha1; protocol="application/pgp-signature" Sender: stable-owner@vger.kernel.org List-ID: --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi, Felipe Ferreri Tonello writes: > [ text/plain ] > Hi Balbi, > > On 09/03/16 07:20, Felipe Balbi wrote: >>=20 >> Hi, >>=20 >> "Felipe F. Tonello" 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_tr= ansmit >>> is not reentrant nor thread-safe. This is due to an implementation deta= il 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 i= t. 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: # v4.4+ >>=20 >> this should be v4.5+ >>=20 >> $ git describe e1e3d7ec5da3 >> v4.4-rc5-59-ge1e3d7ec5da3 >>=20 >>> @@ -651,17 +653,22 @@ static void f_midi_transmit(struct f_midi *midi) >>> { >>> struct usb_ep *ep =3D midi->in_ep; >>> int ret; >>> + unsigned long flags; >>>=20=20 >>> /* We only care about USB requests if IN endpoint is enabled */ >>> if (!ep || !ep->enabled) >>> goto drop_out; >>>=20=20 >>> + spin_lock_irqsave(&midi->transmit_lock, flags); >>> + >>> do { >>> ret =3D f_midi_do_transmit(midi, ep); >>=20 >> 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. >>=20 >> 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. it should apply to my "fixes" branch where fixes for current -rc should go :-) Note that v4.5-rc7 doesn't have f_midi_do_transmit() =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJW3/wAAAoJEIaOsuA1yqREHYgP/0X63sIHSTqqETn3T76U4Xh3 eaRKfl7ltCxGcSbYAHtqPkAulhWH4MZNOHpwB8vf8kX/wW5C6utcSsZ3PnJxL8HW 6+L2Q4dDg+KX1PkhUEWYhH3JVL/qPV1ZbUE/rhWxN6ncQ0cyxTKuPa0tjRAgpyYG ERto6dptIb5Fl2Yhf4PW7PXN78JgwU6k6BxqWu/fht64XE6QooLgEigM8i1ln4QX CHgZzEOexFj6yMoiMFF34driaXxL95sSYtgzMPpFTSD2uHLZiaTSdeSHeNpw+KBS PADjG872+LbGY1njvD9a7Bt4ycbxdpMqkh1xdP47ANhVvaJ15sQ8+E/IO+rOH7az qscAn+FDXzKjKLkpbR0BtllHoxhRPbdb8i60DT9QRxNdtpmFSyvhKJ6yTRBGpxdm 0sN1HnW+Mui3+BUIJag9Zvua9XC6mKHImq0PYcQKjcsTEBguy5vBIBhiKs+pxyHF v3eFSLkHRVD3n3qyjxEr11LD5tsvNMx65o4eu/nCgoQ1Mn+gPOrT8vJEZjGFkY80 34k3SVykAQZNN/IfZTQh3hXJTnEXGMMRAENgLSlRVGEq27UrRob0lVkwqaazTFTF d0UpEVtM40v5n4vWwytBR0kT24fAIJL7ikasjEdgH+Fr7Xq9VYqePp4pjdqwcp84 7KIs9QEp0yf57hT+Kmuq =El5t -----END PGP SIGNATURE----- --=-=-=--