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

[Yao Zi <ziyao@disroot.org>]

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