[PATCH] IOMUX: Fix access past end of console_devices

Sean Anderson seanga2 at gmail.com
Wed Mar 30 19:13:10 CEST 2022


On 3/30/22 1:07 PM, Andy Shevchenko wrote:
> On Wed, Mar 30, 2022 at 8:01 PM Andy Shevchenko
> <andy.shevchenko at gmail.com> wrote:
>> On Wed, Mar 30, 2022 at 7:49 PM Sean Anderson <seanga2 at gmail.com> wrote:
> 
> ...
> 
>>>   #define for_each_console_dev(i, file, dev)             \
>>> -       for (i = 0, dev = console_devices[file][i];     \
>>
>> When we enter the loop, the dev is assigned and perhaps valid
>>
>>> -            i < cd_count[file];                        \
>>> -            i++, dev = console_devices[file][i])
>>> +       for (i = 0; i < cd_count[file] &&               \
>>
>> Not the case anymore.
>>
>>> +               (dev = console_devices[file][i]); i++)
> 
> On the second look, it seems a bit unusual, but for loop checks the
> condition before entering and in such case the dev will be assigned if
> the count is greater than 0.
> So, basically the difference is that dev is left completely
> uninitialized in case of count==0. However, it may not be a problem.

Well, in such a case, the value of console_devices[file][i] is bogus
anyway, so stack garbage is just as good.

If it turns out to be a problem, this can always be rewritten to

	for (i = 0 dev = NULL; i < cd_count[file] &&		\
		(dev = console_devices[file][i]); i++)

> Anyways, I would rather see better written for-loop that we see the iterations

Sorry, I'm not sure what you mean by this...

Ideally this loop would be written like

	for (i = 0 dev = NULL; i < cd_count[file]; i++) {
		dev = console_devices[file][i]);
		/* loop body */
	}

which is much more obviously correct. But since this macro must use the following
block as the loop body, it's trickier to do.

--Sean


More information about the U-Boot mailing list