[scan-admin at coverity.com: New Defects reported by Coverity Scan for Das U-Boot]

[Heinrich Schuchardt]

On 21.01.21 12:36, Sughosh Ganu wrote:
>
>
> On Thu, 21 Jan 2021 at 00:34, Tom Rini <trini at konsulko.com
> <mailto:trini at konsulko.com>> wrote:
>
>     I decided to run Coverity part-way through the merge window this time
>     and here's what's been found so far.
>
>     ----- Forwarded message from scan-admin at coverity.com
>     <mailto:scan-admin at coverity.com> -----
>
>     Date: Mon, 18 Jan 2021 17:53:19 +0000 (UTC)
>     From: scan-admin at coverity.com <mailto:scan-admin at coverity.com>
>     To: tom.rini at gmail.com <mailto:tom.rini at gmail.com>
>     Subject: New Defects reported by Coverity Scan for Das U-Boot
>
>     Hi,
>
>     Please find the latest report on new defect(s) introduced to Das
>     U-Boot found with Coverity Scan.
>
>     23 new defect(s) introduced to Das U-Boot found with Coverity Scan.
>     2 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 20 of 23 defect(s)
>
>     ** CID 316356:  Resource leaks  (RESOURCE_LEAK)
>     /tools/mkeficapsule.c: 225 in add_public_key()
>
>
> <snip>
>  
>
>
>     ________________________________________________________________________________________________________
>     *** CID 316356:  Resource leaks  (RESOURCE_LEAK)
>     /tools/mkeficapsule.c: 225 in add_public_key()
>     219             if (ret < 0) {
>     220                     fprintf(stderr, "%s: Unable to add public
>     key to the FDT\n",
>     221                             __func__);
>     222                     goto err;
>     223             }
>     224     
>     >>>     CID 316356:  Resource leaks  (RESOURCE_LEAK)
>     >>>     Handle variable "srcfd" going out of scope leaks the handle.
>     225             return 0;
>     226     
>     227     err:
>     228             if (sptr)
>     229                     munmap(sptr, src_size);
>     230     
>
>
> I think these should not cause any issues, since the function return
> results in the process termination in both the scenarios of success and
> failure. But i will post a patch to handle these errors to keep the
> resource handling consistent.

Looking at line 234f:

        if (srcfd >= 0)
                close(srcfd);

The comparison is wrong. It should be:

        if (srcfd != -1)
                close(srcfd);

The open.2 man-page says that only -1 signals an error. According to the
man-page -2 is a legal value for a file descriptor.

The initialization of destfd is wrong:

141:
	int destfd = 0;

In case of an error opening srcfd this leads to closing file descriptor
0 which relates to the console input. You should use:

	int destfd = -1;

and

        if (destfd != -1)
                close(destfd);

Best regards

Heinrich