Thread (7 messages) 7 messages, 4 authors, 2023-06-29

Re: [PATCH] usb: gadget: fsl_qe_udc: validate endpoint index for ch9 udc

From: Christophe Leroy <hidden>
Date: 2023-06-29 05:56:39
Also in: linux-usb, lkml


Le 28/06/2023 à 23:10, Leo Li a écrit :
quoted
-----Original Message-----
From: Christophe Leroy <redacted>
Sent: Wednesday, June 28, 2023 2:40 PM
To: Leo Li <redacted>; Ma Ke <redacted>
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 :
quoted
quoted
-----Original Message-----
From: Ma Ke <redacted>
Sent: Wednesday, June 28, 2023 3:15 AM
To: Leo Li <redacted>
Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; linuxppc-
dev@lists.ozlabs.org; linux-kernel@vger.kernel.org; Ma Ke
[off-list ref]
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 <redacted>
---
   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.
quoted
The only missing thing is the blank line between the declarations and the
code, so that we clearly see where declarations end and where code start.
quoted
quoted
   		struct qe_ep *target_ep = &udc->eps[pipe];
   		u16 usep;

--
2.37.2
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help