Re: [PATCH v2 2/3] i2c: designware: Support per-variant reg/bit offsets and intr handling

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] i2c: designware: out-of-bounds MMIO access via debugfs on non-AMD models
--

commit 4dc8f1b4bfdf49722d6f1e25100bf25c45d88afb
Author: Aniket Limaye <a-limaye@ti.com>
Subject: i2c: designware: Support per-variant reg/bit offsets and intr handling

This commit refactors the DesignWare I2C driver to support per-variant
register offsets and interrupt handling. It converts DW_IC_* to a logical enum,
using a regmap to internally translate to physical offsets. It also moves the
interrupt-acknowledgment pattern into a common helper driven by a per-variant
table.

Link: https://lore.kernel.org/r/20260919-tda54-upstream-i2c-v1-2-b0b9f77be18b@ti.com/

> diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
> index a1eca6cd4b75e..eeeb1f1892054 100644
> --- a/drivers/i2c/busses/i2c-designware-common.c
> +++ b/drivers/i2c/busses/i2c-designware-common.c
> @@ -72,11 +72,55 @@ static const char *const abort_sources[] = {
[ ... ]
>  static int dw_reg_read(void *context, unsigned int reg, unsigned int *val)
>  {
>  	struct dw_i2c_dev *dev = context;
>  
> -	*val = readl(dev->base + reg);
> +	*val = readl(dev->base + dev->regs[reg]);
>  
>  	return 0;
>  }
[ ... ]
> @@ -143,11 +189,11 @@ static int i2c_dw_init_regmap(struct dw_i2c_dev *dev)
>  	struct regmap_config map_cfg = {
>  		.reg_bits = 32,
>  		.val_bits = 32,
> -		.reg_stride = 4,
> +		.reg_stride = 1,
>  		.disable_locking = true,
>  		.reg_read = dw_reg_read,
>  		.reg_write = dw_reg_write,
> -		.max_register = DW_IC_COMP_TYPE,
> +		.max_register = DW_IC_REG_MAX - 1,
>  	};
>  	u32 reg;
>  	int ret;
> @@ -163,12 +209,9 @@ static int i2c_dw_init_regmap(struct dw_i2c_dev *dev)
>  	if (ret)
>  		return ret;
>  
> -	reg = readl(dev->base + DW_IC_COMP_TYPE);
> +	reg = readl(dev->base + dev->regs[DW_IC_COMP_TYPE]);
>  	i2c_dw_release_lock(dev);
>  
> -	if ((dev->flags & MODEL_MASK) == MODEL_AMD_NAVI_GPU)
> -		map_cfg.max_register = AMD_UCSI_INTR_REG;
> -

[Severity: High]
By unconditionally setting max_register to DW_IC_REG_MAX - 1, could this expose
the AMD-specific register (AMD_UCSI_INTR_REG, logical ID 40) via the regmap
debugfs interface on all models?

If a privileged user reads the regmap registers file in debugfs on a non-AMD
platform, the iteration would reach logical register 40. This translates to
physical offset 0x474 in dw_i2c_reg_offsets, which dw_reg_read() would attempt
to read via readl(dev->base + dev->regs[reg]).

Could this lead to an out-of-bounds MMIO read and potentially cause a system
crash (such as a bus fault or SError) or hardware hang, since the mapped MMIO
region on standard platforms might not extend to 0x474?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260925-tda54-upstream-i2c-v2-0-261785e58493@ti.com?part=2