usb: typec: ucsi: displayport: Fix OOB altmode array index

PATCH Superseded Jameson Thies 4 participants 5 messages Aug 25 – Sep 2

Subsystems (3)

Stable branch USB USB Type-C class

lore

Share & tools

lore

Jameson Thies

author

2026-08-25 23:46 UTC #0 lore raw

PATCH 1/1 usb: typec: ucsi: displayport: Fix OOB altmode array index

The UCSI displayport driver indexes the connector's port altmode array with the GET_CURRENT_CAM response after checking it is not 0xff. The port altmode array is UCSI_MAX_ALTMODES elements long. If the PPM returns an invalid GET_CURRENT_CAM response above UCSI_MAX_ALTMODES and not equal to 0xff, the kernel may crash with an array index OOB error.

Update the UCSI displayport driver to verify the current cam is less than UCSI_MAX_ALTMODES before accessing the port altmode array.

Fixes af8622f6a585 ("usb: typec: ucsi: Support for DisplayPort alt mode")

Cc [email protected]

Signed-off-by Jameson Thies <[email protected]>

---
 drivers/usb/typec/ucsi/displayport.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/usb/typec/ucsi/displayport.c +1 -1

index 7067f2561b84..8d2032d0762c 100644
@@ -74,7 +74,7 @@ static int ucsi_displayport_enter(struct typec_altmode *alt, u32 *vdo)
 		cur = 0xff;
 	}
 
-	if (cur != 0xff) {
+	if (cur < UCSI_MAX_ALTMODES) {
 		ret = dp->con->port_altmode[cur] == alt ? 0 : -EBUSY;
 		goto err_unlock;
 	}

base-commit: e1e6e541c5c9cf548e9fdc35fc26808c82074440

Signature · 1 line

2.55.0.860.g4b6b3295ed-goog

Benson Leung

Reviewed-by on v1 1/1

2026-08-26 21:03 UTC #1 lore raw

Quoted 6 lines · Jameson Thies

The UCSI displayport driver indexes the connector's port altmode array with the GET_CURRENT_CAM response after checking it is not 0xff. The port altmode array is UCSI_MAX_ALTMODES elements long. If the PPM returns an invalid GET_CURRENT_CAM response above UCSI_MAX_ALTMODES and not equal to 0xff, the kernel may crash with an array index OOB error.

Quoted 6 lines

Update the UCSI displayport driver to verify the current cam is less than UCSI_MAX_ALTMODES before accessing the port altmode array.
Fixes af8622f6a585 ("usb: typec: ucsi: Support for DisplayPort alt mode")
Cc [email protected]
Signed-off-by Jameson Thies <[email protected]>

Reviewed-by Benson Leung <[email protected]>

Quoted 22 lines

---
 drivers/usb/typec/ucsi/displayport.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/usb/typec/ucsi/displayport.c +1 -1

index 7067f2561b84..8d2032d0762c 100644
@@ -74,7 +74,7 @@ static int ucsi_displayport_enter(struct typec_altmode *alt, u32 *vdo)
 		cur = 0xff;
 	}
 
-	if (cur != 0xff) {
+	if (cur < UCSI_MAX_ALTMODES) {
 		ret = dp->con->port_altmode[cur] == alt ? 0 : -EBUSY;
 		goto err_unlock;
 	}

base-commit: e1e6e541c5c9cf548e9fdc35fc26808c82074440

Signature · 1 line

2.55.0.860.g4b6b3295ed-goog

Heikki Krogerus

maintainer

2026-08-31 10:37 UTC #2 lore raw

In reply to Jameson Thies

Quoted 21 lines · Jameson Thies

The UCSI displayport driver indexes the connector's port altmode array with the GET_CURRENT_CAM response after checking it is not 0xff. The port altmode array is UCSI_MAX_ALTMODES elements long. If the PPM returns an invalid GET_CURRENT_CAM response above UCSI_MAX_ALTMODES and not equal to 0xff, the kernel may crash with an array index OOB error.
Update the UCSI displayport driver to verify the current cam is less than UCSI_MAX_ALTMODES before accessing the port altmode array.
Fixes af8622f6a585 ("usb: typec: ucsi: Support for DisplayPort alt mode")
Cc [email protected]
Signed-off-by Jameson Thies <[email protected]>
---
 drivers/usb/typec/ucsi/displayport.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/typec/ucsi/displayport.c +0 -0
index 7067f2561b84..8d2032d0762c 100644

drivers/usb/typec/ucsi/displayport.c static int ucsi_displayport_enter(struct typec_altmode *alt, u32 *vdo) · line 74

Quoted 1 line

drivers/usb/typec/ucsi/displayport.c +0 -0

@@ -74,7 +74,7 @@ static int ucsi_displayport_enter(struct typec_altmode *alt, u32 *vdo)
 		cur = 0xff;

Can you remove the above line while at it?

drivers/usb/typec/ucsi/displayport.c static int ucsi_displayport_enter(struct typec_altmode *alt, u32 *vdo) · line 77

Quoted 4 lines

drivers/usb/typec/ucsi/displayport.c +1 -1
 	}
 
