Message ID | 20230628081511.186850-1-make_ruc2021@163.com |
---|---|
State | New |
Headers | show |
Series | usb: gadget: fsl_qe_udc: validate endpoint index for ch9 udc | expand |
> -----Original Message----- > From: gregkh@linuxfoundation.org <gregkh@linuxfoundation.org> > Sent: Thursday, June 29, 2023 3:41 AM > To: Christophe Leroy <christophe.leroy@csgroup.eu> > Cc: Leo Li <leoyang.li@nxp.com>; Ma Ke <make_ruc2021@163.com>; linux- > usb@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; linux- > kernel@vger.kernel.org > Subject: Re: [PATCH] usb: gadget: fsl_qe_udc: validate endpoint index for > ch9 udc > > On Thu, Jun 29, 2023 at 05:56:30AM +0000, Christophe Leroy wrote: > > > > > > Le 28/06/2023 à 23:10, Leo Li a écrit : > > > > > > > > >> -----Original Message----- > > >> From: Christophe Leroy <christophe.leroy@csgroup.eu> > > >> Sent: Wednesday, June 28, 2023 2:40 PM > > >> To: Leo Li <leoyang.li@nxp.com>; Ma Ke <make_ruc2021@163.com> > > >> Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; > > >> linuxppc- dev@lists.ozlabs.org; linux-kernel@vger.kernel.org > > >> Subject: Re: [PATCH] usb: gadget: fsl_qe_udc: validate endpoint > > >> index for > > >> ch9 udc > > >> > > >> > > >> > > >> Le 28/06/2023 à 19:04, Leo Li a écrit : > > >>> > > >>> > > >>>> -----Original Message----- > > >>>> From: Ma Ke <make_ruc2021@163.com> > > >>>> Sent: Wednesday, June 28, 2023 3:15 AM > > >>>> To: Leo Li <leoyang.li@nxp.com> > > >>>> Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; > > >>>> linuxppc- dev@lists.ozlabs.org; linux-kernel@vger.kernel.org; Ma > > >>>> Ke <make_ruc2021@163.com> > > >>>> Subject: [PATCH] usb: gadget: fsl_qe_udc: validate endpoint index > > >>>> for > > >>>> ch9 udc > > >>>> > > >>>> We should verify the bound of the array to assure that host may > > >>>> not manipulate the index to point past endpoint array. > > >>>> > > >>>> Signed-off-by: Ma Ke <make_ruc2021@163.com> > > >>>> --- > > >>>> drivers/usb/gadget/udc/fsl_qe_udc.c | 2 ++ > > >>>> 1 file changed, 2 insertions(+) > > >>>> > > >>>> diff --git a/drivers/usb/gadget/udc/fsl_qe_udc.c > > >>>> b/drivers/usb/gadget/udc/fsl_qe_udc.c > > >>>> index 3b1cc8fa30c8..f4e5cbd193b7 100644 > > >>>> --- a/drivers/usb/gadget/udc/fsl_qe_udc.c > > >>>> +++ b/drivers/usb/gadget/udc/fsl_qe_udc.c > > >>>> @@ -1959,6 +1959,8 @@ static void ch9getstatus(struct qe_udc > > >>>> *udc, u8 request_type, u16 value, > > >>>> } else if ((request_type & USB_RECIP_MASK) == > > >>>> USB_RECIP_ENDPOINT) { > > >>>> /* Get endpoint status */ > > >>>> int pipe = index & USB_ENDPOINT_NUMBER_MASK; > > >>>> + if (pipe >= USB_MAX_ENDPOINTS) > > >>>> + goto stall; > > >>> > > >>> Thanks. This seems to be the right thing to do. But normally we > > >>> don't mix > > >> declarations with code within a code block. Could we re-arrange > > >> the code a little bit so declarations stay on top? > > >> > > >> But we are at the start of a code block aren't we ? > > > > > > But they were at the beginning of a { } block which is compliant with the > C89 standard. I know gcc is more relaxed from this. But it is probably still > good to stick to the standard? > > > > Sorry I misread the patch and failed to see that the declaration block > > was continuing after the change. > > > > So yes don't interleave code with declarations. Leave declaration at > > the top of a block with a blank line between declarations and code. > > This is fine as-is, no need to change anything. With the approval from Greg, I have no objection to the patch. Acked-by: Li Yang <leoyang.li@nxp.com> Regards, Leo
diff --git a/drivers/usb/gadget/udc/fsl_qe_udc.c b/drivers/usb/gadget/udc/fsl_qe_udc.c index 3b1cc8fa30c8..f4e5cbd193b7 100644 --- a/drivers/usb/gadget/udc/fsl_qe_udc.c +++ b/drivers/usb/gadget/udc/fsl_qe_udc.c @@ -1959,6 +1959,8 @@ static void ch9getstatus(struct qe_udc *udc, u8 request_type, u16 value, } else if ((request_type & USB_RECIP_MASK) == USB_RECIP_ENDPOINT) { /* Get endpoint status */ int pipe = index & USB_ENDPOINT_NUMBER_MASK; + if (pipe >= USB_MAX_ENDPOINTS) + goto stall; struct qe_ep *target_ep = &udc->eps[pipe]; u16 usep;
We should verify the bound of the array to assure that host may not manipulate the index to point past endpoint array. Signed-off-by: Ma Ke <make_ruc2021@163.com> --- drivers/usb/gadget/udc/fsl_qe_udc.c | 2 ++ 1 file changed, 2 insertions(+)