linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] usb: gadget: max3420_udc: Fix out-of-bounds endpoint index access
@ 2025-06-29 20:13 Seungjin Bae
  2025-06-29 21:49 ` [PATCH v2 1/2] " Seungjin Bae
  2025-06-29 21:49 ` [PATCH v2 2/2] " Seungjin Bae
  0 siblings, 2 replies; 5+ messages in thread
From: Seungjin Bae @ 2025-06-29 20:13 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: pip-izony, Kyungtae Kim, Jassi Brar, Felipe Balbi, linux-usb,
	linux-kernel, stable

In the max3420_set_clear_feature() function, the endpoint index `id` can have a value from 0 to 15.
However, the udc->ep array is initialized with a maximum of 4 endpoints in max3420_eps_init().
If host sends a request with a wIndex greater than 3, the access to `udc->ep[id]` will go out-of-bounds,
leading to memory corruption or a potential kernel crash.
This bug was found by code inspection and has not been tested on hardware.

Fixes: 48ba02b2e2b1a ("usb: gadget: add udc driver for max3420")
Cc: stable@vger.kernel.org
Signed-off-by: Seungjin Bae <eeodqql09@gmail.com>
---
 drivers/usb/gadget/udc/max3420_udc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/gadget/udc/max3420_udc.c b/drivers/usb/gadget/udc/max3420_udc.c
index 7349ea774adf..e4ecc7f7f3be 100644
--- a/drivers/usb/gadget/udc/max3420_udc.c
+++ b/drivers/usb/gadget/udc/max3420_udc.c
@@ -596,6 +596,8 @@ static void max3420_set_clear_feature(struct max3420_udc *udc)
 			break;
 
 		id = udc->setup.wIndex & USB_ENDPOINT_NUMBER_MASK;
+		if (id >= MAX3420_MAX_EPS)
+			break;
 		ep = &udc->ep[id];
 
 		spin_lock_irqsave(&ep->lock, flags);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 1/2] usb: gadget: max3420_udc: Fix out-of-bounds endpoint index access
  2025-06-29 20:13 [PATCH] usb: gadget: max3420_udc: Fix out-of-bounds endpoint index access Seungjin Bae
@ 2025-06-29 21:49 ` Seungjin Bae
  2025-06-30  4:56   ` Greg Kroah-Hartman
  2025-06-29 21:49 ` [PATCH v2 2/2] " Seungjin Bae
  1 sibling, 1 reply; 5+ messages in thread
From: Seungjin Bae @ 2025-06-29 21:49 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: pip-izony, Kyungtae Kim, Jassi Brar, Felipe Balbi, linux-usb,
	linux-kernel, stable

In the max3420_set_clear_feature() function, the endpoint index `id` can have a value from 0 to 15.
However, the udc->ep array is initialized with a maximum of 4 endpoints in max3420_eps_init().
If host sends a request with a wIndex greater than 3, the access to `udc->ep[id]` will go out-of-bounds,
leading to memory corruption or a potential kernel crash.
This bug was found by code inspection and has not been tested on hardware.

Fixes: 48ba02b2e2b1a ("usb: gadget: add udc driver for max3420")
Cc: stable@vger.kernel.org
Signed-off-by: Seungjin Bae <eeodqql09@gmail.com>
---
 v1 -> v2: Added a second patch to fix an out-of-bounds bug in the max3420_getstatus() function.
 
 drivers/usb/gadget/udc/max3420_udc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/gadget/udc/max3420_udc.c b/drivers/usb/gadget/udc/max3420_udc.c
index 7349ea774adf..e4ecc7f7f3be 100644
--- a/drivers/usb/gadget/udc/max3420_udc.c
+++ b/drivers/usb/gadget/udc/max3420_udc.c
@@ -596,6 +596,8 @@ static void max3420_set_clear_feature(struct max3420_udc *udc)
 			break;
 
 		id = udc->setup.wIndex & USB_ENDPOINT_NUMBER_MASK;
+		if (id >= MAX3420_MAX_EPS)
+			break;
 		ep = &udc->ep[id];
 
 		spin_lock_irqsave(&ep->lock, flags);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 2/2] usb: gadget: max3420_udc: Fix out-of-bounds endpoint index access
  2025-06-29 20:13 [PATCH] usb: gadget: max3420_udc: Fix out-of-bounds endpoint index access Seungjin Bae
  2025-06-29 21:49 ` [PATCH v2 1/2] " Seungjin Bae
