* [PATCH] image: Fix potentially uninitialized data variable
@ 2023-02-27 19:56 Marek Vasut
2023-03-01 16:02 ` Tom Rini
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Marek Vasut @ 2023-02-27 19:56 UTC (permalink / raw)
To: u-boot
Cc: Marek Vasut, Heinrich Schuchardt, Michal Simek,
Oleksandr Suvorov, Simon Glass, Stefan Roese, Tom Rini
In case fitImage support is disabled, and image_locate_script() is
passed a fitImage, then the 'data' variable is used uninitialized.
Drop into the default: branch of the switch-case statement and do
not return the uninitialized data, and do not modify the return
pointer either, just print an error message.
Reported by clang build:
"
$ make HOSTCC=clang CC=clang KCFLAGS=-Werror sandbox64_defconfig && make HOSTCC=clang CC=clang KCFLAGS=-Werror
...
boot/image-board.c:1006:7: error: variable 'data' is used uninitialized whenever switch case is taken [-Werror,-Wsometimes-uninitialized]
case IMAGE_FORMAT_LEGACY:
^~~~~~~~~~~~~~~~~~~
include/image.h:608:29: note: expanded from macro 'IMAGE_FORMAT_LEGACY'
^~~~
boot/image-board.c:1128:19: note: uninitialized use occurs here
*datap = (char *)data;
^~~~
boot/image-board.c:1001:11: note: initialize the variable 'data' to silence this warning
u32 *data;
^
= NULL
"
Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Cc: Michal Simek <michal.simek@amd.com>
Cc: Oleksandr Suvorov <oleksandr.suvorov@foundries.io>
Cc: Simon Glass <sjg@chromium.org>
Cc: Stefan Roese <sr@denx.de>
Cc: Tom Rini <trini@konsulko.com>
---
boot/image-board.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/boot/image-board.c b/boot/image-board.c
index 25b60ec30b3..9bf70824cb7 100644
--- a/boot/image-board.c
+++ b/boot/image-board.c
@@ -1004,7 +1004,9 @@ int image_locate_script(void *buf, int size, const char *fit_uname,
switch (genimg_get_format(buf)) {
case IMAGE_FORMAT_LEGACY:
- if (IS_ENABLED(CONFIG_LEGACY_IMAGE_FORMAT)) {
+ if (!IS_ENABLED(CONFIG_LEGACY_IMAGE_FORMAT)) {
+ goto exit_image_format;
+ } else {
hdr = buf;
if (!image_check_magic(hdr)) {
@@ -1047,7 +1049,9 @@ int image_locate_script(void *buf, int size, const char *fit_uname,
}
break;
case IMAGE_FORMAT_FIT:
- if (IS_ENABLED(CONFIG_FIT)) {
+ if (!IS_ENABLED(CONFIG_FIT)) {
+ goto exit_image_format;
+ } else {
fit_hdr = buf;
if (fit_check_format(fit_hdr, IMAGE_SIZE_INVAL)) {
puts("Bad FIT image format\n");
@@ -1121,12 +1125,15 @@ fallback:
}
break;
default:
- puts("Wrong image format for \"source\" command\n");
- return -EPERM;
+ goto exit_image_format;
}
*datap = (char *)data;
*lenp = len;
return 0;
+
+exit_image_format:
+ puts("Wrong image format for \"source\" command\n");
+ return -EPERM;
}
--
2.39.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] image: Fix potentially uninitialized data variable
2023-02-27 19:56 [PATCH] image: Fix potentially uninitialized data variable Marek Vasut
@ 2023-03-01 16:02 ` Tom Rini
2023-03-02 3:18 ` Marek Vasut
2023-03-01 23:38 ` Simon Glass
2023-03-31 14:16 ` Tom Rini
2 siblings, 1 reply; 5+ messages in thread
From: Tom Rini @ 2023-03-01 16:02 UTC (permalink / raw)
To: Marek Vasut
Cc: u-boot, Heinrich Schuchardt, Michal Simek, Oleksandr Suvorov,
Simon Glass, Stefan Roese
[-- Attachment #1: Type: text/plain, Size: 1419 bytes --]
On Mon, Feb 27, 2023 at 08:56:31PM +0100, Marek Vasut wrote:
> In case fitImage support is disabled, and image_locate_script() is
> passed a fitImage, then the 'data' variable is used uninitialized.
> Drop into the default: branch of the switch-case statement and do
> not return the uninitialized data, and do not modify the return
> pointer either, just print an error message.
>
> Reported by clang build:
> "
> $ make HOSTCC=clang CC=clang KCFLAGS=-Werror sandbox64_defconfig && make HOSTCC=clang CC=clang KCFLAGS=-Werror
> ...
> boot/image-board.c:1006:7: error: variable 'data' is used uninitialized whenever switch case is taken [-Werror,-Wsometimes-uninitialized]
> case IMAGE_FORMAT_LEGACY:
> ^~~~~~~~~~~~~~~~~~~
> include/image.h:608:29: note: expanded from macro 'IMAGE_FORMAT_LEGACY'
> ^~~~
> boot/image-board.c:1128:19: note: uninitialized use occurs here
> *datap = (char *)data;
> ^~~~
> boot/image-board.c:1001:11: note: initialize the variable 'data' to silence this warning
> u32 *data;
> ^
> = NULL
> "
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
Reviewed-by: Tom Rini <trini@konsulko.com>
And can you please do a follow-up that adds sandbox64 + clang, after the
sandbox + clang test in GitLab/Azure?
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] image: Fix potentially uninitialized data variable
2023-02-27 19:56 [PATCH] image: Fix potentially uninitialized data variable Marek Vasut
2023-03-01 16:02 ` Tom Rini
@ 2023-03-01 23:38 ` Simon Glass
2023-03-31 14:16 ` Tom Rini
2 siblings, 0 replies; 5+ messages in thread
From: Simon Glass @ 2023-03-01 23:38 UTC (permalink / raw)
To: Marek Vasut
Cc: u-boot, Heinrich Schuchardt, Michal Simek, Oleksandr Suvorov,
Stefan Roese, Tom Rini
On Mon, 27 Feb 2023 at 12:56, Marek Vasut
<marek.vasut+renesas@mailbox.org> wrote:
>
> In case fitImage support is disabled, and image_locate_script() is
> passed a fitImage, then the 'data' variable is used uninitialized.
> Drop into the default: branch of the switch-case statement and do
> not return the uninitialized data, and do not modify the return
> pointer either, just print an error message.
>
> Reported by clang build:
> "
> $ make HOSTCC=clang CC=clang KCFLAGS=-Werror sandbox64_defconfig && make HOSTCC=clang CC=clang KCFLAGS=-Werror
> ...
> boot/image-board.c:1006:7: error: variable 'data' is used uninitialized whenever switch case is taken [-Werror,-Wsometimes-uninitialized]
> case IMAGE_FORMAT_LEGACY:
> ^~~~~~~~~~~~~~~~~~~
> include/image.h:608:29: note: expanded from macro 'IMAGE_FORMAT_LEGACY'
> ^~~~
> boot/image-board.c:1128:19: note: uninitialized use occurs here
> *datap = (char *)data;
> ^~~~
> boot/image-board.c:1001:11: note: initialize the variable 'data' to silence this warning
> u32 *data;
> ^
> = NULL
> "
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> ---
> Cc: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> Cc: Michal Simek <michal.simek@amd.com>
> Cc: Oleksandr Suvorov <oleksandr.suvorov@foundries.io>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Stefan Roese <sr@denx.de>
> Cc: Tom Rini <trini@konsulko.com>
> ---
> boot/image-board.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
Reviewed-by: Simon Glass <sjg@chromium.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] image: Fix potentially uninitialized data variable
2023-03-01 16:02 ` Tom Rini
@ 2023-03-02 3:18 ` Marek Vasut
0 siblings, 0 replies; 5+ messages in thread
From: Marek Vasut @ 2023-03-02 3:18 UTC (permalink / raw)
To: Tom Rini, Marek Vasut
Cc: u-boot, Heinrich Schuchardt, Michal Simek, Oleksandr Suvorov,
Simon Glass, Stefan Roese
On 3/1/23 17:02, Tom Rini wrote:
> On Mon, Feb 27, 2023 at 08:56:31PM +0100, Marek Vasut wrote:
>
>> In case fitImage support is disabled, and image_locate_script() is
>> passed a fitImage, then the 'data' variable is used uninitialized.
>> Drop into the default: branch of the switch-case statement and do
>> not return the uninitialized data, and do not modify the return
>> pointer either, just print an error message.
>>
>> Reported by clang build:
>> "
>> $ make HOSTCC=clang CC=clang KCFLAGS=-Werror sandbox64_defconfig && make HOSTCC=clang CC=clang KCFLAGS=-Werror
>> ...
>> boot/image-board.c:1006:7: error: variable 'data' is used uninitialized whenever switch case is taken [-Werror,-Wsometimes-uninitialized]
>> case IMAGE_FORMAT_LEGACY:
>> ^~~~~~~~~~~~~~~~~~~
>> include/image.h:608:29: note: expanded from macro 'IMAGE_FORMAT_LEGACY'
>> ^~~~
>> boot/image-board.c:1128:19: note: uninitialized use occurs here
>> *datap = (char *)data;
>> ^~~~
>> boot/image-board.c:1001:11: note: initialize the variable 'data' to silence this warning
>> u32 *data;
>> ^
>> = NULL
>> "
>>
>> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
>
> Reviewed-by: Tom Rini <trini@konsulko.com>
>
> And can you please do a follow-up that adds sandbox64 + clang, after the
> sandbox + clang test in GitLab/Azure?
The CI is chewing on it here:
ahttps://source.denx.de/u-boot/custodians/u-boot-sh/-/commit/f80a434c2ea4fdae7adf0e4c4a0ecd9784d6808e
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] image: Fix potentially uninitialized data variable
2023-02-27 19:56 [PATCH] image: Fix potentially uninitialized data variable Marek Vasut
2023-03-01 16:02 ` Tom Rini
2023-03-01 23:38 ` Simon Glass
@ 2023-03-31 14:16 ` Tom Rini
2 siblings, 0 replies; 5+ messages in thread
From: Tom Rini @ 2023-03-31 14:16 UTC (permalink / raw)
To: Marek Vasut
Cc: u-boot, Heinrich Schuchardt, Michal Simek, Oleksandr Suvorov,
Simon Glass, Stefan Roese
[-- Attachment #1: Type: text/plain, Size: 1386 bytes --]
On Mon, Feb 27, 2023 at 08:56:31PM +0100, Marek Vasut wrote:
> In case fitImage support is disabled, and image_locate_script() is
> passed a fitImage, then the 'data' variable is used uninitialized.
> Drop into the default: branch of the switch-case statement and do
> not return the uninitialized data, and do not modify the return
> pointer either, just print an error message.
>
> Reported by clang build:
> "
> $ make HOSTCC=clang CC=clang KCFLAGS=-Werror sandbox64_defconfig && make HOSTCC=clang CC=clang KCFLAGS=-Werror
> ...
> boot/image-board.c:1006:7: error: variable 'data' is used uninitialized whenever switch case is taken [-Werror,-Wsometimes-uninitialized]
> case IMAGE_FORMAT_LEGACY:
> ^~~~~~~~~~~~~~~~~~~
> include/image.h:608:29: note: expanded from macro 'IMAGE_FORMAT_LEGACY'
> ^~~~
> boot/image-board.c:1128:19: note: uninitialized use occurs here
> *datap = (char *)data;
> ^~~~
> boot/image-board.c:1001:11: note: initialize the variable 'data' to silence this warning
> u32 *data;
> ^
> = NULL
> "
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> Reviewed-by: Tom Rini <trini@konsulko.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
Applied to u-boot/next, thanks!
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-03-31 14:20 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-27 19:56 [PATCH] image: Fix potentially uninitialized data variable Marek Vasut
2023-03-01 16:02 ` Tom Rini
2023-03-02 3:18 ` Marek Vasut
2023-03-01 23:38 ` Simon Glass
2023-03-31 14:16 ` Tom Rini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).