Message ID | 20240625022306.2568122-1-make24@iscas.ac.cn (mailing list archive) |
---|---|
State | Accepted |
Commit | ee0d382feb44ec0f445e2ad63786cd7f3f6a8199 |
Headers | show |
Series | [v4] 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. 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
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
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
>>> 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
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
>>>>> 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
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
>> 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
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>
>> 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 --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:
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(+)