Re: [PATCH v4 6/8] misc: amd-sbi: Add SBTSI ioctl register transfer interface

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Data race, DMA buffer corruption, and TOCTOU vulnerability due to concurrent hwmon and ioctl access to shared I3C buffers.
--

--- Patch [6]: [PATCH v4 6/8] misc: amd-sbi: Add SBTSI ioctl register transfer interface ---
commit 01a49ea8ef63627c7743953c52073396818f7f3a
Author: Prathima <Prathima.Lk@amd.com>

misc: amd-sbi: Add SBTSI ioctl register transfer interface

Implement IOCTL interface for SB-TSI driver to enable userspace access
to TSI register read/write operations through the AMD Advanced Platform
Management Link (APML) protocol.

> diff --git a/drivers/misc/amd-sbi/tsi-core.c b/drivers/misc/amd-sbi/tsi-core.c
> index 1c6f37f26d945..5c178702c67ab 100644
> --- a/drivers/misc/amd-sbi/tsi-core.c
> +++ b/drivers/misc/amd-sbi/tsi-core.c
[ ... ]
> @@ -77,7 +92,116 @@ int sbtsi_xfer(struct sbtsi_data *data, u8 reg, u8 *val, bool is_read)
>  	if (data->is_i3c)
>  		return is_read ? sbtsi_i3c_read(data, reg, val)
>  			       : sbtsi_i3c_write(data, reg, *val);
> -
>  	return sbtsi_i2c_xfer(data, reg, val, is_read);
>  }
>  EXPORT_SYMBOL_GPL(sbtsi_xfer);
> +
> +/*
> + * The mutex protects against concurrent register transfers to the device
> + * over the shared bus.
> + */
> +static int sbtsi_xfer_ioctl(struct sbtsi_data *data, u8 reg, u8 *val, bool is_read)
> +{
> +	guard(sbtsi)(data);
> +
> +	if (data->detached)
> +		return -ENODEV;
> +
> +	return sbtsi_xfer(data, reg, val, is_read);
> +}

[Severity: High]
The commit message mentions that the hwmon path is placed under the same
lock in the next patch, completing the serialization.

Since this patch introduces the ioctl path before the hwmon path is locked,
can an unprivileged user reading the hwmon sysfs attribute (temp1_input)
concurrently with the new SBTSI_IOCTL_REG_XFER_CMD ioctl cause data
corruption?

For I3C devices, sbtsi_xfer() calls sbtsi_i3c_read() and sbtsi_i3c_write(),
which use the shared priv->tx and priv->rx arrays for DMA transfers:

drivers/misc/amd-sbi/tsi-core.c:sbtsi_i3c_read() {
	...
	priv->tx[0] = reg;

	/* Write the register address (DMA_TO_DEVICE). */
	xfers[0].rnw = false;
	xfers[0].len = 1;
	xfers[0].data.out = priv->tx;
	...
}

Will this allow concurrent access to overwrite priv->tx while a privileged
ioctl is preparing or executing a DMA transfer, leading to buffer
corruption during git bisect?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710111642.850022-1-Akshay.Gupta@amd.com?part=6