Re: [PATCH v3 3/4] hwmon: (kb9002) Add driver for Kandou KB9002 retimer

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Stack-allocated buffers are passed to `i2c_transfer()`, which is unsafe if the underlying I2C adapter driver uses DMA.
--

--- Patch [3]: [PATCH v3 3/4] hwmon: (kb9002) Add driver for Kandou KB9002 retimer ---
commit 2acb53c63efe6bebfdbda9c58f2be139429caa78
Author: Andy Chung <Andy.Chung@amd.com>

hwmon: (kb9002) Add driver for Kandou KB9002 retimer

This commit adds a hwmon driver for the Kandou KB9002 8-lane PCIe 5.0
retimer. It exposes the firmware aggregated maximum die temperature as
temp1_input and adds debugfs entries for firmware version and boot status.

> diff --git a/drivers/hwmon/kb9002.c b/drivers/hwmon/kb9002.c
> new file mode 100644
> index 0000000000000..61a6e522e770d
> --- /dev/null
> +++ b/drivers/hwmon/kb9002.c

[ ... ]

> +static int kb9002_i2c_read(struct i2c_client *client, u32 reg, u32 *val)
> +{
> +	u8 addr[4];
> +	u8 rbuf[4];
> +	struct i2c_msg msgs[2] = {
> +		{
> +			.addr = client->addr,
> +			.flags = 0,
> +			.len = sizeof(addr),
> +			.buf = addr,
> +		},
> +		{
> +			.addr = client->addr,
> +			.flags = I2C_M_RD,
> +			.len = sizeof(rbuf),
> +			.buf = rbuf,
> +		},
> +	};
> +	int ret;
> +
> +	put_unaligned_be32(reg, addr);
> +
> +	ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));

[Severity: High]
Does this pass stack-allocated buffers (addr and rbuf) to i2c_transfer()?
Passing stack memory to the I2C API is unsafe if the underlying adapter uses
DMA, which can lead to mapping errors or stack memory corruption. Could these
buffers be allocated dynamically or placed in a device data structure instead?

> +	if (ret < 0)

[ ... ]

> +static int kb9002_i2c_write(struct i2c_client *client, u32 reg, u32 val)
> +{
> +	u8 buf[8];
> +	struct i2c_msg msg = {
> +		.addr = client->addr,
> +		.flags = 0,
> +		.len = sizeof(buf),
> +		.buf = buf,
> +	};
> +	int ret;
> +
> +	put_unaligned_be32(reg, &buf[0]);
> +	put_unaligned_be32(val, &buf[4]);
> +
> +	ret = i2c_transfer(client->adapter, &msg, 1);

[Severity: High]
Are we also passing the stack-allocated buf to i2c_transfer() here?
Similar to the read path, using stack memory for I2C transfers is unsafe
for adapters that use DMA.

> +	if (ret < 0)

[ ... ]

> +static int kb9002_read_revid(struct i2c_client *client, u32 *revid)
> +{
> +	u8 dummy;
> +	int ret;
> +	int i;
> +
> +	for (i = 0; i < KB9002_REVID_READ_RETRIES; i++) {
> +		ret = kb9002_i2c_read(client, KB9002_HW_REG_REVID, revid);
> +		if (ret)
> +			return ret;
> +		if ((*revid >> 24) != 0xff)
> +			return 0;
> +		/* Drain one byte from the chip to re-align the I2C FIFO. */
> +		i2c_master_recv(client, &dummy, 1);

[Severity: High]
Does i2c_master_recv() receive the stack-allocated dummy variable?
This function internally calls i2c_transfer(), which again risks DMA
mapping issues with stack memory.

> +	}
> +
> +	return -EIO;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-kb9002-upstream-v3-0-70563922be8d@amd.com?part=3