From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com ([134.134.136.20]:14983 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932957AbdCJLri (ORCPT ); Fri, 10 Mar 2017 06:47:38 -0500 From: Felipe Balbi To: Krzysztof Opasiak , Linux USB Cc: stable@vger.kernel.org Subject: Re: [PATCH 2/2] usb: gadget: function: f_fs: pass companion descriptor along In-Reply-To: <8e061284-c5e1-2e09-1ca4-184164de1314@samsung.com> References: <20170131130823.17237-1-felipe.balbi@linux.intel.com> <20170131130823.17237-2-felipe.balbi@linux.intel.com> <19629578-9093-7087-067d-fc2619f974f0@samsung.com> <87y3xr5ikw.fsf@linux.intel.com> <8e061284-c5e1-2e09-1ca4-184164de1314@samsung.com> Date: Fri, 10 Mar 2017 13:43:08 +0200 Message-ID: <87lgsdxsqb.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Sender: stable-owner@vger.kernel.org List-ID: --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi, Krzysztof Opasiak writes: >> Krzysztof Opasiak writes: >>> On 01/31/2017 02:08 PM, Felipe Balbi wrote: >>>> If we're dealing with SuperSpeed endpoints, we need >>>> to make sure to pass along the companion descriptor >>>> and initialize fields needed by the Gadget >>>> API. Eventually, f_fs.c should be converted to use >>>> config_ep_by_speed() like all other functions, >>>> though. >>>> >>>> Cc: >>>> Signed-off-by: Felipe Balbi >>>> --- >>>> >>>> Will be sent in a pull request during v4.11-rc >>>> >>>> drivers/usb/gadget/function/f_fs.c | 15 +++++++++++++-- >>>> 1 file changed, 13 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/f= unction/f_fs.c >>>> index 87fccf611b69..86aba2ebb3ef 100644 >>>> --- a/drivers/usb/gadget/function/f_fs.c >>>> +++ b/drivers/usb/gadget/function/f_fs.c >>>> @@ -1833,11 +1833,14 @@ static int ffs_func_eps_enable(struct ffs_func= tion *func) >>>> spin_lock_irqsave(&func->ffs->eps_lock, flags); >>>> while(count--) { >>>> struct usb_endpoint_descriptor *ds; >>>> + struct usb_ss_ep_comp_descriptor *comp_desc =3D NULL; >>>> + int needs_comp_desc =3D false; >>>> int desc_idx; >>>>=20=20 >>>> - if (ffs->gadget->speed =3D=3D USB_SPEED_SUPER) >>>> + if (ffs->gadget->speed =3D=3D USB_SPEED_SUPER) { >>>> desc_idx =3D 2; >>>> - else if (ffs->gadget->speed =3D=3D USB_SPEED_HIGH) >>>> + needs_comp_desc =3D true; >>>> + } else if (ffs->gadget->speed =3D=3D USB_SPEED_HIGH) >>>> desc_idx =3D 1; >>>> else >>>> desc_idx =3D 0; >>>> @@ -1854,6 +1857,14 @@ static int ffs_func_eps_enable(struct ffs_funct= ion *func) >>>>=20=20 >>>> ep->ep->driver_data =3D ep; >>>> ep->ep->desc =3D ds; >>>> + >>>> + comp_desc =3D (struct usb_ss_ep_comp_descriptor *)(ds + >>>> + USB_DT_ENDPOINT_SIZE); >>>> + ep->ep->maxburst =3D comp_desc->bMaxBurst + 1; >>>> + >>>> + if (needs_comp_desc) >>>> + ep->ep->comp_desc =3D comp_desc; >>>> + >>> >>> Please correct me if I'm wrong but wouldn't we read rubbish here if user >>> provided us SS ep descriptor without companion descriptor? >>=20 >> companion desc is required for SS endpoints, it's also required that >> they follow EP desc. If user doesn't write it, don't they deserve the >> errors they'll have? >>=20 > > But do we deserve to access potentially unallocated memory inside kernel > each time when some malicious application requests this?;) > > In my humble opinion user should get -EINVAL or sth like this from > write(desc, sizeof(desc)) instead of some random data in companion > descriptor. We are *already* getting random data in companion descriptor :-) But I agree that -EINVAL would be really nice. Can you cook up a patch for that? =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEElLzh7wn96CXwjh2IzL64meEamQYFAljCkUwACgkQzL64meEa mQaLCRAAtUUYnjRyLjgqGz2neRZMBeGMRnrgJcj1C2WSfRH/djgay4nR2PHie76D rUb7cKVojozoiV5GOpupQVj8ss8shLyUIU0wKTulngtVCeutJ05NYT2iCzpTY+rP NuZDGvp/UjREeBohnYxl0MDWf/wGcsg2NDF1PAHNui7WsPksB0p+N799fi6QGJWH WacAnfhSjxneiI1DIvNypjMYx3eUzTu9xqbyprlKdqnzYch3ReYMxz/zAkUsJiZe 2Eq44Gn7x3z24sILuYRPsHJ4po8ONIOLbmYTVH+Ly+RUbSc4VRJ14ubfawbR+1CZ /p2WMe46QuSzfN0r1Ulp1fgkPJf5rpy8JDaDQ0DjkX2u53abzRXMPX6QsMP1pgNR W29EIOU2pr4DXypZKZThw7p1a8KCjpqvkiqoGPJ6IqadPHdEoFYYSbtDzu8UyDGP wDj7T4rIp0vw17kIRZgU61SHROlns3o6VRYJEyaETcuB0NdH1Jfu3qJCHXT6vS3E GayAXa5JRh91sDOFZTF9XcBS6xI4KRYGiEqW0wsDccO6Nm7T4DxmEHeqXRPd2ybP luFJwsxblMnWzE0DO5PkG/NVukemTR9qYNPh0vQY4EFF0+zgHGHp8TEBam678njh jlkGr8zTXp4TRpqaAXzOPwIEbry9SbT1uWCM5TkGv4Ire7QuEYQ= =xgji -----END PGP SIGNATURE----- --=-=-=--