[PATCH] stm32mp: fix various array bounds checks

Patrice CHOTARD patrice.chotard at foss.st.com
Wed Apr 19 10:02:53 CEST 2023



On 3/24/23 10:39, Patrice CHOTARD wrote:
> Hi Rasmus
> 
> On 3/24/23 08:55, Rasmus Villemoes wrote:
>> In all these cases, the index on the LHS is immediately afterwards
>> used to access the array appearing in the ARRAY_SIZE() on the RHS - so
>> if that index is equal to the array size, we'll access
>> one-past-the-end of the array.
>>
>> Signed-off-by: Rasmus Villemoes <rasmus.villemoes at prevas.dk>
>> ---
>>
>> Patch generated with '-U5' to make that access obvious from the diff context.
>>
>>  arch/arm/mach-stm32mp/cpu.c                 | 4 ++--
>>  board/st/stm32mp1/stm32mp1.c                | 2 +-
>>  drivers/ram/stm32mp1/stm32mp1_interactive.c | 2 +-
>>  3 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c
>> index dc4112d5e6..e2f67fc423 100644
>> --- a/arch/arm/mach-stm32mp/cpu.c
>> +++ b/arch/arm/mach-stm32mp/cpu.c
>> @@ -188,11 +188,11 @@ static void setup_boot_mode(void)
>>  
>>  	log_debug("%s: boot_ctx=0x%x => boot_mode=%x, instance=%d forced=%x\n",
>>  		  __func__, boot_ctx, boot_mode, instance, forced_mode);
>>  	switch (boot_mode & TAMP_BOOT_DEVICE_MASK) {
>>  	case BOOT_SERIAL_UART:
>> -		if (instance > ARRAY_SIZE(serial_addr))
>> +		if (instance >= ARRAY_SIZE(serial_addr))
>>  			break;
>>  		/* serial : search associated node in devicetree */
>>  		sprintf(cmd, "serial@%x", serial_addr[instance]);
>>  		if (uclass_get_device_by_name(UCLASS_SERIAL, cmd, &dev)) {
>>  			/* restore console on error */
>> @@ -218,11 +218,11 @@ static void setup_boot_mode(void)
>>  		env_set("boot_device", "usb");
>>  		env_set("boot_instance", "0");
>>  		break;
>>  	case BOOT_FLASH_SD:
>>  	case BOOT_FLASH_EMMC:
>> -		if (instance > ARRAY_SIZE(sdmmc_addr))
>> +		if (instance >= ARRAY_SIZE(sdmmc_addr))
>>  			break;
>>  		/* search associated sdmmc node in devicetree */
>>  		sprintf(cmd, "mmc@%x", sdmmc_addr[instance]);
>>  		if (uclass_get_device_by_name(UCLASS_MMC, cmd, &dev)) {
>>  			printf("mmc%d = %s not found in device tree!\n",
>> diff --git a/board/st/stm32mp1/stm32mp1.c b/board/st/stm32mp1/stm32mp1.c
>> index ca8f0255ae..1a1b1844c8 100644
>> --- a/board/st/stm32mp1/stm32mp1.c
>> +++ b/board/st/stm32mp1/stm32mp1.c
>> @@ -870,11 +870,11 @@ int mmc_get_boot(void)
>>  		STM32_SDMMC1_BASE,
>>  		STM32_SDMMC2_BASE,
>>  		STM32_SDMMC3_BASE
>>  	};
>>  
>> -	if (instance > ARRAY_SIZE(sdmmc_addr))
>> +	if (instance >= ARRAY_SIZE(sdmmc_addr))
>>  		return 0;
>>  
>>  	/* search associated sdmmc node in devicetree */
>>  	snprintf(cmd, sizeof(cmd), "mmc@%x", sdmmc_addr[instance]);
>>  	if (uclass_get_device_by_name(UCLASS_MMC, cmd, &dev)) {
>> diff --git a/drivers/ram/stm32mp1/stm32mp1_interactive.c b/drivers/ram/stm32mp1/stm32mp1_interactive.c
>> index f0fe7e61e3..2c19847c66 100644
>> --- a/drivers/ram/stm32mp1/stm32mp1_interactive.c
>> +++ b/drivers/ram/stm32mp1/stm32mp1_interactive.c
>> @@ -389,11 +389,11 @@ bool stm32mp1_ddr_interactive(void *priv,
>>  	log_debug("** step %d ** %s / %d\n", step, step_str[step], next_step);
>>  
>>  	if (next_step < 0)
>>  		return false;
>>  
>> -	if (step < 0 || step > ARRAY_SIZE(step_str)) {
>> +	if (step < 0 || step >= ARRAY_SIZE(step_str)) {
>>  		printf("** step %d ** INVALID\n", step);
>>  		return false;
>>  	}
>>  
>>  	printf("%d:%s\n", step, step_str[step]);
> 
> Good catch !
> 
> Reviewed-by: Patrice Chotard <patrice.chotard at foss.st.com>
> 
> Thanks
> Patrice

Applied on u-boot-stm/master, thanks

Patrice


More information about the U-Boot mailing list