On Tue, Oct 03, 2023 at 03:11:03PM -0700, linux-dev@xxxxxxxxxxxx wrote:
> From: Dean Anderson <linux-dev@xxxxxxxxxxxx>
>
> Fixes f_fs.c handling USB_RECIP_ENDPOINT request types incorrectly for
> endpoints not belonging to it. f_fs.c needs to distinguish between IN
> and OUT endpoints, not just the endpoint number. Otherwise, f_fs may
> handle endpoints belonging to other functions. This occurs in the
> gadget/composite.c composite_setup function in the req_match callback.
>
> Signed-off-by: Dean Anderson <linux-dev@xxxxxxxxxxxx>
What commit id does this fix?
> ---
> drivers/usb/gadget/function/f_fs.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
> index c727cb5de871..fb95ca4aa025 100644
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -71,7 +71,7 @@ struct ffs_function {
> struct ffs_data *ffs;
>
> struct ffs_ep *eps;
> - u8 eps_revmap[16];
> + u8 eps_revmap[32]; /* 16 in endpoints, 16 out endpoints*/
Can't this be two separate arrays so you don't have to do fun masks to
pick out which is an in and which is an out?
> short *interfaces_nums;
>
> struct usb_function function;
> @@ -2847,6 +2847,7 @@ static int __ffs_func_bind_do_descs(enum ffs_entity_type type, u8 *valuep,
> struct usb_ep *ep;
> u8 bEndpointAddress;
> u16 wMaxPacketSize;
> + u8 addr;
>
> /*
> * We back up bEndpointAddress because autoconfig overwrites
> @@ -2870,8 +2871,9 @@ static int __ffs_func_bind_do_descs(enum ffs_entity_type type, u8 *valuep,
>
> ffs_ep->ep = ep;
> ffs_ep->req = req;
> - func->eps_revmap[ds->bEndpointAddress &
> - USB_ENDPOINT_NUMBER_MASK] = idx + 1;
> + addr = ((ds->bEndpointAddress & USB_ENDPOINT_DIR_MASK) >> 3)
> + | (ds->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK);
> + func->eps_revmap[addr] = idx + 1;
> /*
> * If we use virtual address mapping, we restore
> * original bEndpointAddress value.
> @@ -3403,7 +3405,9 @@ static void ffs_func_resume(struct usb_function *f)
>
> static int ffs_func_revmap_ep(struct ffs_function *func, u8 num)
> {
> - num = func->eps_revmap[num & USB_ENDPOINT_NUMBER_MASK];
> + u8 addr = ((num & USB_ENDPOINT_DIR_MASK) >> 3)
> + | (num & USB_ENDPOINT_NUMBER_MASK);
> + num = func->eps_revmap[addr];
That's messy, again, 2 arrays would make this much simpler I think?
thanks,
greg k-h
