Re: [PATCH v2] hwmon: (pmbus/q54sj108a2) fix stack overflow in debugfs read

[Guenter Roeck <linux@roeck-us.net>]

On 3/3/26 23:06, Sanman Pradhan wrote:
> The q54sj108a2_debugfs_read function suffers from a stack buffer overflow
> due to incorrect arguments passed to bin2hex(). The function currently
> passes 'data' as the destination and 'data_char' as the source.
> 
> Because bin2hex() converts each input byte into two hex characters, a
> 32-byte block read results in 64 bytes of output. Since 'data' is only
> 34 bytes (I2C_SMBUS_BLOCK_MAX + 2), this writes 30 bytes past the end
> of the buffer onto the stack.
> 
> Additionally, the arguments were swapped: it was reading from the
> zero-initialized 'data_char' and writing to 'data', resulting in
> all-zero output regardless of the actual I2C read.
> 
> Fix this by:
> 1. Expanding 'data_char' to 66 bytes to safely hold the hex output.
> 2. Correcting the bin2hex() argument order and using the actual read count.
> 3. Using a pointer to select the correct output buffer for the final
>     simple_read_from_buffer call.
> 
> Fixes: d014538aa385 ("hwmon: (pmbus) Driver for Delta power supplies Q54SJ108A2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sanman Pradhan <psanman@juniper.net>
> ---
> 
> v2:
> - Fixed email formatting/line-wrapping issues
> 
Unfortunately:

WARNING: From:/Signed-off-by: email address mismatch: 'From: Sanman Pradhan <sanman.p211993@gmail.com>' != 'Signed-off-by: Sanman Pradhan <psanman@juniper.net>'

Guenter

> ---
>   drivers/hwmon/pmbus/q54sj108a2.c | 19 ++++++++++---------
>   1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/hwmon/pmbus/q54sj108a2.c b/drivers/hwmon/pmbus/q54sj108a2.c
> index fc030ca34480..d5d60a9af8c5 100644
> --- a/drivers/hwmon/pmbus/q54sj108a2.c
> +++ b/drivers/hwmon/pmbus/q54sj108a2.c
> @@ -79,7 +79,8 @@ static ssize_t q54sj108a2_debugfs_read(struct file *file, char __user *buf,
>   	int idx = *idxp;
>   	struct q54sj108a2_data *psu = to_psu(idxp, idx);
>   	char data[I2C_SMBUS_BLOCK_MAX + 2] = { 0 };
> -	char data_char[I2C_SMBUS_BLOCK_MAX + 2] = { 0 };
> +	char data_char[I2C_SMBUS_BLOCK_MAX * 2 + 2] = { 0 };
> +	char *out = data;
>   	char *res;
>   
>   	switch (idx) {
> @@ -150,27 +151,27 @@ static ssize_t q54sj108a2_debugfs_read(struct file *file, char __user *buf,
>   		if (rc < 0)
>   			return rc;
>   
> -		res = bin2hex(data, data_char, 32);
> -		rc = res - data;
> -
> +		res = bin2hex(data_char, data, rc);
> +		rc = res - data_char;
> +		out = data_char;
>   		break;
>   	case Q54SJ108A2_DEBUGFS_FLASH_KEY:
>   		rc = i2c_smbus_read_block_data(psu->client, PMBUS_FLASH_KEY_WRITE, data);
>   		if (rc < 0)
>   			return rc;
>   
> -		res = bin2hex(data, data_char, 4);
> -		rc = res - data;
> -
> +		res = bin2hex(data_char, data, rc);
> +		rc = res - data_char;
> +		out = data_char;
>   		break;
>   	default:
>   		return -EINVAL;
>   	}
>   
> -	data[rc] = '\n';
> +	out[rc] = '\n';
>   	rc += 2;
>   
> -	return simple_read_from_buffer(buf, count, ppos, data, rc);
> +	return simple_read_from_buffer(buf, count, ppos, out, rc);
>   }
>   
>   static ssize_t q54sj108a2_debugfs_write(struct file *file, const char __user *buf,