[PATCH] usb: gadget: aspeed_udc: validate endpoint index for ast udc

[PATCH] usb: gadget: aspeed_udc: validate endpoint index for ast udc

[Ma Ke]

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)
+			goto stall;
 		status = udc->ep[epnum].stopped;
 		break;
 	default:
-- 
2.25.1

Re: [PATCH] usb: gadget: aspeed_udc: validate endpoint index for ast udc

[Markus Elfring]

> 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

Re: [PATCH] usb: gadget: aspeed_udc: validate endpoint index for ast udc

[Greg Kroah-Hartman]

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

Re: [PATCH] usb: gadget: aspeed_udc: validate endpoint index for ast udc

[Andrew Jeffery]

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