Skip to contentSkip to conversation

i2c: cp2615: fix serial string NULL-deref at probe

PATCH Applied Johan Hovold 3 participants 4 messages Mar 9 – Mar 19

Subsystems (3)
Stable branch I2C subsystem host drivers CP2615 I2C driver
lore
Share & tools
lore

Johan Hovold

author
#0 lore raw

PATCH 1/1 i2c: cp2615: fix serial string NULL-deref at probe

The cp2615 driver uses the USB device serial string as the i2c adapter name but does not make sure that the string exists.

Verify that the device has a serial number before accessing it to avoid triggering a NULL-pointer dereference (e.g. with malicious devices).

Fixes 4a7695429ead ("i2c: cp2615: add i2c driver for Silicon Labs' CP2615 Digital Audio Bridge")
Cc [email protected] # 5.13
Cc Bence Csókás <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/i2c/busses/i2c-cp2615.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/i2c/busses/i2c-cp2615.c +3 -0
index c1dbf7961a02..951de6249834 100644
@@ -297,6 +297,9 @@ cp2615_i2c_probe(struct usb_interface *usbif, const struct usb_device_id *id)
 	if (!adap)
 		return -ENOMEM;
 
+	if (!usbdev->serial)
+		return -EINVAL;
+
 	strscpy(adap->name, usbdev->serial, sizeof(adap->name));
 	adap->owner = THIS_MODULE;
 	adap->dev.parent = &usbif->dev;
Signature · 1 line
2.52.0
maintainer Reviewed-by on v1 1/1
#1 lore raw
Reviewed-by Bence Csókás <[email protected]>
Quoted 19 lines · Johan Hovold

The cp2615 driver uses the USB device serial string as the i2c adapter name but does not make sure that the string exists.

Verify that the device has a serial number before accessing it to avoid triggering a NULL-pointer dereference (e.g. with malicious devices).

Fixes 4a7695429ead ("i2c: cp2615: add i2c driver for Silicon Labs' CP2615 Digital Audio Bridge")
Cc [email protected] # 5.13
Cc Bence Csókás <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
  drivers/i2c/busses/i2c-cp2615.c | 3 +++
  1 file changed, 3 insertions(+)
drivers/i2c/busses/i2c-cp2615.c +0 -0
index c1dbf7961a02..951de6249834 100644

drivers/i2c/busses/i2c-cp2615.c cp2615_i2c_probe(struct usb_interface *usbif, const struct usb_device_id *id) · line 305

Quoted 9 lines
drivers/i2c/busses/i2c-cp2615.c +3 -0
@@ -297,6 +297,9 @@ cp2615_i2c_probe(struct usb_interface *usbif, const struct usb_device_id *id)
  	if (!adap)
  		return -ENOMEM;
 
+	if (!usbdev->serial)
+		return -EINVAL;
+
  	strscpy(adap->name, usbdev->serial, sizeof(adap->name));
  	adap->owner = THIS_MODULE;
  	adap->dev.parent = &usbif->dev;

I didn't realize at the time I wrote this that serial can be NULL, I was under the impression that the USB core would pass me an empty string if there's no iSerial string descriptor (alas, it does not). AFAIK real CP2615s will always have a serial, so returning error should not be a major problem. However, we could just as easily skip strscpy() and go on with the probe with an empty name. But I'm fine with either solution.

Bence

author
#2 lore raw
Quoted 13 lines · Bence Csókás
Reviewed-by Bence Csókás <[email protected]>
Quoted 10 lines · Johan Hovold

The cp2615 driver uses the USB device serial string as the i2c adapter name but does not make sure that the string exists.

Verify that the device has a serial number before accessing it to avoid triggering a NULL-pointer dereference (e.g. with malicious devices).

drivers/i2c/busses/i2c-cp2615.c +0 -0
@@ -297,6 +297,9 @@ cp2615_i2c_probe(struct usb_interface *usbif, const struct usb_device_id *id)
  	if (!adap)
  		return -ENOMEM;
Quoted 9 lines

drivers/i2c/busses/i2c-cp2615.c cp2615_i2c_probe(struct usb_interface *usbif, const struct usb_device_id *id) · line 305

Quoted 6 lines
drivers/i2c/busses/i2c-cp2615.c +3 -0
+	if (!usbdev->serial)
+		return -EINVAL;
+
  	strscpy(adap->name, usbdev->serial, sizeof(adap->name));
  	adap->owner = THIS_MODULE;
  	adap->dev.parent = &usbif->dev;

AFAIK real CP2615s will always have a serial, so returning error should not be a major problem.

That's my reasoning as well. In the unlikely event that there'll ever be valid firmware without a serial string we can amend the driver.

Johan

maintainer Applied
#3 lore raw
In reply to Johan Hovold

Hi Johan,

Quoted 10 lines · Johan Hovold

The cp2615 driver uses the USB device serial string as the i2c adapter name but does not make sure that the string exists.

Verify that the device has a serial number before accessing it to avoid triggering a NULL-pointer dereference (e.g. with malicious devices).

Fixes 4a7695429ead ("i2c: cp2615: add i2c driver for Silicon Labs' CP2615 Digital Audio Bridge")
Cc [email protected] # 5.13
Cc Bence Csókás <[email protected]>
Signed-off-by Johan Hovold <[email protected]>

merged to i2c/i2c-host-fixes.

Thanks,
Andi