[PATCH 0/4] hwmon: PMBus driver bug fixes

[PATCH 0/4] hwmon: PMBus driver bug fixes

[Pradhan, Sanman]

From: Sanman Pradhan <psanman@juniper.net>

Four bug fixes found during a code audit of drivers/hwmon/pmbus/:

 1. tps53679: Prevent out-of-bounds stack read when block read returns
    zero bytes.

 2. tps53679: Use binary-safe comparison and logging for PMBUS_IC_DEVICE_ID;
    distinguish short reads (-EIO) from wrong device IDs (-ENODEV).

 3. pxe1610: Propagate page-select write failure instead of silently
    proceeding with an indeterminate register page.

 4. ltc4286: Add missing PMBUS namespace import to silence modpost
    warnings.

Sanman Pradhan (4):
  hwmon: (tps53679) Fix array access with zero-length block read
  hwmon: (tps53679) Fix device ID comparison and printing in
    tps53676_identify()
  hwmon: (pxe1610) Check return value of page-select write in probe
  hwmon: (ltc4286) Add missing MODULE_IMPORT_NS("PMBUS")

 drivers/hwmon/pmbus/ltc4286.c  |  1 +
 drivers/hwmon/pmbus/pxe1610.c  |  5 ++++-
 drivers/hwmon/pmbus/tps53679.c | 14 +++++++++-----
 3 files changed, 14 insertions(+), 6 deletions(-)

-- 
2.34.1

[PATCH 1/4] hwmon: (tps53679) Fix array access with zero-length block read

[Pradhan, Sanman]

From: Sanman Pradhan <psanman@juniper.net>

i2c_smbus_read_block_data() can return 0, indicating a zero-length
read. When this happens, tps53679_identify_chip() accesses buf[ret - 1]
which is buf[-1], reading one byte before the buffer on the stack.

Fix by changing the check from "ret < 0" to "ret <= 0", treating a
zero-length read as an error (-EIO), which prevents the out-of-bounds
array access.

Also fix a typo in the adjacent comment: "if present" instead of
duplicate "if".

Fixes: 75ca1e5875fe ("hwmon: (pmbus/tps53679) Add support for TPS53685")
Signed-off-by: Sanman Pradhan <psanman@juniper.net>
---
 drivers/hwmon/pmbus/tps53679.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/pmbus/tps53679.c b/drivers/hwmon/pmbus/tps53679.c
index df2726659a4e..1a6abc32afe2 100644
--- a/drivers/hwmon/pmbus/tps53679.c
+++ b/drivers/hwmon/pmbus/tps53679.c
@@ -103,10 +103,10 @@ static int tps53679_identify_chip(struct i2c_client *client,
 	}
 
 	ret = i2c_smbus_read_block_data(client, PMBUS_IC_DEVICE_ID, buf);
-	if (ret < 0)
-		return ret;
+	if (ret <= 0)
+		return ret < 0 ? ret : -EIO;
 
-	/* Adjust length if null terminator if present */
+	/* Adjust length if null terminator is present */
 	buf_len = (buf[ret - 1] != '\x00' ? ret : ret - 1);
 
 	id_len = strlen(id);
-- 
2.34.1

[PATCH 2/4] 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.

Also add a short-read guard: if fewer than 5 bytes are returned,
report -EIO (protocol/hardware failure) rather than -ENODEV, since
a truncated response is not the same as a wrong device ID.

Fixes: cb3d37b59012 ("hwmon: (pmbus/tps53679) Add support for TI TPS53676")
Signed-off-by: Sanman Pradhan <psanman@juniper.net>
---
 drivers/hwmon/pmbus/tps53679.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/pmbus/tps53679.c b/drivers/hwmon/pmbus/tps53679.c
index 1a6abc32afe2..d6600502d6b0 100644
--- a/drivers/hwmon/pmbus/tps53679.c
+++ b/drivers/hwmon/pmbus/tps53679.c
@@ -175,8 +175,12 @@ 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 < 5) {
+		dev_err(&client->dev, "Short device ID read: %*ph\n", ret, buf);
+		return -EIO;
+	}
+	if (memcmp(buf, "TI\x53\x67\x60", 5)) {
+		dev_err(&client->dev, "Unexpected device ID: %*ph\n", ret, buf);
 		return -ENODEV;
 	}
 
-- 
2.34.1

[PATCH 3/4] hwmon: (pxe1610) Check return value of page-select write in probe

[Pradhan, Sanman]

From: Sanman Pradhan <psanman@juniper.net>

pxe1610_probe() writes PMBUS_PAGE to select page 0 but does not check
the return value. If the write fails, subsequent register reads operate
on an indeterminate page, leading to silent misconfiguration.

Check the return value and propagate the error using dev_err_probe(),
which also handles -EPROBE_DEFER correctly without log spam.

Fixes: 28ccdfd11344 ("hwmon: (pmbus) Add Infineon PXE1610 VR driver")
Signed-off-by: Sanman Pradhan <psanman@juniper.net>
---
 drivers/hwmon/pmbus/pxe1610.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/pmbus/pxe1610.c b/drivers/hwmon/pmbus/pxe1610.c
index 6a4a978eca7e..24c1f961c766 100644
--- a/drivers/hwmon/pmbus/pxe1610.c
+++ b/drivers/hwmon/pmbus/pxe1610.c
@@ -104,7 +104,10 @@ static int pxe1610_probe(struct i2c_client *client)
 	 * By default this device doesn't boot to page 0, so set page 0
 	 * to access all pmbus registers.
 	 */
-	i2c_smbus_write_byte_data(client, PMBUS_PAGE, 0);
+	ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, 0);
+	if (ret < 0)
+		return dev_err_probe(&client->dev, ret,
+				     "Failed to set page 0\n");
 
 	/* Read Manufacturer id */
 	ret = i2c_smbus_read_block_data(client, PMBUS_MFR_ID, buf);
