[PATCH] tools: Fix handle leak in mmap_fdt function

[PATCH] tools: Fix handle leak in mmap_fdt function

[ant.v.moryakov]

From: Maks Mishin <maks.mishinFZ@gmail.com>

The handle 'ptr' is created at fit_common.c:91 by calling 
function 'mmap' and lost at fit_common.c:127: 
Added call of free for `ptr` if ptr != MAP_FAILED.

Trigger was found by the Svace static analyzer.

Signed-off-by: Maks Mishin <maks.mishinFZ@gmail.com>
---
 tools/fit_common.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/fit_common.c b/tools/fit_common.c
index d1cde16c1c..135e105929 100644
--- a/tools/fit_common.c
+++ b/tools/fit_common.c
@@ -123,6 +123,7 @@ err:
 		close(fd);
 	if (delete_on_error)
 		unlink(fname);
+	free(ptr);
 
 	return -1;
 }
-- 
2.34.1

Re: [PATCH] tools: Fix handle leak in mmap_fdt function

[Yao Zi]

On Fri, Apr 18, 2025 at 11:15:23AM +0300, ant.v.moryakov@gmail.com wrote:
> From: Maks Mishin <maks.mishinFZ@gmail.com>
> 
> The handle 'ptr' is created at fit_common.c:91 by calling 
> function 'mmap' and lost at fit_common.c:127: 
> Added call of free for `ptr` if ptr != MAP_FAILED.

First, why do you free() a mmapped memory chunk? This really sounds
unreasonable to me.

Second, you don't validate ptr before freeing it actually. It's possible
to branch to label "err" before ptr is initialized, thus you must guard
the free in case of garbage data in ptr (or just initialize ptr to NULL).

> Trigger was found by the Svace static analyzer.
> 
> Signed-off-by: Maks Mishin <maks.mishinFZ@gmail.com>
> ---
>  tools/fit_common.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/fit_common.c b/tools/fit_common.c
> index d1cde16c1c..135e105929 100644
> --- a/tools/fit_common.c
> +++ b/tools/fit_common.c
> @@ -123,6 +123,7 @@ err:
>  		close(fd);
>  	if (delete_on_error)
>  		unlink(fname);
> +	free(ptr);
>  
>  	return -1;
>  }
> -- 
> 2.34.1

Thanks,
Yao Zi

[PATCH] tools: Fix handle leak in mmap_fdt function

[Maks Mishin]

The handle 'ptr' is created at fit_common.c:91 by calling 
function 'mmap' and lost at fit_common.c:127: 
Added call of free for `ptr` if ptr != MAP_FAILED.

Trigger was found by the Svace static analyzer.

Signed-off-by: Maks Mishin <maks.mishinFZ@gmail.com>
---
 tools/fit_common.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/fit_common.c b/tools/fit_common.c
index d1cde16c1c..135e105929 100644
--- a/tools/fit_common.c
+++ b/tools/fit_common.c
@@ -123,6 +123,7 @@ err:
 		close(fd);
 	if (delete_on_error)
 		unlink(fname);
+	free(ptr);
 
 	return -1;
 }
-- 
2.34.1

Re: [PATCH] tools: Fix handle leak in mmap_fdt function

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1001 bytes --]

On Sun, Feb 09, 2025 at 05:56:41PM +0300, Maks Mishin wrote:

> The handle 'ptr' is created at fit_common.c:91 by calling 
> function 'mmap' and lost at fit_common.c:127: 
> Added call of free for `ptr` if ptr != MAP_FAILED.
> 
> Trigger was found by the Svace static analyzer.
> 
> Signed-off-by: Maks Mishin <maks.mishinFZ@gmail.com>
> ---
>  tools/fit_common.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/fit_common.c b/tools/fit_common.c
> index d1cde16c1c..135e105929 100644
> --- a/tools/fit_common.c
> +++ b/tools/fit_common.c
> @@ -123,6 +123,7 @@ err:
>  		close(fd);
>  	if (delete_on_error)
>  		unlink(fname);
> +	free(ptr);
>  
>  	return -1;
>  }

This introduces:
/home/uboot/u-boot/u-boot/tools/fit_common.c: In function 'mmap_fdt':
/home/uboot/u-boot/u-boot/tools/fit_common.c:126:9: warning: 'ptr' may be used uninitialized in
 this function [-Wmaybe-uninitialized]
  126 |         free(ptr);
      |         ^~~~~~~~~

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]