[PATCH v2] hwmon: (tps53679) Fix device ID comparison and printing in tps53676_identify()

[PATCH v2] hwmon: (tps53679) Fix device ID comparison and printing in tps53676_identify()

[Pradhan, Sanman]

From: Sanman Pradhan <psanman@juniper.net>

tps53676_identify() uses strncmp() to compare the device ID buffer
against a byte sequence containing embedded non-printable bytes
(\x53\x67\x60). strncmp() is semantically wrong for binary data
comparison; use memcmp() instead.

Additionally, the buffer from i2c_smbus_read_block_data() is not
NUL-terminated, so printing it with "%s" in the error path is
undefined behavior and may read past the buffer. Use "%*ph" to
hex-dump the actual bytes returned.

Per the datasheet, the expected device ID is the 6-byte sequence
54 49 53 67 60 00 ("TI\x53\x67\x60\x00"), so compare all 6 bytes
including the trailing NUL.

Fixes: cb3d37b59012 ("hwmon: (pmbus/tps53679) Add support for TI TPS53676")
Signed-off-by: Sanman Pradhan <psanman@juniper.net>
---
v2:
 - Compare full 6-byte datasheet device ID including trailing NUL
 - Return -ENODEV for any mismatch
---
 drivers/hwmon/pmbus/tps53679.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/pmbus/tps53679.c b/drivers/hwmon/pmbus/tps53679.c
index 1a6abc32afe2..94258e8cfd90 100644
--- a/drivers/hwmon/pmbus/tps53679.c
+++ b/drivers/hwmon/pmbus/tps53679.c
@@ -175,8 +175,8 @@ static int tps53676_identify(struct i2c_client *client,
 	ret = i2c_smbus_read_block_data(client, PMBUS_IC_DEVICE_ID, buf);
 	if (ret < 0)
 		return ret;
-	if (strncmp("TI\x53\x67\x60", buf, 5)) {
-		dev_err(&client->dev, "Unexpected device ID: %s\n", buf);
+	if (ret != 6 || memcmp(buf, "TI\x53\x67\x60\x00", 6)) {
+		dev_err(&client->dev, "Unexpected device ID: %*ph\n", ret, buf);
 		return -ENODEV;
 	}
 
-- 
2.34.1

Re: [PATCH v2] hwmon: (tps53679) Fix device ID comparison and printing in tps53676_identify()

[Guenter Roeck]

On Mon, Mar 30, 2026 at 03:56:40PM +0000, Pradhan, Sanman wrote:
> From: Sanman Pradhan <psanman@juniper.net>
> 
> tps53676_identify() uses strncmp() to compare the device ID buffer
> against a byte sequence containing embedded non-printable bytes
> (\x53\x67\x60). strncmp() is semantically wrong for binary data
> comparison; use memcmp() instead.
> 
> Additionally, the buffer from i2c_smbus_read_block_data() is not
> NUL-terminated, so printing it with "%s" in the error path is
> undefined behavior and may read past the buffer. Use "%*ph" to
> hex-dump the actual bytes returned.
> 
> Per the datasheet, the expected device ID is the 6-byte sequence
> 54 49 53 67 60 00 ("TI\x53\x67\x60\x00"), so compare all 6 bytes
> including the trailing NUL.
> 
> Fixes: cb3d37b59012 ("hwmon: (pmbus/tps53679) Add support for TI TPS53676")
> Signed-off-by: Sanman Pradhan <psanman@juniper.net>

Applied.

Thanks,
Guenter

Re: [PATCH v2] hwmon: (tps53679) Fix device ID comparison and printing in tps53676_identify()

[Andy Shevchenko]

On Mon, Mar 30, 2026 at 03:56:40PM +0000, Pradhan, Sanman wrote:
> 
> tps53676_identify() uses strncmp() to compare the device ID buffer
> against a byte sequence containing embedded non-printable bytes
> (\x53\x67\x60). strncmp() is semantically wrong for binary data
> comparison; use memcmp() instead.
> 
> Additionally, the buffer from i2c_smbus_read_block_data() is not
> NUL-terminated, so printing it with "%s" in the error path is
> undefined behavior and may read past the buffer. Use "%*ph" to
> hex-dump the actual bytes returned.
> 
> Per the datasheet, the expected device ID is the 6-byte sequence
> 54 49 53 67 60 00 ("TI\x53\x67\x60\x00"), so compare all 6 bytes
> including the trailing NUL.

Your patch seems okay to me. But just for your information I would do it
differently. Id est the sequence you got for the comparison is the perfect
ASCII one, but this is a coincident as the actual ID is in FourCC BCD.

The better check is to compare first two bytes to TI and device ID separately
as FourCC. Do you want me to send a patch or you can do it yourself?

(Also note that there is no need to use *ph as we know the length and it's
 always the same, so %6ph suffice, but again, you want to print the FourCC,
 %p4c.)

...

>  	ret = i2c_smbus_read_block_data(client, PMBUS_IC_DEVICE_ID, buf);
>  	if (ret < 0)
>  		return ret;
> -	if (strncmp("TI\x53\x67\x60", buf, 5)) {
> -		dev_err(&client->dev, "Unexpected device ID: %s\n", buf);
> +	if (ret != 6 || memcmp(buf, "TI\x53\x67\x60\x00", 6)) {
> +		dev_err(&client->dev, "Unexpected device ID: %*ph\n", ret, buf);
>  		return -ENODEV;
>  	}

#define TPS53676_DEVICE_ID_MAGIC	0x53676000

	if (ret != 6)
		dev_err(&client->dev, "Malformed response\n");
		return -EIO;
	}
	if (strncmp(buf, "TI", 2)) {
		dev_err(&client->dev, "Unexpected vendor ID: %2pE\n", buf); // use %pE since it maybe unprintable
		return -ENODEV;
	}
	if (get_unaligned_be32(buf + 2) != _DEVICE_ID_MAGIC) {
		dev_err(&client->dev, "Unexpected device ID: %p4c\n", buf + 2);
		return -ENODEV;
	}


-- 
With Best Regards,
Andy Shevchenko