1. Patchew
  2. linux
  3. View series

[PATCH] usb: host: max3421: Fix shift-out-of-bounds in max3421_hub_control()

pip-izony posted 1 patch 1 week, 1 day ago
There is a newer version of this series
drivers/usb/host/max3421-hcd.c | 4 ++++
1 file changed, 4 insertions(+)
[PATCH] usb: host: max3421: Fix shift-out-of-bounds in max3421_hub_control()
Posted by pip-izony 1 week, 1 day ago 
From: Seungjin Bae <eeodqql09@gmail.com>

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>
---
 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
Re: [PATCH] usb: host: max3421: Fix shift-out-of-bounds in max3421_hub_control()
Posted by Greg Kroah-Hartman 1 week, 1 day ago 
On Sat, May 16, 2026 at 08:01:46PM -0400, pip-izony wrote:
> From: Seungjin Bae <eeodqql09@gmail.com>
> 
> 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>
> ---
>  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;

Cool, what tool found this?  I've been running some static checkers and
I don't think it turned this one up yet.

thanks,

greg k-h
Re: [PATCH] usb: host: max3421: Fix shift-out-of-bounds in max3421_hub_control()
Posted by Seungjin Bae 1 week ago 
2026년 5월 17일 (일) 오전 1:49, Greg Kroah-Hartman <gregkh@linuxfoundation.org>님이 작성:
>
> On Sat, May 16, 2026 at 08:01:46PM -0400, pip-izony wrote:
> > From: Seungjin Bae <eeodqql09@gmail.com>
> >
> > 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>
> > ---
> >  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;
>
> Cool, what tool found this?  I've been running some static checkers and
> I don't think it turned this one up yet.
>
> thanks,
>
> greg k-h

Thanks for your interest!

It's a KLEE-based symbolic execution tool I've been developing for
kernel drivers. It's still a work in progress, but I'd be happy to
share more details and the tool itself once it's in better shape.

Seungjin Bae
Re: [PATCH] usb: host: max3421: Fix shift-out-of-bounds in max3421_hub_control()
Posted by Greg Kroah-Hartman 1 week ago 
On Sun, May 17, 2026 at 02:19:07PM -0400, Seungjin Bae wrote:
> 2026년 5월 17일 (일) 오전 1:49, Greg Kroah-Hartman <gregkh@linuxfoundation.org>님이 작성:
> >
> > On Sat, May 16, 2026 at 08:01:46PM -0400, pip-izony wrote:
> > > From: Seungjin Bae <eeodqql09@gmail.com>
> > >
> > > 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>
> > > ---
> > >  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;
> >
> > Cool, what tool found this?  I've been running some static checkers and
> > I don't think it turned this one up yet.
> >
> > thanks,
> >
> > greg k-h
> 
> Thanks for your interest!
> 
> It's a KLEE-based symbolic execution tool I've been developing for
> kernel drivers. It's still a work in progress, but I'd be happy to
> share more details and the tool itself once it's in better shape.

As per our documentation, you MUST document the fact that you are using
a tool to find/fix things.  Please fix up our newly submitted patches to
include that information.

thanks,

greg k-h
[PATCH v3 1/2] usb: host: max3421: Fix shift-out-of-bounds in max3421_hub_control()
Posted by pip-izony 1 week ago 
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.
 v2 -> v3: No functional changes; reorganized as part of a 2-patch series.

 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
[PATCH v4 1/2] usb: host: max3421: Fix shift-out-of-bounds in max3421_hub_control()
Posted by pip-izony 6 days, 7 hours ago 
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.

This issue was found using a KLEE-based symbolic execution tool for
kernel drivers that I'm currently developing.

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.
 v2 -> v3: No functional changes; reorganized as part of a 2-patch series.
 v3 -> v4: Document the tool used to find this issue.

 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
[PATCH v4 2/2] usb: host: max3421: Reject hub port requests for non-existent ports
Posted by pip-izony 6 days, 7 hours ago 
From: Seungjin Bae <eeodqql09@gmail.com>

The `max3421_hub_control()` function handles USB hub class requests
to the virtual root hub. The `GetPortStatus` case correctly rejects
requests with `index != 1`, since the virtual root hub has only a
single port. However, the `ClearPortFeature` and `SetPortFeature`
cases lack the same check.

