[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