Skip to contentSkip to conversation

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

PATCH In review Sven Peter 2 participants 6 messages Sep 14, 2021 – Sep 15, 2021

Subsystems (2)
USB USB Type-C class
lore
Share & tools
lore

Sven Peter

author
#0 lore raw

PATCH v2 1/3 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]>

---
no changes for v2

 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 v2 2/3 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]>

---
v1 -> v2:
- removed the WARN_ON to not crash machines running with panic-on-warn as pointed out by greg k-h

 drivers/usb/typec/tipd/core.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/usb/typec/tipd/core.c +3 -0
index c18ec3785592..8c79ba17a157 100644
@@ -139,6 +139,9 @@ static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
 {
 	u8 data[TPS_MAX_LEN + 1];
 
+	if (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 Reviewed-by on v2 2/3
#2 lore raw

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

Quoted 5 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]>
Reviewed-by Heikki Krogerus <[email protected]>
Quoted 24 lines

---
v1 -> v2:
- removed the WARN_ON to not crash machines running with panic-on-warn as pointed out by greg k-h

 drivers/usb/typec/tipd/core.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/usb/typec/tipd/core.c +3 -0
index c18ec3785592..8c79ba17a157 100644
@@ -139,6 +139,9 @@ static int tps6598x_block_write(struct tps6598x *tps, u8 reg,
 {
 	u8 data[TPS_MAX_LEN + 1];
 
+	if (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

thanks,

Signature · 1 line
heikki
author
#3 lore raw

PATCH v2 3/3 usb: typec: tipd: Remove WARN_ON in tps6598x_block_read

In reply to Sven Peter

Calling tps6598x_block_read with a higher than allowed len can be handled by just returning an error. There's no need to crash systems with panic-on-warn enabled.

Signed-off-by Sven Peter <[email protected]>

---
v1 -> v2:
- added this patch to also remove the WARN_ON in tps6598x_block_read as suggested by greg k-h

 drivers/usb/typec/tipd/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/typec/tipd/core.c +1 -1
index 8c79ba17a157..93e56291f0cf 100644
@@ -117,7 +117,7 @@ tps6598x_block_read(struct tps6598x *tps, u8 reg, void *val, size_t len)
 	u8 data[TPS_MAX_LEN + 1];
 	int ret;
 
-	if (WARN_ON(len + 1 > sizeof(data)))
+	if (len + 1 > sizeof(data))
 		return -EINVAL;
 
 	if (!tps->i2c_protocol)
Signature · 1 line
2.25.1
maintainer Reviewed-by on v2 3/3
#4 lore raw

Re: [PATCH v2 3/3] usb: typec: tipd: Remove WARN_ON in tps6598x_block_read

Quoted 5 lines · Sven Peter

Calling tps6598x_block_read with a higher than allowed len can be handled by just returning an error. There's no need to crash systems with panic-on-warn enabled.

Signed-off-by Sven Peter <[email protected]>
Reviewed-by Heikki Krogerus <[email protected]>
Quoted 23 lines

---
v1 -> v2:
- added this patch to also remove the WARN_ON in tps6598x_block_read as suggested by greg k-h

 drivers/usb/typec/tipd/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/typec/tipd/core.c +1 -1
index 8c79ba17a157..93e56291f0cf 100644
@@ -117,7 +117,7 @@ tps6598x_block_read(struct tps6598x *tps, u8 reg, void *val, size_t len)
 	u8 data[TPS_MAX_LEN + 1];
 	int ret;
 
-	if (WARN_ON(len + 1 > sizeof(data)))
+	if (len + 1 > sizeof(data))
 		return -EINVAL;
 
 	if (!tps->i2c_protocol)
Signature · 1 line
2.25.1

thanks,

Signature · 1 line
heikki
maintainer Reviewed-by on v2 1/3
#5 lore raw
In reply to Sven Peter
Quoted 5 lines · Sven Peter

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]>
Reviewed-by Heikki Krogerus <[email protected]>
Quoted 30 lines

---
no changes for v2

 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

thanks,

Signature · 1 line
heikki