Re: [PATCH v2 4/5] USB: gadget: f_fs: add SuperSpeed Plus support
From: Greg Kroah-Hartman
Date: Thu Dec 10 2020 - 10:13:02 EST
On Tue, Dec 01, 2020 at 02:32:18AM +0000, Peter Chen wrote:
> On 20-11-30 01:13:00, Jack Pham wrote:
> > On Fri, Nov 27, 2020 at 03:05:58PM +0100, Greg Kroah-Hartman wrote:
> > > From: "taehyun.cho" <taehyun.cho@xxxxxxxxxxx>
> > >
> > > Setup the descriptors for SuperSpeed Plus for f_fs. This allows the
> > > gadget to work properly without crashing at SuperSpeed rates.
> > >
> > > Cc: Felipe Balbi <balbi@xxxxxxxxxx>
> > > Cc: stable <stable@xxxxxxxxxxxxxxx>
> > > Signed-off-by: taehyun.cho <taehyun.cho@xxxxxxxxxxx>
> > > Signed-off-by: Will McVicker <willmcvicker@xxxxxxxxxx>
> > > Reviewed-by: Peter Chen <peter.chen@xxxxxxx>
> > > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > > ---
> > > drivers/usb/gadget/function/f_fs.c | 5 +++++
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
> > > index 046f770a76da..a34a7c96a1ab 100644
> > > --- a/drivers/usb/gadget/function/f_fs.c
> > > +++ b/drivers/usb/gadget/function/f_fs.c
> > > @@ -1327,6 +1327,7 @@ static long ffs_epfile_ioctl(struct file *file, unsigned code,
> > > struct usb_endpoint_descriptor *desc;
> > >
> > > switch (epfile->ffs->gadget->speed) {
> > > + case USB_SPEED_SUPER_PLUS:
> > > case USB_SPEED_SUPER:
> > > desc_idx = 2;
> > > break;
> > > @@ -3222,6 +3223,10 @@ static int _ffs_func_bind(struct usb_configuration *c,
> > > func->function.os_desc_n =
> > > c->cdev->use_os_string ? ffs->interfaces_count : 0;
> > >
> > > + if (likely(super)) {
> > > + func->function.ssp_descriptors =
> > > + usb_copy_descriptors(func->function.ss_descriptors);
> > > + }
> > > /* And we're done */
> > > ffs_event_add(ffs, FUNCTIONFS_BIND);
> > > return 0;
> > > --
> >
> > Hi Greg,
> >
> > FWIW I had sent a very similar patch[1] a while back (twice in fact)
> > but got no response about it. Looks like Taehyun's patch already went
> > through Google for this, I assume it must be working on their Android
> > kernels so I've no problem with you or Felipe taking this instead.
> >
> > Only one difference with my patch though is mine additionally clears the
> > func->function.ssp_descriptors member to NULL upon ffs_func_unbind() as
> > otherwise it could lead to a dangling reference in case the function is
> > re-bound and userspace does not issue SS descriptors the next time.
> > Realistically I don't think that's possible, except maybe when fuzzing?
> >
>
> Yours is better, since there is no judgement for
> func->function.ssp_descriptors at __ffs_func_bind_do_descs, without
> clearing its value can't cause problem.
Ok, I've taken the older patch instead, thanks!
greg k-h