[PATCH v2] erofs-utils: fix memory leaks and allocation issue

[Gao Xiang]

On 2025/9/10 13:05, Yuezhang Mo wrote:
> This patch addresses 4 memory leaks and 1 allocation issue to
> ensure proper cleanup and allocation:
> 
> 1. Fixed memory leak by freeing sbi->zmgr in z_erofs_compress_exit().
> 2. Fixed memory leak by freeing inode->chunkindexes in erofs_iput().
> 3. Fixed incorrect allocation of chunk index array in
>     erofs_rebuild_write_blob_index() to ensure correct array allocation
>     and avoid out-of-bounds access.
> 4. Fixed memory leak of struct erofs_blobchunk not being freed by
>     calling erofs_blob_exit() in rebuild mode.
> 5. Fixed memory leak caused by repeated initialization of the first
>     blob device's sbi by checking whether sbi has been initialized.
> 
> Signed-off-by: Yuezhang Mo <Yuezhang.Mo at sony.com>
> Reviewed-by: Friendy Su <friendy.su at sony.com>
> Reviewed-by: Daniel Palmer <daniel.palmer at sony.com>
> ---
>   lib/compress.c |  2 ++
>   lib/inode.c    |  3 +++
>   lib/rebuild.c  | 13 ++++++++-----
>   mkfs/main.c    |  2 +-
>   4 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/compress.c b/lib/compress.c
> index 622a205..dd537ec 100644
> --- a/lib/compress.c
> +++ b/lib/compress.c
> @@ -2171,5 +2171,7 @@ int z_erofs_compress_exit(struct erofs_sb_info *sbi)
>   		}
>   #endif
>   	}
> +
> +	free(sbi->zmgr);
>   	return 0;
>   }
> diff --git a/lib/inode.c b/lib/inode.c
> index 7ee6b3d..38e2bb2 100644
> --- a/lib/inode.c
> +++ b/lib/inode.c
> @@ -159,6 +159,9 @@ unsigned int erofs_iput(struct erofs_inode *inode)
>   	} else {
>   		free(inode->i_link);
>   	}
> +
> +	if (inode->datalayout == EROFS_INODE_CHUNK_BASED)
> +		free(inode->chunkindexes);
>   	free(inode);
>   	return 0;
>   }
> diff --git a/lib/rebuild.c b/lib/rebuild.c
> index 0358567..461e18e 100644
> --- a/lib/rebuild.c
> +++ b/lib/rebuild.c
> @@ -186,7 +186,7 @@ static int erofs_rebuild_write_blob_index(struct erofs_sb_info *dst_sb,
>   
>   	unit = sizeof(struct erofs_inode_chunk_index);
>   	inode->extent_isize = count * unit;
> -	idx = malloc(max(sizeof(*idx), sizeof(void *)));
> +	idx = calloc(count, max(sizeof(*idx), sizeof(void *)));
>   	if (!idx)
>   		return -ENOMEM;
>   	inode->chunkindexes = idx;
> @@ -428,10 +428,13 @@ int erofs_rebuild_load_tree(struct erofs_inode *root, struct erofs_sb_info *sbi,
>   		erofs_uuid_unparse_lower(sbi->uuid, uuid_str);
>   		fsid = uuid_str;
>   	}
> -	ret = erofs_read_superblock(sbi);
> -	if (ret) {
> -		erofs_err("failed to read superblock of %s", fsid);
> -		return ret;
> +
> +	if (!sbi->devs) {

`sbi->devs` may be NULL if ondisk_extradevs == 0? (see
erofs_init_devices()).

I think we could just introduce a new `sbi->sb_valid`
boolean, and set up this in erofs_read_superblock()
instead in the short term.

Thanks,
Gao Xiang

> +		ret = erofs_read_superblock(sbi);
> +		if (ret) {
> +			erofs_err("failed to read superblock of %s", fsid);
> +			return ret;
> +		}
>   	}
>   
>   	inode.nid = sbi->root_nid;
> diff --git a/mkfs/main.c b/mkfs/main.c
> index 28588db..3dd5815 100644
> --- a/mkfs/main.c
> +++ b/mkfs/main.c
> @@ -1908,7 +1908,7 @@ exit:
>   	erofs_dev_close(&g_sbi);
>   	erofs_cleanup_compress_hints();
>   	erofs_cleanup_exclude_rules();
> -	if (cfg.c_chunkbits)
> +	if (cfg.c_chunkbits || source_mode == EROFS_MKFS_SOURCE_REBUILD)
>   		erofs_blob_exit();
>   	erofs_xattr_cleanup_name_prefixes();
>   	erofs_rebuild_cleanup();