Re: [PATCH v5 5/6] iio: imu: inv_icm42607: Implement MREGx register access

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] iio: imu: inv_icm42607: error codes clobbered in mreg read/write
--

commit e8858e48c698c3e653b91adc09876f8bc8ccd389
Author: Kanak Shilledar <kanak.shilledar@axis.com>

iio: imu: inv_icm42607: Implement MREGx register access

This commit implements indirect register access for MREG banks via a 16-bit
virtual regmap layered on top of the 8-bit bus regmap. It sets up the bank
switching sequence for MREG accesses while forwarding Bank 0 accesses
directly to the bus regmap.

> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> index 87b1499a3689f..ea0d50237817b 100644
> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c

[ ... ]

> +static int inv_icm42607_mreg_read(struct regmap *map, unsigned int reg,
> +				  u8 *data, size_t count)
> +{

[ ... ]

> +	ret = regmap_read(map, INV_ICM42607_REG_M_R, &val);
> +	if (ret)
> +		goto out;
> +
> +	fsleep(INV_ICM42607_MREG_ACCESS_DELAY_US);
> +
> +	*data = val;
> +out:
> +	/* Restore direct access. */
> +	ret = regmap_write(map, INV_ICM42607_REG_BLK_SEL_R, 0);

[Severity: High]
Does this overwrite the primary read error code in ret?

If the function jumps to the out label due to a bus error, and the cleanup
regmap_write succeeds (returning 0), it masks the original failure.

This causes the caller to receive a success code while operating on
uninitialized memory.

Since these values can be transformed and exposed to userspace, such as via
IIO sysfs attributes, could this lead to a kernel stack information leak?

> +	inv_icm42607_mclk_put(map, idle_set);
> +
> +	return ret;
> +}
> +
> +static int inv_icm42607_mreg_write(struct regmap *map, unsigned int reg,
> +				   const u8 *data, size_t count)
> +{

[ ... ]

> +	ret = regmap_write(map, INV_ICM42607_REG_M_W, *data);
> +	if (ret)
> +		goto out;
> +
> +	fsleep(INV_ICM42607_MREG_ACCESS_DELAY_US);
> +
> +out:
> +	/* Restore direct access. */
> +	ret = regmap_write(map, INV_ICM42607_REG_BLK_SEL_W, 0);

[Severity: High]
Similarly, does this overwrite the write error code?

If an I2C or SPI bus error happens earlier in the function and causes a jump
to the out label, a successful cleanup write will overwrite the error code
and falsely report success to the caller.

> +	inv_icm42607_mclk_put(map, idle_set);
> +
> +	return ret;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261002-b4-inv_icm42370p-v5-0-c65281b745c9@axis.com?part=5