Re: [PATCH v2] phy: stm32: Optimize tuning values from DT.

[Christian Bruel <christian.bruel@foss.st.com>]

Hi Dan,

Thanks for reporting. I think this error is a false positive.

In this situation, the impedance values that are looked-up are ordere 
(the compiler probably doesn't analysis this) so the imp_looup test that 
defines the imp_of must have matched.
Note that the boundaries was checked during probe.

already discussed here:
https://lore.kernel.org/all/ef1ea5da-4344-4c59-8d2b1b52533ef0cd@foss.st.com/

I'll provide a new revision to make this code more friendly with this error

Regards

Christian

On 1/17/25 09:38, Dan Carpenter wrote:
> Hi Christian,
> 
> kernel test robot noticed the following build warnings:
> 
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Christian-Bruel/phy-stm32-Optimize-tuning-values-from-DT/20250113-172435
> base:   v6.13-rc7
> patch link:    https://lore.kernel.org/r/20250113092001.1344151-1-christian.bruel%40foss.st.com
> patch subject: [PATCH v2] phy: stm32: Optimize tuning values from DT.
> config: arm-randconfig-r071-20250117 (https://download.01.org/0day-ci/archive/20250117/202501171619.0XDYDyBZ-lkp@intel.com/config)
> compiler: arm-linux-gnueabi-gcc (GCC) 14.2.0
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> | Closes: https://lore.kernel.org/r/202501171619.0XDYDyBZ-lkp@intel.com/
> 
> New smatch warnings:
> drivers/phy/st/phy-stm32-combophy.c:147 stm32_impedance_tune() error: buffer overflow 'imp_lookup' 8 <= 8 (assuming for loop doesn't break)
> drivers/phy/st/phy-stm32-combophy.c:564 stm32_combophy_probe() warn: passing zero to 'dev_err_probe'
> 
> vim +147 drivers/phy/st/phy-stm32-combophy.c
> 
> 31219ca5436f01 Christian Bruel 2025-01-13  119  static void stm32_impedance_tune(struct stm32_combophy *combophy)
> 47e1bb6b4ba098 Christian Bruel 2024-09-30  120  {
> 47e1bb6b4ba098 Christian Bruel 2024-09-30  121  	u8 imp_of, vswing_of;
> 2de679ecd724b8 Arnd Bergmann   2024-11-11  122  	u32 regval;
> 47e1bb6b4ba098 Christian Bruel 2024-09-30  123
> 31219ca5436f01 Christian Bruel 2025-01-13  124  	if (combophy->microohm) {
> 2de679ecd724b8 Arnd Bergmann   2024-11-11  125  		regval = 0;
> 2de679ecd724b8 Arnd Bergmann   2024-11-11  126  		for (imp_of = 0; imp_of < ARRAY_SIZE(imp_lookup); imp_of++) {
> 31219ca5436f01 Christian Bruel 2025-01-13  127  			if (imp_lookup[imp_of].microohm <= combophy->microohm) {
> 2de679ecd724b8 Arnd Bergmann   2024-11-11  128  				regval = FIELD_PREP(STM32MP25_PCIEPRG_IMPCTRL_OHM, imp_of);
> 47e1bb6b4ba098 Christian Bruel 2024-09-30  129  				break;
> 2de679ecd724b8 Arnd Bergmann   2024-11-11  130  			}
> 2de679ecd724b8 Arnd Bergmann   2024-11-11  131  		}
> 
> If we don't hit the break sttaement above
> 
> 47e1bb6b4ba098 Christian Bruel 2024-09-30  132
> 47e1bb6b4ba098 Christian Bruel 2024-09-30  133  		dev_dbg(combophy->dev, "Set %u micro-ohms output impedance\n",
> 47e1bb6b4ba098 Christian Bruel 2024-09-30  134  			imp_lookup[imp_of].microohm);
>                                                                                     ^^^^^^
> Then this is an out of bounds access.
> 
> 47e1bb6b4ba098 Christian Bruel 2024-09-30  135
> 47e1bb6b4ba098 Christian Bruel 2024-09-30  136  		regmap_update_bits(combophy->regmap, SYSCFG_PCIEPRGCR,
> 47e1bb6b4ba098 Christian Bruel 2024-09-30  137  				   STM32MP25_PCIEPRG_IMPCTRL_OHM,
> 2de679ecd724b8 Arnd Bergmann   2024-11-11  138  				   regval);
> 47e1bb6b4ba098 Christian Bruel 2024-09-30  139  	} else {
> 31219ca5436f01 Christian Bruel 2025-01-13  140  		/* default is 50 ohm */
> 31219ca5436f01 Christian Bruel 2025-01-13  141  		imp_of = 3;
> 47e1bb6b4ba098 Christian Bruel 2024-09-30  142  	}
> 47e1bb6b4ba098 Christian Bruel 2024-09-30  143
> 31219ca5436f01 Christian Bruel 2025-01-13  144  	if (combophy->microvolt) {
> 2de679ecd724b8 Arnd Bergmann   2024-11-11  145  		regval = 0;
> 2de679ecd724b8 Arnd Bergmann   2024-11-11  146  		for (vswing_of = 0; vswing_of < ARRAY_SIZE(imp_lookup[imp_of].vswing); vswing_of++) {
> 31219ca5436f01 Christian Bruel 2025-01-13 @147  			if (imp_lookup[imp_of].vswing[vswing_of] >= combophy->microvolt) {
> 2de679ecd724b8 Arnd Bergmann   2024-11-11  148  				regval = FIELD_PREP(STM32MP25_PCIEPRG_IMPCTRL_VSWING, vswing_of);
> 47e1bb6b4ba098 Christian Bruel 2024-09-30  149  				break;
> 2de679ecd724b8 Arnd Bergmann   2024-11-11  150  			}
> 2de679ecd724b8 Arnd Bergmann   2024-11-11  151  		}
> 47e1bb6b4ba098 Christian Bruel 2024-09-30  152
> 47e1bb6b4ba098 Christian Bruel 2024-09-30  153  		dev_dbg(combophy->dev, "Set %u microvolt swing\n",
> 47e1bb6b4ba098 Christian Bruel 2024-09-30  154  			 imp_lookup[imp_of].vswing[vswing_of]);
> 47e1bb6b4ba098 Christian Bruel 2024-09-30  155
> 47e1bb6b4ba098 Christian Bruel 2024-09-30  156  		regmap_update_bits(combophy->regmap, SYSCFG_PCIEPRGCR,
> 47e1bb6b4ba098 Christian Bruel 2024-09-30  157  				   STM32MP25_PCIEPRG_IMPCTRL_VSWING,
> 2de679ecd724b8 Arnd Bergmann   2024-11-11  158  				   regval);
> 47e1bb6b4ba098 Christian Bruel 2024-09-30  159  	}
> 47e1bb6b4ba098 Christian Bruel 2024-09-30  160  }
>