-- 
2.34.1

[PATCH 4/4] hwmon: (ltc4286) Add missing MODULE_IMPORT_NS("PMBUS")

[Pradhan, Sanman]

From: Sanman Pradhan <psanman@juniper.net>

ltc4286.c uses PMBus core symbols exported in the PMBUS namespace,
such as pmbus_do_probe(), but does not declare MODULE_IMPORT_NS("PMBUS").

Add the missing namespace import to avoid modpost warnings.

Fixes: 0c459759ca97 ("hwmon: (pmbus) Add ltc4286 driver")
Signed-off-by: Sanman Pradhan <psanman@juniper.net>
---
 drivers/hwmon/pmbus/ltc4286.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/hwmon/pmbus/ltc4286.c b/drivers/hwmon/pmbus/ltc4286.c
index aabd0bcdfeee..8715d380784a 100644
--- a/drivers/hwmon/pmbus/ltc4286.c
+++ b/drivers/hwmon/pmbus/ltc4286.c
@@ -173,3 +173,4 @@ module_i2c_driver(ltc4286_driver);
 MODULE_AUTHOR("Delphine CC Chiu <Delphine_CC_Chiu@wiwynn.com>");
 MODULE_DESCRIPTION("PMBUS driver for LTC4286 and compatibles");
 MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("PMBUS");
-- 
2.34.1

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

[Guenter Roeck]

Hi,

On 3/29/26 10:09, 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.
> 
> Also add a short-read guard: if fewer than 5 bytes are returned,
> report -EIO (protocol/hardware failure) rather than -ENODEV, since
> a truncated response is not the same as a wrong device ID.
> 
Ah, but we don't know and can not assume that this is a truncated ID.
It might as well be some other device.

> Fixes: cb3d37b59012 ("hwmon: (pmbus/tps53679) Add support for TI TPS53676")
> Signed-off-by: Sanman Pradhan <psanman@juniper.net>
> ---
>   drivers/hwmon/pmbus/tps53679.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwmon/pmbus/tps53679.c b/drivers/hwmon/pmbus/tps53679.c
> index 1a6abc32afe2..d6600502d6b0 100644
> --- a/drivers/hwmon/pmbus/tps53679.c
> +++ b/drivers/hwmon/pmbus/tps53679.c
> @@ -175,8 +175,12 @@ 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 < 5) {
> +		dev_err(&client->dev, "Short device ID read: %*ph\n", ret, buf);
> +		return -EIO;

This is not an -EIO: It suggests a different device. Please change the comparison
to "ret != 0" and return -ENODEV. In other words,

> +	}
> +	if (memcmp(buf, "TI\x53\x67\x60", 5)) {

Just change the original code. The expected device ID, according to the datasheet,
is, in hex, "54 49 53 67 60 00", with a terminating 0x00. So, when using memcmp,
the comparison should be something like

	if (ret != 6 || memcmp(buf, "TI\x53\x67\x60\x00", 6)) {

Thanks,
Guenter

Re: [PATCH 1/4] hwmon: (tps53679) Fix array access with zero-length block read

[Guenter Roeck]

On Sun, Mar 29, 2026 at 05:09:40PM +0000, Pradhan, Sanman wrote:
> From: Sanman Pradhan <psanman@juniper.net>
> 
> i2c_smbus_read_block_data() can return 0, indicating a zero-length
> read. When this happens, tps53679_identify_chip() accesses buf[ret - 1]
> which is buf[-1], reading one byte before the buffer on the stack.
> 
> Fix by changing the check from "ret < 0" to "ret <= 0", treating a
> zero-length read as an error (-EIO), which prevents the out-of-bounds
> array access.
> 
> Also fix a typo in the adjacent comment: "if present" instead of
> duplicate "if".
> 
> Fixes: 75ca1e5875fe ("hwmon: (pmbus/tps53679) Add support for TPS53685")
> Signed-off-by: Sanman Pradhan <psanman@juniper.net>

Applied.

Thanks,
Guenter

Re: [PATCH 3/4] hwmon: (pxe1610) Check return value of page-select write in probe

[Guenter Roeck]

On Sun, Mar 29, 2026 at 05:09:48PM +0000, Pradhan, Sanman wrote:
> From: Sanman Pradhan <psanman@juniper.net>
> 
> pxe1610_probe() writes PMBUS_PAGE to select page 0 but does not check
> the return value. If the write fails, subsequent register reads operate
> on an indeterminate page, leading to silent misconfiguration.
> 
> Check the return value and propagate the error using dev_err_probe(),
> which also handles -EPROBE_DEFER correctly without log spam.
> 
> Fixes: 28ccdfd11344 ("hwmon: (pmbus) Add Infineon PXE1610 VR driver")
> Signed-off-by: Sanman Pradhan <psanman@juniper.net>

Applied, after fixing the "Fixes" SHA.

Thanks,
Guenter

Re: [PATCH 4/4] hwmon: (ltc4286) Add missing MODULE_IMPORT_NS("PMBUS")

[Guenter Roeck]

On Sun, Mar 29, 2026 at 05:09:53PM +0000, Pradhan, Sanman wrote:
> From: Sanman Pradhan <psanman@juniper.net>
> 
> ltc4286.c uses PMBus core symbols exported in the PMBUS namespace,
> such as pmbus_do_probe(), but does not declare MODULE_IMPORT_NS("PMBUS").
> 
> Add the missing namespace import to avoid modpost warnings.
> 
> Fixes: 0c459759ca97 ("hwmon: (pmbus) Add ltc4286 driver")
> Signed-off-by: Sanman Pradhan <psanman@juniper.net>

Applied.

Thanks,
Guenter