Fwd: New Defects reported by Coverity Scan for Das U-Boot
Hey all,
Here's the latest Coverity scan report.
---------- Forwarded message --------- From: scan-admin@coverity.com Date: Mon, Jan 22, 2024 at 6:26 PM Subject: New Defects reported by Coverity Scan for Das U-Boot To: tom.rini@gmail.com
Hi,
Please find the latest report on new defect(s) introduced to Das U-Boot found with Coverity Scan.
1 new defect(s) introduced to Das U-Boot found with Coverity Scan. 7 defect(s), reported by Coverity Scan earlier, were marked fixed in the recent build analyzed by Coverity Scan.
New defect(s) Reported-by: Coverity Scan Showing 1 of 1 defect(s)
** CID 478860: Code maintainability issues (UNUSED_VALUE) /tools/image-host.c: 359 in fit_image_read_key_iv_data()
________________________________________________________________________________________________________ *** CID 478860: Code maintainability issues (UNUSED_VALUE) /tools/image-host.c: 359 in fit_image_read_key_iv_data() 353 if (ret >= sizeof(filename)) { 354 printf("Can't format the key or IV filename when setting up the cipher: insufficient buffer space\n"); 355 ret = -1; 356 } 357 if (ret < 0) { 358 printf("Can't format the key or IV filename when setting up the cipher: snprintf error\n");
CID 478860: Code maintainability issues (UNUSED_VALUE) Assigning value "-1" to "ret" here, but that stored value is overwritten before it can be used.
359 ret = -1; 360 } 361 362 ret = fit_image_read_data(filename, key_iv_data, expected_size); 363 364 return ret;
----- End forwarded message -----
Hi Tom, sorry about that. Please find attached a patch.
Can you please review?
Thanks, Hugo
Signed-off-by: Hugo Cornelis hugo.cornelis@essensium.com --- tools/image-host.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/image-host.c b/tools/image-host.c index b2a0f2e6d1..06b05dcacb 100644 --- a/tools/image-host.c +++ b/tools/image-host.c @@ -351,12 +351,12 @@ static int fit_image_read_key_iv_data(const char *keydir, const char *key_iv_nam ret = snprintf(filename, sizeof(filename), "%s/%s%s", keydir, key_iv_name, ".bin"); if (ret >= sizeof(filename)) { - printf("Can't format the key or IV filename when setting up the cipher: insufficient buffer space\n"); - ret = -1; + fprintf(stderr, "Can't format the key or IV filename when setting up the cipher: insufficient buffer space\n"); + return -1; } if (ret < 0) { - printf("Can't format the key or IV filename when setting up the cipher: snprintf error\n"); - ret = -1; + fprintf(stderr, "Can't format the key or IV filename when setting up the cipher: snprintf error\n"); + return -1; }
ret = fit_image_read_data(filename, key_iv_data, expected_size);
On 1/23/24 09:15, Hugo Cornelis wrote:
Thanks Hugo for addressing the issue. Could you, please, resend the patch with an appropriate commit message. Please, use scripts/get_maintainers.pl to determine the list of addressees.
Otherwise the patch looks good to me.
The initial assignment in fit_image_read_key_iv_data()
int ret = -1;
is superfluous. You may want to fix that too
It helps to know which patch introduced the problem. Please, add
Fixes: bc01d9ff93f3 ("image-host: refactor and protect for very long filenames")
Best regards
Heinrich
Signed-off-by: Hugo Cornelis hugo.cornelis@essensium.com
tools/image-host.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/image-host.c b/tools/image-host.c index b2a0f2e6d1..06b05dcacb 100644 --- a/tools/image-host.c +++ b/tools/image-host.c @@ -351,12 +351,12 @@ static int fit_image_read_key_iv_data(const char *keydir, const char *key_iv_nam ret = snprintf(filename, sizeof(filename), "%s/%s%s", keydir, key_iv_name, ".bin"); if (ret >= sizeof(filename)) {
printf("Can't format the key or IV filename when setting up the cipher: insufficient buffer space\n");ret = -1;
fprintf(stderr, "Can't format the key or IV filename when setting up the cipher: insufficient buffer space\n");return -1;
printf("Can't format the key or IV filename when setting up the cipher: snprintf error\n");ret = -1;
fprintf(stderr, "Can't format the key or IV filename when setting up the cipher: snprintf error\n");return -1;}
ret = fit_image_read_data(filename, key_iv_data, expected_size);
A recent refactoring in image-host.c messed up the return values of the function that reads the encryptiong keys. This patch fixes this and also makes sure that error output goes to stderr instead of to stdout.
Signed-off-by: Hugo Cornelis hugo.cornelis@essensium.com --- tools/image-host.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/tools/image-host.c b/tools/image-host.c index b2a0f2e6d1..7bfc0cb6b1 100644 --- a/tools/image-host.c +++ b/tools/image-host.c @@ -346,17 +346,17 @@ static int fit_image_read_key_iv_data(const char *keydir, const char *key_iv_nam unsigned char *key_iv_data, int expected_size) { char filename[PATH_MAX]; - int ret = -1; + int ret;
ret = snprintf(filename, sizeof(filename), "%s/%s%s", keydir, key_iv_name, ".bin"); if (ret >= sizeof(filename)) { - printf("Can't format the key or IV filename when setting up the cipher: insufficient buffer space\n"); - ret = -1; + fprintf(stderr, "Can't format the key or IV filename when setting up the cipher: insufficient buffer space\n"); + return -1; } if (ret < 0) { - printf("Can't format the key or IV filename when setting up the cipher: snprintf error\n"); - ret = -1; + fprintf(stderr, "Can't format the key or IV filename when setting up the cipher: snprintf error\n"); + return -1; }
ret = fit_image_read_data(filename, key_iv_data, expected_size);
On Thu, Mar 21, 2024 at 12:22:22PM +0100, Hugo Cornelis wrote:
A recent refactoring in image-host.c messed up the return values of the function that reads the encryptiong keys. This patch fixes this and also makes sure that error output goes to stderr instead of to stdout.
Signed-off-by: Hugo Cornelis hugo.cornelis@essensium.com
Applied to u-boot/master, thanks!
participants (3)
-
Heinrich Schuchardt -
Hugo Cornelis -
Tom Rini