Re: [PATCH] usb: gadget: udc-xilinx: validate ep number before indexing rambase[]

Sergio Pérez
2025-06-30T20:23:38Z

> On Fri, Jun 27, 2025 at 12:01:22AM -0600, Sergio Perez Gonzalez wrote:
> > Issue flagged by coverity. The size of the rambase array is 8,
> > usb_enpoint_num() can return 0 to 15, prevent out of bounds reads.
>
> But how can that happen with this hardware? As the array states, this
> hardware only has that many endpoints availble to it, so how can it ever
> be larger?
>

Hardware will likely behave and not report more endpoints than it
supports, but I thought that there is still a possibility that this
can be exploited, taking into account this patch:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7f14c7227f342d9932f9b918893c8814f86d2a0d

and this CVE:
https://www.cvedetails.com/cve/CVE-2022-27223/

However, looking more closely the above patch, the endpoint number is
extracted from a struct different than the "usb_endpoint_descriptor":
"epnum = udc->setup.wIndex & USB_ENDPOINT_NUMBER_MASK;"
in contrast with the code that I'm touching. The CVE does not add more
details to understand if the part of the code that I'm changing is not
subject to the vulnerability.

> > Link: https://scan7.scan.coverity.com/#/project-view/53936/11354?selectedIssue=1644635
> > Signed-off-by: Sergio Perez Gonzalez <sperezglz@gmail.com>
>
> What commit id does this fix?

The last commit that touches this code is : fd2f928a5f7bc2f9588 ("usb:
gadget: udc-xilinx: Use USB API functions rather than constants") ,
although, I think the previous code gives functionally the same
behavior.

>
>
> > ---
> > drivers/usb/gadget/udc/udc-xilinx.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/usb/gadget/udc/udc-xilinx.c b/drivers/usb/gadget/udc/udc-xilinx.c
> > index 8d803a612bb1..0c3714de2e3b 100644
> > --- a/drivers/usb/gadget/udc/udc-xilinx.c
> > +++ b/drivers/usb/gadget/udc/udc-xilinx.c
> > @@ -814,6 +814,12 @@ static int __xudc_ep_enable(struct xusb_ep *ep,
> > ep->is_in = ((desc->bEndpointAddress & USB_DIR_IN) != 0);
> > /* Bit 3...0:endpoint number */
> > ep->epnumber = usb_endpoint_num(desc);
> > + if (ep->epnumber >= XUSB_MAX_ENDPOINTS) {
> > + dev_dbg(udc->dev, "bad endpoint index %d: only 0 to %d supported\n",
> > + ep->epnumber, (XUSB_MAX_ENDPOINTS - 1));
> > + return -EINVAL;
>
> Any hints as to how this was tested?

I don't have access to such xilinx hardware, given that it was marked
as a high severity defect in coverity and it is basically extending a
validation that was already added in other parts of the code, I
decided to propose the patch without runtime testing.

Thanks,
Sergio
>
> thanks,
>
> greg k-h