* [PATCH] mkeficapsule: Free up resources used for adding public key to dtb
@ 2021-01-21 11:52 Sughosh Ganu
2021-01-21 13:32 ` Heinrich Schuchardt
0 siblings, 1 reply; 2+ messages in thread
From: Sughosh Ganu @ 2021-01-21 11:52 UTC (permalink / raw)
To: u-boot
Fix the issues flagged by Coverity on resources not being released in
the add_public_key function.
Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---
tools/mkeficapsule.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
index 270943fc90..caf0a1b231 100644
--- a/tools/mkeficapsule.c
+++ b/tools/mkeficapsule.c
@@ -150,6 +150,7 @@ static int add_public_key(const char *pkey_file, const char *dtb_file,
if (srcfd == -1) {
fprintf(stderr, "%s: Can't open %s: %s\n",
__func__, pkey_file, strerror(errno));
+ ret = -1;
goto err;
}
@@ -157,6 +158,7 @@ static int add_public_key(const char *pkey_file, const char *dtb_file,
if (ret == -1) {
fprintf(stderr, "%s: Can't stat %s: %s\n",
__func__, pkey_file, strerror(errno));
+ ret = -1;
goto err;
}
@@ -167,6 +169,7 @@ static int add_public_key(const char *pkey_file, const char *dtb_file,
if ((sptr == MAP_FAILED) || (errno != 0)) {
fprintf(stderr, "%s: Failed to mmap %s:%s\n",
__func__, pkey_file, strerror(errno));
+ ret = -1;
goto err;
}
@@ -175,6 +178,7 @@ static int add_public_key(const char *pkey_file, const char *dtb_file,
if (destfd == -1) {
fprintf(stderr, "%s: Can't open %s: %s\n",
__func__, dtb_file, strerror(errno));
+ ret = -1;
goto err;
}
@@ -189,6 +193,7 @@ static int add_public_key(const char *pkey_file, const char *dtb_file,
if (ftruncate(destfd, dtb.st_size)) {
fprintf(stderr, "%s: Can't expand %s: %s\n",
__func__, dtb_file, strerror(errno));
+ ret = -1;
goto err;;
}
@@ -199,11 +204,13 @@ static int add_public_key(const char *pkey_file, const char *dtb_file,
if ((dptr == MAP_FAILED) || (errno != 0)) {
fprintf(stderr, "%s: Failed to mmap %s:%s\n",
__func__, dtb_file, strerror(errno));
+ ret = -1;
goto err;
}
if (fdt_check_header(dptr)) {
fprintf(stderr, "%s: Invalid FDT header\n", __func__);
+ ret = -1;
goto err;
}
@@ -211,6 +218,7 @@ static int add_public_key(const char *pkey_file, const char *dtb_file,
if (ret) {
fprintf(stderr, "%s: Cannot expand FDT: %s\n",
__func__, fdt_strerror(ret));
+ ret = -1;
goto err;
}
@@ -219,10 +227,11 @@ static int add_public_key(const char *pkey_file, const char *dtb_file,
if (ret < 0) {
fprintf(stderr, "%s: Unable to add public key to the FDT\n",
__func__);
+ ret = -1;
goto err;
}
- return 0;
+ ret = 0;
err:
if (sptr)
@@ -237,7 +246,7 @@ err:
if (destfd >= 0)
close(destfd);
- return -1;
+ return ret;
}
static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
--
2.17.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* [PATCH] mkeficapsule: Free up resources used for adding public key to dtb
2021-01-21 11:52 [PATCH] mkeficapsule: Free up resources used for adding public key to dtb Sughosh Ganu
@ 2021-01-21 13:32 ` Heinrich Schuchardt
0 siblings, 0 replies; 2+ messages in thread
From: Heinrich Schuchardt @ 2021-01-21 13:32 UTC (permalink / raw)
To: u-boot
On 21.01.21 12:52, Sughosh Ganu wrote:
> Fix the issues flagged by Coverity on resources not being released in
> the add_public_key function.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
> tools/mkeficapsule.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c
> index 270943fc90..caf0a1b231 100644
> --- a/tools/mkeficapsule.c
> +++ b/tools/mkeficapsule.c
> @@ -150,6 +150,7 @@ static int add_public_key(const char *pkey_file, const char *dtb_file,
> if (srcfd == -1) {
> fprintf(stderr, "%s: Can't open %s: %s\n",
> __func__, pkey_file, strerror(errno));
> + ret = -1;
> goto err;
> }
>
> @@ -157,6 +158,7 @@ static int add_public_key(const char *pkey_file, const char *dtb_file,
> if (ret == -1) {
> fprintf(stderr, "%s: Can't stat %s: %s\n",
> __func__, pkey_file, strerror(errno));
> + ret = -1;
> goto err;
> }
>
> @@ -167,6 +169,7 @@ static int add_public_key(const char *pkey_file, const char *dtb_file,
> if ((sptr == MAP_FAILED) || (errno != 0)) {
> fprintf(stderr, "%s: Failed to mmap %s:%s\n",
> __func__, pkey_file, strerror(errno));
> + ret = -1;
> goto err;
> }
>
> @@ -175,6 +178,7 @@ static int add_public_key(const char *pkey_file, const char *dtb_file,
> if (destfd == -1) {
> fprintf(stderr, "%s: Can't open %s: %s\n",
> __func__, dtb_file, strerror(errno));
> + ret = -1;
> goto err;
> }
>
> @@ -189,6 +193,7 @@ static int add_public_key(const char *pkey_file, const char *dtb_file,
> if (ftruncate(destfd, dtb.st_size)) {
> fprintf(stderr, "%s: Can't expand %s: %s\n",
> __func__, dtb_file, strerror(errno));
> + ret = -1;
> goto err;;
Thanks for providing the patch.
Please, remove the duplicate semicolon here.
main() should return the constant EXIT_FAILURE (glibc defines this as 1)
if a failure occurs, not -1.
scripts/checkpatch.pl -f tools/mkeficapsule.c
reports some other style issues:
CHECK: Unnecessary parentheses around 'sptr == MAP_FAILED'
#167: FILE: tools/mkeficapsule.c:167:
+ if ((sptr == MAP_FAILED) || (errno != 0)) {
CHECK: Unnecessary parentheses around 'errno != 0'
#167: FILE: tools/mkeficapsule.c:167:
+ if ((sptr == MAP_FAILED) || (errno != 0)) {
WARNING: Statements terminations use 1 semicolon
#192: FILE: tools/mkeficapsule.c:192:
+ goto err;;
CHECK: Unnecessary parentheses around 'dptr == MAP_FAILED'
#199: FILE: tools/mkeficapsule.c:199:
+ if ((dptr == MAP_FAILED) || (errno != 0)) {
CHECK: Unnecessary parentheses around 'errno != 0'
#199: FILE: tools/mkeficapsule.c:199:
+ if ((dptr == MAP_FAILED) || (errno != 0)) {
In U-Boot we prefer to avoid terms like (errno != 0) in a logical statement:
if (dptr == MAP_FAILED || errno) {
But it is completely unnecessary to check errno at all here because
errno is only set if mmap() returns MAP_FAILED (see the mmap()
man-page). So the line should be:
192
if (dptr == MAP_FAILED) {
Now you can remove the following line:
195:
- errno = 0;
Best regards
Heinrich
> }
>
> @@ -199,11 +204,13 @@ static int add_public_key(const char *pkey_file, const char *dtb_file,
> if ((dptr == MAP_FAILED) || (errno != 0)) {
> fprintf(stderr, "%s: Failed to mmap %s:%s\n",
> __func__, dtb_file, strerror(errno));
> + ret = -1;
> goto err;
> }
>
> if (fdt_check_header(dptr)) {
> fprintf(stderr, "%s: Invalid FDT header\n", __func__);
> + ret = -1;
> goto err;
> }
>
> @@ -211,6 +218,7 @@ static int add_public_key(const char *pkey_file, const char *dtb_file,
> if (ret) {
> fprintf(stderr, "%s: Cannot expand FDT: %s\n",
> __func__, fdt_strerror(ret));
> + ret = -1;
> goto err;
> }
>
> @@ -219,10 +227,11 @@ static int add_public_key(const char *pkey_file, const char *dtb_file,
> if (ret < 0) {
> fprintf(stderr, "%s: Unable to add public key to the FDT\n",
> __func__);
> + ret = -1;
> goto err;
> }
>
> - return 0;
> + ret = 0;
>
> err:
> if (sptr)
> @@ -237,7 +246,7 @@ err:
> if (destfd >= 0)
> close(destfd);
>
> - return -1;
> + return ret;
> }
>
> static int create_fwbin(char *path, char *bin, efi_guid_t *guid,
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-01-21 13:32 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-21 11:52 [PATCH] mkeficapsule: Free up resources used for adding public key to dtb Sughosh Ganu
2021-01-21 13:32 ` Heinrich Schuchardt
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.