@ 2025-06-29 21:49 ` Seungjin Bae
  1 sibling, 0 replies; 5+ messages in thread
From: Seungjin Bae @ 2025-06-29 21:49 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: pip-izony, Kyungtae Kim, Jassi Brar, Felipe Balbi, linux-usb,
	linux-kernel, stable

Similar to max3420_set_clear_feature() function, the max3420_getstatus() function also fails to validate the endpoint index
from wIndex before using it to access the udc->ep array.

The udc->ep array is initialized to handle 4 endpoints, but the index derived from the `wIndex & USB_ENDPOINT_NUMBER_MASK`
can be up to 15. This can lead to an out-of-bounds access, causing memory corruption or a potential kernel crash.
This bug was found by code inspection and has not been tested on hardware.

Fixes: 48ba02b2e2b1a ("usb: gadget: add udc driver for max3420")
Cc: stable@vger.kernel.org
Signed-off-by: Seungjin Bae <eeodqql09@gmail.com>
---
 v1 -> v2: Added a second patch to fix an out-of-bounds bug in the max3420_getstatus() function.
 
 drivers/usb/gadget/udc/max3420_udc.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/max3420_udc.c b/drivers/usb/gadget/udc/max3420_udc.c
index e4ecc7f7f3be..ff6c7f9d71d8 100644
--- a/drivers/usb/gadget/udc/max3420_udc.c
+++ b/drivers/usb/gadget/udc/max3420_udc.c
@@ -536,6 +536,7 @@ static void max3420_getstatus(struct max3420_udc *udc)
 {
 	struct max3420_ep *ep;
 	u16 status = 0;
+	int id;
 
 	switch (udc->setup.bRequestType & USB_RECIP_MASK) {
 	case USB_RECIP_DEVICE:
@@ -548,7 +549,10 @@ static void max3420_getstatus(struct max3420_udc *udc)
 			goto stall;
 		break;
 	case USB_RECIP_ENDPOINT:
-		ep = &udc->ep[udc->setup.wIndex & USB_ENDPOINT_NUMBER_MASK];
+		id = udc->setup.wIndex & USB_ENDPOINT_NUMBER_MASK;
+		if (id >= MAX3420_MAX_EPS)
+			goto stall;
+		ep = &udc->ep[id];
 		if (udc->setup.wIndex & USB_DIR_IN) {
 			if (!ep->ep_usb.caps.dir_in)
 				goto stall;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 1/2] usb: gadget: max3420_udc: Fix out-of-bounds endpoint index access
  2025-06-29 21:49 ` [PATCH v2 1/2] " Seungjin Bae
@ 2025-06-30  4:56   ` Greg Kroah-Hartman
  2025-07-13 16:05     ` pip-izony
  0 siblings, 1 reply; 5+ messages in thread
From: Greg Kroah-Hartman @ 2025-06-30  4:56 UTC (permalink / raw)
  To: Seungjin Bae
  Cc: Kyungtae Kim, Jassi Brar, Felipe Balbi, linux-usb, linux-kernel,
	stable

On Sun, Jun 29, 2025 at 05:49:45PM -0400, Seungjin Bae wrote:
> In the max3420_set_clear_feature() function, the endpoint index `id` can have a value from 0 to 15.
> However, the udc->ep array is initialized with a maximum of 4 endpoints in max3420_eps_init().
> If host sends a request with a wIndex greater than 3, the access to `udc->ep[id]` will go out-of-bounds,
> leading to memory corruption or a potential kernel crash.
> This bug was found by code inspection and has not been tested on hardware.

Please wrap your lines at 72 columns.

Also, you sent 2 patches, with identical subject lines, but they did
different things.  That's not ok as you know.

And I think you really need to test this on hardware.  How could that
request ever have a windex set to greater than 3?  Is that a hardware
value or a user-controlled value?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 1/2] usb: gadget: max3420_udc: Fix out-of-bounds endpoint index access
  2025-06-30  4:56   ` Greg Kroah-Hartman
@ 2025-07-13 16:05     ` pip-izony
  0 siblings, 0 replies; 5+ messages in thread
From: pip-izony @ 2025-07-13 16:05 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Kyungtae Kim, Jassi Brar, Felipe Balbi, linux-usb, linux-kernel,
	stable

> Also, you sent 2 patches, with identical subject lines, but they did
> different things. That's not ok as you know.

My apologies for the mistake. I will separate them properly in the
next version of the patch series.

> And I think you really need to test this on hardware.  How could that
> request ever have a windex set to greater than 3?  Is that a hardware
> value or a user-controlled value?

The wIndex field of a SETUP packet is sent by the USB host and can
be controlled by a malicious or malformed host.
This same class of vulnerability was identified and fixed in other
UDC drivers, as described in CVE-2022-27223 and fixed in the xilinx
UDC driver by commit 7f14c7227f34 ("USB: gadget: validate endpoint 
index for xilinx udc").

Following this established pattern, I added the necessary bounds
check to the max3420_udc driver before wIndex is used to access
the endpoint array.

Thank you.

Seungjin Bae

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-07-13 16:06 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-29 20:13 [PATCH] usb: gadget: max3420_udc: Fix out-of-bounds endpoint index access Seungjin Bae
2025-06-29 21:49 ` [PATCH v2 1/2] " Seungjin Bae
2025-06-30  4:56   ` Greg Kroah-Hartman
2025-07-13 16:05     ` pip-izony
2025-06-29 21:49 ` [PATCH v2 2/2] " Seungjin Bae

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).