Fix this by extending the `index != 1` rejection to both cases,
matching the existing behavior of `GetPortStatus`.

Fixes: 2d53139f3162 ("Add support for using a MAX3421E chip as a host driver.")
Suggested-by: Alan Stern <stern@rowland.harvard.edu>
Reviewed-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Seungjin Bae <eeodqql09@gmail.com>
---
 v3: New patch in v3, suggested by Alan Stern.
 v3 -> v4: No functional changes.

 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 3d6b351dcb1a..73e76d0e6973 100644
--- a/drivers/usb/host/max3421-hcd.c
+++ b/drivers/usb/host/max3421-hcd.c
@@ -1685,6 +1685,8 @@ max3421_hub_control(struct usb_hcd *hcd, u16 type_req, u16 value, u16 index,
 	case ClearHubFeature:
 		break;
 	case ClearPortFeature:
+		if (index != 1)
+			goto error;
 		switch (value) {
 		case USB_PORT_FEAT_SUSPEND:
 			break;
@@ -1728,6 +1730,8 @@ max3421_hub_control(struct usb_hcd *hcd, u16 type_req, u16 value, u16 index,
 		break;
 
 	case SetPortFeature:
+		if (index != 1)
+			goto error;
 		switch (value) {
 		case USB_PORT_FEAT_LINK_STATE:
 		case USB_PORT_FEAT_U1_TIMEOUT:
-- 
2.43.0
[PATCH v3 2/2] usb: host: max3421: Reject hub port requests for non-existent ports
Posted by pip-izony 1 week ago 
From: Seungjin Bae <eeodqql09@gmail.com>

The `max3421_hub_control()` function handles USB hub class requests
to the virtual root hub. The `GetPortStatus` case correctly rejects
requests with `index != 1`, since the virtual root hub has only a
single port. However, the `ClearPortFeature` and `SetPortFeature`
cases lack the same check.

Fix this by extending the `index != 1` rejection to both cases,
matching the existing behavior of `GetPortStatus`.

Fixes: 2d53139f3162 ("Add support for using a MAX3421E chip as a host driver.")
Suggested-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Seungjin Bae <eeodqql09@gmail.com>
---
 v3: New patch in v3, suggested by Alan Stern.

 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 3d6b351dcb1a..73e76d0e6973 100644
--- a/drivers/usb/host/max3421-hcd.c
+++ b/drivers/usb/host/max3421-hcd.c
@@ -1685,6 +1685,8 @@ max3421_hub_control(struct usb_hcd *hcd, u16 type_req, u16 value, u16 index,
 	case ClearHubFeature:
 		break;
 	case ClearPortFeature:
+		if (index != 1)
+			goto error;
 		switch (value) {
 		case USB_PORT_FEAT_SUSPEND:
 			break;
@@ -1728,6 +1730,8 @@ max3421_hub_control(struct usb_hcd *hcd, u16 type_req, u16 value, u16 index,
 		break;
 
 	case SetPortFeature:
+		if (index != 1)
+			goto error;
 		switch (value) {
 		case USB_PORT_FEAT_LINK_STATE:
 		case USB_PORT_FEAT_U1_TIMEOUT:
-- 
2.43.0
Re: [PATCH v3 2/2] usb: host: max3421: Reject hub port requests for non-existent ports
Posted by Alan Stern 6 days, 8 hours ago 
On Sun, May 17, 2026 at 03:03:08PM -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. The `GetPortStatus` case correctly rejects
> requests with `index != 1`, since the virtual root hub has only a
> single port. However, the `ClearPortFeature` and `SetPortFeature`
> cases lack the same check.
> 
> Fix this by extending the `index != 1` rejection to both cases,
> matching the existing behavior of `GetPortStatus`.
> 
> Fixes: 2d53139f3162 ("Add support for using a MAX3421E chip as a host driver.")
> Suggested-by: Alan Stern <stern@rowland.harvard.edu>
> Signed-off-by: Seungjin Bae <eeodqql09@gmail.com>
> ---

Reviewed-by: Alan Stern <stern@rowland.harvard.edu>

Heh, this made me go back and look at dummy-hcd.c.  It's missing the 
same check in the same places!

Would you like to submit a patch to fix that driver as well?  If not, 
I'll take care of it.

Alan Stern

>  v3: New patch in v3, suggested by Alan Stern.
> 
>  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 3d6b351dcb1a..73e76d0e6973 100644
> --- a/drivers/usb/host/max3421-hcd.c
> +++ b/drivers/usb/host/max3421-hcd.c
> @@ -1685,6 +1685,8 @@ max3421_hub_control(struct usb_hcd *hcd, u16 type_req, u16 value, u16 index,
>  	case ClearHubFeature:
>  		break;
>  	case ClearPortFeature:
> +		if (index != 1)
> +			goto error;
>  		switch (value) {
>  		case USB_PORT_FEAT_SUSPEND:
>  			break;
> @@ -1728,6 +1730,8 @@ max3421_hub_control(struct usb_hcd *hcd, u16 type_req, u16 value, u16 index,
>  		break;
>  
>  	case SetPortFeature:
> +		if (index != 1)
> +			goto error;
>  		switch (value) {
>  		case USB_PORT_FEAT_LINK_STATE:
>  		case USB_PORT_FEAT_U1_TIMEOUT:
> -- 
> 2.43.0
>
Re: [PATCH v3 2/2] usb: host: max3421: Reject hub port requests for non-existent ports
Posted by Seungjin Bae 6 days, 8 hours ago 
2026년 5월 18일 (월) 오후 5:54, Alan Stern <stern@rowland.harvard.edu>님이 작성:
>
> On Sun, May 17, 2026 at 03:03:08PM -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. The `GetPortStatus` case correctly rejects
> > requests with `index != 1`, since the virtual root hub has only a
> > single port. However, the `ClearPortFeature` and `SetPortFeature`
> > cases lack the same check.
> >
> > Fix this by extending the `index != 1` rejection to both cases,
> > matching the existing behavior of `GetPortStatus`.
> >
> > Fixes: 2d53139f3162 ("Add support for using a MAX3421E chip as a host driver.")
> > Suggested-by: Alan Stern <stern@rowland.harvard.edu>
> > Signed-off-by: Seungjin Bae <eeodqql09@gmail.com>
> > ---
>
> Reviewed-by: Alan Stern <stern@rowland.harvard.edu>
>
> Heh, this made me go back and look at dummy-hcd.c.  It's missing the
> same check in the same places!
>
> Would you like to submit a patch to fix that driver as well?  If not,
> I'll take care of it.
Thanks for pointing this out.
I'll send a separate patch for that shortly.

Seungjin Bae
>
> Alan Stern
>
> >  v3: New patch in v3, suggested by Alan Stern.
> >
> >  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 3d6b351dcb1a..73e76d0e6973 100644
> > --- a/drivers/usb/host/max3421-hcd.c
> > +++ b/drivers/usb/host/max3421-hcd.c
> > @@ -1685,6 +1685,8 @@ max3421_hub_control(struct usb_hcd *hcd, u16 type_req, u16 value, u16 index,
> >       case ClearHubFeature:
> >               break;
> >       case ClearPortFeature:
> > +             if (index != 1)
> > +                     goto error;
> >               switch (value) {
> >               case USB_PORT_FEAT_SUSPEND:
> >                       break;
> > @@ -1728,6 +1730,8 @@ max3421_hub_control(struct usb_hcd *hcd, u16 type_req, u16 value, u16 index,
> >               break;
> >
> >       case SetPortFeature:
> > +             if (index != 1)
> > +                     goto error;
> >               switch (value) {
> >               case USB_PORT_FEAT_LINK_STATE:
> >               case USB_PORT_FEAT_U1_TIMEOUT:
> > --
> > 2.43.0
> >
[PATCH v2] usb: host: max3421: Fix shift-out-of-bounds in max3421_hub_control()
Posted by pip-izony 1 week, 1 day ago 
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
Re: [PATCH v2] usb: host: max3421: Fix shift-out-of-bounds in max3421_hub_control()
Posted by Alan Stern 1 week, 1 day ago 
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
Re: [PATCH v2] usb: host: max3421: Fix shift-out-of-bounds in max3421_hub_control()
Posted by Seungjin Bae 1 week ago 
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

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.