-	if (cur != 0xff) {
+	if (cur < UCSI_MAX_ALTMODES) {

Shouldn't that be the other way around?

drivers/usb/typec/ucsi/displayport.c static int ucsi_displayport_enter(struct typec_altmode *alt, u32 *vdo) · line 80

Quoted 3 lines

drivers/usb/typec/ucsi/displayport.c +0 -0

 		ret = dp->con->port_altmode[cur] == alt ? 0 : -EBUSY;
 		goto err_unlock;
 	}

Thanks,

Signature · 1 line

heikki

Andrei Kuchynski

2026-08-31 11:35 UTC #3 lore raw

Quoted 27 lines · Heikki Krogerus

Quoted 21 lines · Jameson Thies
The UCSI displayport driver indexes the connector's port altmode array with the GET_CURRENT_CAM response after checking it is not 0xff. The port altmode array is UCSI_MAX_ALTMODES elements long. If the PPM returns an invalid GET_CURRENT_CAM response above UCSI_MAX_ALTMODES and not equal to 0xff, the kernel may crash with an array index OOB error.
Update the UCSI displayport driver to verify the current cam is less than UCSI_MAX_ALTMODES before accessing the port altmode array.
Fixes af8622f6a585 ("usb: typec: ucsi: Support for DisplayPort alt mode")
Cc [email protected]
Signed-off-by Jameson Thies <[email protected]>
---
 drivers/usb/typec/ucsi/displayport.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/typec/ucsi/displayport.c +0 -0
index 7067f2561b84..8d2032d0762c 100644
drivers/usb/typec/ucsi/displayport.c static int ucsi_displayport_enter(struct typec_altmode *alt, u32 *vdo) · line 74
Quoted 1 line
drivers/usb/typec/ucsi/displayport.c +0 -0
@@ -74,7 +74,7 @@ static int ucsi_displayport_enter(struct typec_altmode *alt, u32 *vdo)
              cur = 0xff;
Can you remove the above line while at it?

Quoted 7 lines

drivers/usb/typec/ucsi/displayport.c static int ucsi_displayport_enter(struct typec_altmode *alt, u32 *vdo) · line 77
Quoted 4 lines
drivers/usb/typec/ucsi/displayport.c +1 -1
      }
 
-     if (cur != 0xff) {
+     if (cur < UCSI_MAX_ALTMODES) {
Shouldn't that be the other way around?

Looks fine, but shouldn't we return an error if an invalid value is received, similar to the Thunderbolt implementation?

        if (cur != 0xff) {
                if (cur >= UCSI_MAX_ALTMODES ||
dp->con->port_altmode[cur] != alt)
                        ret = -EBUSY;
                else
                        ret = 0;
                goto err_unlock;

Thanks,
Andrei

Jameson Thies

author

2026-09-02 01:16 UTC #4 lore raw

Hi Heikki and Andrei,
thanks for taking a look. I'll follow up with a v2 of this patch to address your comments. Answers below with some additional comments.

Quoted 1 line

Quoted 1 line
Can you remove the above line while at it?

Will remove in v2.

Quoted 1 line

Quoted 1 line
Shouldn't that be the other way around?

I'm not sure I understand. Because UCSI_MAX_ALTMODES is set to 30 in ucsi.h, verifying 'cur' is less than UCSI_MAX_ALTMODES will still evaluate to false if 'cur' is 0xff. This update maintains the existing functionality and prevents the driver from using an out of bounds index.

Quoted 2 lines

Looks fine, but shouldn't we return an error if an invalid value is received, similar to the Thunderbolt implementation?

Sounds good to me, but I think we should split up the UCSI_MAX_ALTMODES check and the port_altmode check. Returning EBUSY doesn't seem like an appropriate error code if the PPM returns an out of range option. I think we should return EINVAL here.