[v2,4/5] USB: gadget: f_fs: add SuperSpeed Plus support
diff mbox series

Message ID 20201127140559.381351-5-gregkh@linuxfoundation.org
State New, archived
Headers show
Series
  • USB: Gadget SuperSpeed fixes and a cleanup
Related show

Commit Message

Greg KH Nov. 27, 2020, 2:05 p.m. UTC
From: "taehyun.cho" <taehyun.cho@samsung.com>

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@kernel.org>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: taehyun.cho <taehyun.cho@samsung.com>
Signed-off-by: Will McVicker <willmcvicker@google.com>
Reviewed-by: Peter Chen <peter.chen@nxp.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/usb/gadget/function/f_fs.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Jack Pham Nov. 30, 2020, 9:13 a.m. UTC | #1
On Fri, Nov 27, 2020 at 03:05:58PM +0100, Greg Kroah-Hartman wrote:
> From: "taehyun.cho" <taehyun.cho@samsung.com>
> 
> 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@kernel.org>
> Cc: stable <stable@vger.kernel.org>
> Signed-off-by: taehyun.cho <taehyun.cho@samsung.com>
> Signed-off-by: Will McVicker <willmcvicker@google.com>
> Reviewed-by: Peter Chen <peter.chen@nxp.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  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?

[1] https://patchwork.kernel.org/project/linux-usb/patch/20201027230731.9073-1-jackp@codeaurora.org/

Jack
Greg KH Dec. 10, 2020, 3:12 p.m. UTC | #2
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@samsung.com>
> > > 
> > > 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@kernel.org>
> > > Cc: stable <stable@vger.kernel.org>
> > > Signed-off-by: taehyun.cho <taehyun.cho@samsung.com>
> > > Signed-off-by: Will McVicker <willmcvicker@google.com>
> > > Reviewed-by: Peter Chen <peter.chen@nxp.com>
> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > ---
> > >  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

Patch
diff mbox series

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;