From: Seungjin Bae <eeodqql09@gmail.com>

The `max3421_hub_control()` function handles USB hub class requests
to the virtual root hub. In the `default` branches of both the
`ClearPortFeature` and `SetPortFeature` switch statements, it modifies
`max3421_hcd->port_status` by left shifting 1 by the request's `value`
parameter. However, it does not validate whether this shift will exceed
the width of `port_status`.

So if a malicious userspace task with access to the root hub via
/dev/bus/usb/.../001 issues a USBDEVFS_CONTROL ioctl with `wValue`
greater than or equal to 32, the left shift operation invokes
shift-out-of-bounds undefined behavior. This results in arbitrary
bit corruption of `port_status`, including the normally-immutable
change bits, which can bypass internal state checks and confuse the
hub status.

Fix this by rejecting requests whose `value` exceeds the shift width
before performing the shift.

Fixes: 2d53139f3162 ("Add support for using a MAX3421E chip as a host driver.")
Signed-off-by: Seungjin Bae <eeodqql09@gmail.com>
---
 v1 -> v2: Restored a paragraph in the commit message that was truncated.

 drivers/usb/host/max3421-hcd.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/host/max3421-hcd.c b/drivers/usb/host/max3421-hcd.c
index 0e17c988d36a..3d6b351dcb1a 100644
--- a/drivers/usb/host/max3421-hcd.c
+++ b/drivers/usb/host/max3421-hcd.c
@@ -1694,6 +1694,8 @@ max3421_hub_control(struct usb_hcd *hcd, u16 type_req, u16 value, u16 index,
 						!pdata->vbus_active_level);
 			fallthrough;
 		default:
+			if (value >= 32)
+				goto error;
 			max3421_hcd->port_status &= ~(1 << value);
 		}
 		break;
@@ -1747,6 +1749,8 @@ max3421_hub_control(struct usb_hcd *hcd, u16 type_req, u16 value, u16 index,
 			max3421_reset_port(hcd);
 			fallthrough;
 		default:
+			if (value >= 32)
+				goto error;
 			if ((max3421_hcd->port_status & USB_PORT_STAT_POWER)
 			    != 0)
 				max3421_hcd->port_status |= (1 << value);
-- 
2.43.0

On Sat, May 16, 2026 at 08:07:31PM -0400, pip-izony wrote:
> From: Seungjin Bae <eeodqql09@gmail.com>
> 
> The `max3421_hub_control()` function handles USB hub class requests
> to the virtual root hub. In the `default` branches of both the
> `ClearPortFeature` and `SetPortFeature` switch statements, it modifies
> `max3421_hcd->port_status` by left shifting 1 by the request's `value`
> parameter. However, it does not validate whether this shift will exceed
> the width of `port_status`.
> 
> So if a malicious userspace task with access to the root hub via
> /dev/bus/usb/.../001 issues a USBDEVFS_CONTROL ioctl with `wValue`
> greater than or equal to 32, the left shift operation invokes
> shift-out-of-bounds undefined behavior. This results in arbitrary
> bit corruption of `port_status`, including the normally-immutable
> change bits, which can bypass internal state checks and confuse the
> hub status.
> 
> Fix this by rejecting requests whose `value` exceeds the shift width
> before performing the shift.

Another problem is that the root hub is supposed to reject requests to 
clear or set a feature for a non-existent port.  Just as in the 
GetPortStatus case, the ClearPortFeature and SetPortFeature cases should 
check for index != 1.

Alan Stern

2026년 5월 16일 (토) 오후 9:15, Alan Stern <stern@rowland.harvard.edu>님이 작성:
>
> On Sat, May 16, 2026 at 08:07:31PM -0400, pip-izony wrote:
> > From: Seungjin Bae <eeodqql09@gmail.com>
> >
> > The `max3421_hub_control()` function handles USB hub class requests
> > to the virtual root hub. In the `default` branches of both the
> > `ClearPortFeature` and `SetPortFeature` switch statements, it modifies
> > `max3421_hcd->port_status` by left shifting 1 by the request's `value`
> > parameter. However, it does not validate whether this shift will exceed
> > the width of `port_status`.
> >
> > So if a malicious userspace task with access to the root hub via
> > /dev/bus/usb/.../001 issues a USBDEVFS_CONTROL ioctl with `wValue`
> > greater than or equal to 32, the left shift operation invokes
> > shift-out-of-bounds undefined behavior. This results in arbitrary
> > bit corruption of `port_status`, including the normally-immutable
> > change bits, which can bypass internal state checks and confuse the
> > hub status.
> >
> > Fix this by rejecting requests whose `value` exceeds the shift width
> > before performing the shift.
>
> Another problem is that the root hub is supposed to reject requests to
> clear or set a feature for a non-existent port.  Just as in the
> GetPortStatus case, the ClearPortFeature and SetPortFeature cases should
> check for index != 1.
>
> Alan Stern

Good point.
I'll add the index != 1 check to both the ClearPortFeature and
SetPortFeature cases in v3.
Thank you for the review.

Seungjin Bae