Hi Anton, On 2026-05-21T19:13:58, Anton Moryakov <[email protected]> wrote: > tools: fit_common: fix memory leak in mmap_fdt error path > > Static analysis reported that memory mapped by mmap() is not unmapped > in the error path of mmap_fdt(). > > If mmap() succeeds but a subsequent operation fails (fstat, ftruncate, > fdt_check_header, or fdt_open_into), the function jumps to the err: > label which closes the file descriptor but does not call munmap() on > the mapped region, causing a memory leak. > > Fix: add munmap(ptr, sbuf->st_size) in the error path, guarded by > ptr != MAP_FAILED to avoid unmapping an invalid pointer. > > Signed-off-by: Anton Moryakov <[email protected]> > > tools/fit_common.c | 2 ++ > 1 file changed, 2 insertions(+)
> diff --git a/tools/fit_common.c b/tools/fit_common.c > @@ -123,6 +123,8 @@ err: > close(fd); > if (delete_on_error) > unlink(fname); > + if (ptr != MAP_FAILED) > + munmap(ptr, sbuf->st_size); > > return -1; > } Not quite: ptr is declared uninitialised at the top of mmap_fdt() and is only assigned by the mmap() call on line 90. The earlier error paths (open(), fstat(), ftruncate()) all goto err before ptr has been written, so this new check reads an indeterminate value - undefined behaviour, and may call munmap() on garbage. Please initialise it at declaration: void *ptr = MAP_FAILED; sbuf->st_size is only valid after fstat() has succeeded, but MAP_FAILED as the sentinel covers that, since ptr only becomes non-MAP_FAILED after fstat() and (optionally) ftruncate() have populated sbuf->st_size. BTW, since mmap_fdt() is only used by short-lived host tools (mkimage, fit_info, fit_check_sign, etc) that exit shortly after, the kernel reclaims the mapping anyway - this is a static-analysis cleanup rather than a real leak. Worth mentioning in the commit message. Regards, Simon
