| Message ID | 20240622095618.1890093-1-make24@iscas.ac.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: gadget: aspeed_udc: validate endpoint index for ast udc | expand |
> We should verify the bound of the array to assure that host > may not manipulate the index to point past endpoint array. * Can an imperative wording be more desirable for such a change description? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.10-rc4#n94 * Will any tags (like “Fixes”) become relevant here? Regards, Markus
On Sat, Jun 22, 2024 at 05:24:25PM +0200, Markus Elfring wrote: > > We should verify the bound of the array to assure that host > > may not manipulate the index to point past endpoint array. > > * Can an imperative wording be more desirable for such a change description? > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.10-rc4#n94 > > * Will any tags (like “Fixes”) become relevant here? Hi, This is the semi-friendly patch-bot of Greg Kroah-Hartman. Markus, you seem to have sent a nonsensical or otherwise pointless review comment to a patch submission on a Linux kernel developer mailing list. I strongly suggest that you not do this anymore. Please do not bother developers who are actively working to produce patches and features with comments that, in the end, are a waste of time. Patch submitter, please ignore Markus's suggestion; you do not need to follow it at all. The person/bot/AI that sent it is being ignored by almost all Linux kernel maintainers for having a persistent pattern of behavior of producing distracting and pointless commentary, and inability to adapt to feedback. Please feel free to also ignore emails from them. thanks, greg k-h's patch email bot
On Sat, 2024-06-22 at 17:56 +0800, Ma Ke wrote: > 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 <make24@iscas.ac.cn> > --- > drivers/usb/gadget/udc/aspeed_udc.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/usb/gadget/udc/aspeed_udc.c b/drivers/usb/gadget/udc/aspeed_udc.c > index 3916c8e2ba01..95060592c231 100644 > --- a/drivers/usb/gadget/udc/aspeed_udc.c > +++ b/drivers/usb/gadget/udc/aspeed_udc.c > @@ -1009,6 +1009,8 @@ static void ast_udc_getstatus(struct ast_udc_dev *udc) > break; > case USB_RECIP_ENDPOINT: > epnum = crq.wIndex & USB_ENDPOINT_NUMBER_MASK; > + if (epnum >= USB_MAX_ENDPOINTS) Shouldn't this be `epnum >= AST_UDC_NUM_ENDPOINTS`? Further, USB_MAX_ENDPOINTS doesn't appear to be defined here? What steps did you take to test this patch? Andrew
diff --git a/drivers/usb/gadget/udc/aspeed_udc.c b/drivers/usb/gadget/udc/aspeed_udc.c index 3916c8e2ba01..95060592c231 100644 --- a/drivers/usb/gadget/udc/aspeed_udc.c +++ b/drivers/usb/gadget/udc/aspeed_udc.c @@ -1009,6 +1009,8 @@ static void ast_udc_getstatus(struct ast_udc_dev *udc) break; case USB_RECIP_ENDPOINT: epnum = crq.wIndex & USB_ENDPOINT_NUMBER_MASK; + if (epnum >= USB_MAX_ENDPOINTS) + goto stall; status = udc->ep[epnum].stopped; break; default:
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 <make24@iscas.ac.cn> --- drivers/usb/gadget/udc/aspeed_udc.c | 2 ++ 1 file changed, 2 insertions(+)