Skip to contentSkip to conversation

usb: typec: tipd: Don't read/write more bytes than required

PATCH Superseded Sven Peter 2 participants 5 messages Sep 14, 2021

Subsystems (2)
USB USB Type-C class

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Sven Peter

author
#0 lore raw

PATCH 1/2 usb: typec: tipd: Don't read/write more bytes than required

tps6598x_block_read/write always read 65 bytes of data even when much less is required when I2C_FUNC_I2C is used. Reduce this to the correct number.

Signed-off-by Sven Peter <[email protected]>
---
 drivers/usb/typec/tipd/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/usb/typec/tipd/core.c +2 -2
index 21b3ae25c76d..c18ec3785592 100644
@@ -123,7 +123,7 @@ tps6598x_block_read(struct tps6598x *tps, u8 reg, void *val, size_t len)
 	if (!tps->i2c_protocol)
 		return regmap_raw_read(tps->regmap, reg, val, len);
 
-	ret = regmap_raw_read(tps->regmap, reg, data, sizeof(data));
+	ret = regmap_raw_read(tps->regmap, reg, data, len + 1);
 	if (ret)
 		return ret;
 

@@ -145,7 +145,7 @@ static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
 	data[0] = len;
 	memcpy(&data[1], val, len);
 
-	return regmap_raw_write(tps->regmap, reg, data, sizeof(data));
+	return regmap_raw_write(tps->regmap, reg, data, len + 1);
 }
 
 static inline int tps6598x_read16(struct tps6598x *tps, u8 reg, u16 *val)
Signature · 1 line
2.25.1
author
#1 lore raw

PATCH 2/2 usb: typec: tipd: Add an additional overflow check

tps6598x_block_read already checks for the maximum length of the read but tps6598x_block_write does not. Add the symmetric check there as well.

Signed-off-by Sven Peter <[email protected]>
---
 drivers/usb/typec/tipd/core.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/usb/typec/tipd/core.c +3 -0
index c18ec3785592..70e2d0d410c9 100644
@@ -139,6 +139,9 @@ static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
 {
 	u8 data[TPS_MAX_LEN + 1];
 
+	if (WARN_ON(len + 1 > sizeof(data)))
+		return -EINVAL;
+
 	if (!tps->i2c_protocol)
 		return regmap_raw_write(tps->regmap, reg, val, len);
Signature · 1 line
2.25.1
maintainer
#2 lore raw

Re: [PATCH 2/2] usb: typec: tipd: Add an additional overflow check

Quoted 14 lines · Sven Peter

tps6598x_block_read already checks for the maximum length of the read but tps6598x_block_write does not. Add the symmetric check there as well.

Signed-off-by Sven Peter <[email protected]>
---
 drivers/usb/typec/tipd/core.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/usb/typec/tipd/core.c +0 -0
index c18ec3785592..70e2d0d410c9 100644

drivers/usb/typec/tipd/core.c static int tps6598x_block_write(struct tps6598x *tps, u8 reg, · line 143

Quoted 5 lines
drivers/usb/typec/tipd/core.c +2 -0
@@ -139,6 +139,9 @@ static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
 {
 	u8 data[TPS_MAX_LEN + 1];
 
+	if (WARN_ON(len + 1 > sizeof(data)))
+		return -EINVAL;

No need to crash anything. If this is a valid thing for us to check, let's check it and handle the error, but we should not reboot systems that are running with panic-on-warn enabled, right?

thanks,

greg k-h

author
#3 lore raw

Re: [PATCH 2/2] usb: typec: tipd: Add an additional overflow check

Quoted 18 lines · Greg Kroah-Hartman
Quoted 17 lines · Sven Peter

tps6598x_block_read already checks for the maximum length of the read but tps6598x_block_write does not. Add the symmetric check there as well.

Signed-off-by Sven Peter <[email protected]>
---
 drivers/usb/typec/tipd/core.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/usb/typec/tipd/core.c +0 -0
index c18ec3785592..70e2d0d410c9 100644
@@ -139,6 +139,9 @@ static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
 {
 	u8 data[TPS_MAX_LEN + 1];
Quoted 6 lines

drivers/usb/typec/tipd/core.c static int tps6598x_block_write(struct tps6598x *tps, u8 reg, · line 143

Quoted 2 lines
drivers/usb/typec/tipd/core.c +2 -0
+	if (WARN_ON(len + 1 > sizeof(data)))
+		return -EINVAL;

No need to crash anything. If this is a valid thing for us to check, let's check it and handle the error, but we should not reboot systems that are running with panic-on-warn enabled, right?

Sure, that makes sense. I guess the same point applies to the WARN_ON in the same check in tps6598x_block_read. I can add a patch to remove that one to v2 as well.

thanks,

Sven

maintainer
#4 lore raw

Re: [PATCH 2/2] usb: typec: tipd: Add an additional overflow check

Quoted 28 lines · Sven Peter
Quoted 18 lines · Greg Kroah-Hartman
Quoted 17 lines · Sven Peter

tps6598x_block_read already checks for the maximum length of the read but tps6598x_block_write does not. Add the symmetric check there as well.

Signed-off-by Sven Peter <[email protected]>
---
 drivers/usb/typec/tipd/core.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/usb/typec/tipd/core.c +0 -0
index c18ec3785592..70e2d0d410c9 100644
@@ -139,6 +139,9 @@ static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
 {
 	u8 data[TPS_MAX_LEN + 1];
Quoted 6 lines

drivers/usb/typec/tipd/core.c static int tps6598x_block_write(struct tps6598x *tps, u8 reg, · line 143

Quoted 2 lines
drivers/usb/typec/tipd/core.c +2 -0
+	if (WARN_ON(len + 1 > sizeof(data)))
+		return -EINVAL;

No need to crash anything. If this is a valid thing for us to check, let's check it and handle the error, but we should not reboot systems that are running with panic-on-warn enabled, right?

Quoted 3 lines

Sure, that makes sense. I guess the same point applies to the WARN_ON in the same check in tps6598x_block_read. I can add a patch to remove that one to v2 as well.

That would be great, thanks.

greg k-h