diff mbox series

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

Message ID 20240625022306.2568122-1-make24@iscas.ac.cn (mailing list archive)
State New, archived
Headers show
Series [v4] usb: gadget: aspeed_udc: validate endpoint index for ast udc | expand

Commit Message

Ma Ke June 25, 2024, 2:23 a.m. UTC
We should verify the bound of the array to assure that host
may not manipulate the index to point past endpoint array.

Found by static analysis.

Signed-off-by: Ma Ke <make24@iscas.ac.cn>
---
Changes in v4:
- used a consistent email address to send patches, sorry for my negligence.
Changes in v3:
- added the changelog as suggested.
Changes in v2:
- used the correct macro-defined constants as suggested;
- explained the method for finding and testing vulnerabilities.
---
 drivers/usb/gadget/udc/aspeed_udc.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Markus Elfring June 25, 2024, noon UTC | #1
> We should verify the bound of the array to assure that host
> may not manipulate the index to point past endpoint array.

Why did you not choose an imperative wording for your change description?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.10-rc5#n94


> Found by static analysis.

Were any special tools involved?


How do you think about to add any tags (like “Fixes” and “Cc”) accordingly?

Regards,
Markus
Greg KH June 25, 2024, 12:29 p.m. UTC | #2
On Tue, Jun 25, 2024 at 02:00:15PM +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.
> 
> Why did you not choose an imperative wording for your change description?
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.10-rc5#n94
> 
> 
> > Found by static analysis.
> 
> Were any special tools involved?
> 
> 
> How do you think about to add any tags (like “Fixes” and “Cc”) accordingly?


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
Greg KH June 25, 2024, 12:30 p.m. UTC | #3
On Tue, Jun 25, 2024 at 02:00:15PM +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.
> 
> Why did you not choose an imperative wording for your change description?
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.10-rc5#n94

Markus, please stop reviewing USB patches.  This is not helpful at all,
and causes new developers extra work for no reason at all.

You have been warned many times about this, and many people have talked
to you about this.  If you continue, you will have to be banned the
mailing lists, again.

greg k-h
Markus Elfring June 25, 2024, 12:50 p.m. UTC | #4
>>> We should verify the bound of the array to assure that host
>>> may not manipulate the index to point past endpoint array.
>>
>> Why did you not choose an imperative wording for your change description?
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.10-rc5#n94
>
> Markus, please stop reviewing USB patches.  This is not helpful at all,
> and causes new developers extra work for no reason at all.

How does this feedback fit to the linked information source?

Regards,
Markus
Greg KH June 25, 2024, 2:30 p.m. UTC | #5
On Tue, Jun 25, 2024 at 02:50: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.
> >>
> >> Why did you not choose an imperative wording for your change description?
> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.10-rc5#n94
> >
> > Markus, please stop reviewing USB patches.  This is not helpful at all,
> > and causes new developers extra work for no reason at all.
> 
> How does this feedback fit to the linked information source?

That is not what I wrote.

I wrote, "Please stop reviewing USB patches."

Please stop now.

greg k-h
Markus Elfring June 25, 2024, 3:20 p.m. UTC | #6
>>>>> We should verify the bound of the array to assure that host
>>>>> may not manipulate the index to point past endpoint array.
>>>>
>>>> Why did you not choose an imperative wording for your change description?
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.10-rc5#n94
>>>
>>> Markus, please stop reviewing USB patches.  This is not helpful at all,
>>> and causes new developers extra work for no reason at all.
>>
>> How does this feedback fit to the linked information source?
>
> That is not what I wrote.

You indicated concerns according to patch review processes,
didn't you?

See also:
* Patch submission notes
  https://elixir.bootlin.com/linux/v6.10-rc5/source/Documentation/process/maintainer-tip.rst#L100

* Contributor Covenant Code of Conduct
  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/code-of-conduct.rst?h=v6.10-rc5#n3


> I wrote, "Please stop reviewing USB patches."
>
> Please stop now.

I might be going to influence evolution of this software area in other ways
under other circumstances.

Regards,
Markus
Greg KH June 25, 2024, 3:25 p.m. UTC | #7
On Tue, Jun 25, 2024 at 05:20:07PM +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.
> >>>>
> >>>> Why did you not choose an imperative wording for your change description?
> >>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.10-rc5#n94
> >>>
> >>> Markus, please stop reviewing USB patches.  This is not helpful at all,
> >>> and causes new developers extra work for no reason at all.
> >>
> >> How does this feedback fit to the linked information source?
> >
> > That is not what I wrote.
> 
> You indicated concerns according to patch review processes,
> didn't you?
> 
> See also:
> * Patch submission notes
>   https://elixir.bootlin.com/linux/v6.10-rc5/source/Documentation/process/maintainer-tip.rst#L100

This is not the tip tree.

> * Contributor Covenant Code of Conduct
>   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/code-of-conduct.rst?h=v6.10-rc5#n3

I do not see how this is relevant here.

> > I wrote, "Please stop reviewing USB patches."
> >
> > Please stop now.
> 
> I might be going to influence evolution of this software area in other ways
> under other circumstances.

Please take some time and find other projects to help out.

greg k-h
Markus Elfring June 25, 2024, 4:12 p.m. UTC | #8
>> You indicated concerns according to patch review processes,
>> didn't you?
>>
>> See also:
>> * Patch submission notes
>>   https://elixir.bootlin.com/linux/v6.10-rc5/source/Documentation/process/maintainer-tip.rst#L100
>
> This is not the tip tree.

I know.

But I got the impression that some information sources
(also from the Linux development reference documentation)
can provide advices and further guidance for recurring patch review concerns.


>> I might be going to influence evolution of this software area in other ways
>> under other circumstances.
>
> Please take some time and find other projects to help out.

I found several opportunities already to improve something through the years.

Concrete example for a selected data representation:
https://patchwork.kernel.org/project/linux-usb/list/?submitter=170303&archive=both

Regards,
Markus
Andrew Jeffery June 26, 2024, 12:07 a.m. UTC | #9
On Tue, 2024-06-25 at 10:23 +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.
> 
> Found by static analysis.
> 
> Signed-off-by: Ma Ke <make24@iscas.ac.cn>

Reviewed-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Markus Elfring June 26, 2024, 8:55 a.m. UTC | #10
>> You indicated concerns according to patch review processes,
>> didn't you?
>>
>> See also:
>> * Patch submission notes
>>   https://elixir.bootlin.com/linux/v6.10-rc5/source/Documentation/process/maintainer-tip.rst#L100
>
> This is not the tip tree.

Would you eventually like to support the creation and maintenance of a document
like “Documentation/process/maintainer-usb.rst”?

Regards,
Markus
diff mbox series

Patch

diff --git a/drivers/usb/gadget/udc/aspeed_udc.c b/drivers/usb/gadget/udc/aspeed_udc.c
index 3916c8e2ba01..d972ef4644bc 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 >= AST_UDC_NUM_ENDPOINTS)
+			goto stall;
 		status = udc->ep[epnum].stopped;
 		break;
 	default: