| Message ID | 20210121115221.16058-1-sughosh.ganu@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | mkeficapsule: Free up resources used for adding public key to dtb | expand |
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, >
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,
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(-) -- 2.17.1