[PATCH v2] IOMUX: Fix access past end of console_devices

We should only access console_devices[file][i] once we have checked that i < cd_count[file]. Otherwise, we will access uninitialized memory at the end of the loop. console_devices[file][i] should not be NULL, but putting the assignment in the loop condition allows us to ensure that i is checked beforehand. This isn't a bug, but it does make valgrind stop complaining.
Fixes: 400797cad3 ("IOMUX: Split out for_each_console_dev() helper macro") Signed-off-by: Sean Anderson seanga2@gmail.com Reviewed-by: Andrew Scull ascull@google.com ---
Changes in v2: - Put each clause of the for loop on its own line
include/iomux.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/iomux.h b/include/iomux.h index 37f5f6dee6..35caa697eb 100644 --- a/include/iomux.h +++ b/include/iomux.h @@ -24,10 +24,10 @@ extern struct stdio_dev **console_devices[MAX_FILES]; */ extern int cd_count[MAX_FILES];
-#define for_each_console_dev(i, file, dev) \ - for (i = 0, dev = console_devices[file][i]; \ - i < cd_count[file]; \ - i++, dev = console_devices[file][i]) +#define for_each_console_dev(i, file, dev) \ + for (i = 0; \ + i < cd_count[file] && (dev = console_devices[file][i]); \ + i++)
int iomux_match_device(struct stdio_dev **, const int, struct stdio_dev *); int iomux_doenv(const int, const char *);

On Wed, Apr 06, 2022 at 02:36:35PM -0400, Sean Anderson wrote:
We should only access console_devices[file][i] once we have checked that i < cd_count[file]. Otherwise, we will access uninitialized memory at the end of the loop. console_devices[file][i] should not be NULL, but putting the assignment in the loop condition allows us to ensure that i is checked beforehand. This isn't a bug, but it does make valgrind stop complaining.
Acked-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
Fixes: 400797cad3 ("IOMUX: Split out for_each_console_dev() helper macro") Signed-off-by: Sean Anderson seanga2@gmail.com Reviewed-by: Andrew Scull ascull@google.com
Changes in v2:
- Put each clause of the for loop on its own line
include/iomux.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/iomux.h b/include/iomux.h index 37f5f6dee6..35caa697eb 100644 --- a/include/iomux.h +++ b/include/iomux.h @@ -24,10 +24,10 @@ extern struct stdio_dev **console_devices[MAX_FILES]; */ extern int cd_count[MAX_FILES];
-#define for_each_console_dev(i, file, dev) \
- for (i = 0, dev = console_devices[file][i]; \
i < cd_count[file]; \
i++, dev = console_devices[file][i])
+#define for_each_console_dev(i, file, dev) \
- for (i = 0; \
i < cd_count[file] && (dev = console_devices[file][i]); \
i++)
int iomux_match_device(struct stdio_dev **, const int, struct stdio_dev *); int iomux_doenv(const int, const char *); -- 2.35.1

On Wed, Apr 06, 2022 at 02:36:35PM -0400, Sean Anderson wrote:
We should only access console_devices[file][i] once we have checked that i < cd_count[file]. Otherwise, we will access uninitialized memory at the end of the loop. console_devices[file][i] should not be NULL, but putting the assignment in the loop condition allows us to ensure that i is checked beforehand. This isn't a bug, but it does make valgrind stop complaining.
Fixes: 400797cad3 ("IOMUX: Split out for_each_console_dev() helper macro") Signed-off-by: Sean Anderson seanga2@gmail.com Reviewed-by: Andrew Scull ascull@google.com Acked-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
Applied to u-boot/master, thanks!
participants (3)
-
Andy Shevchenko
-
Sean Anderson
-
Tom Rini