[PATCH v1] erofs-utils: misc: Fix potential memory leak in realloc failure path

[Gao Xiang]

Hi Sandeep,

On 2024/7/19 04:22, Sandeep Dhavale wrote:
> As realloc returns NULL on failure, the original value will be
> overwritten if it is used as lvalue. Fix this by using a temporary
> variable to hold the return value and exit with -ENOMEM in case of
> failure. This patch fixes 2 of the realloc blocks with similar fix.

Thanks for the patch!

Is it a lint issue?  Although I think currently userspace malloc
failures is rare, it'd be better to handle them.

> 
> Signed-off-by: Sandeep Dhavale <dhavale at google.com>
> ---
>   fsck/main.c | 9 +++++++--
>   lib/data.c  | 5 +++--
>   2 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/fsck/main.c b/fsck/main.c
> index 8ec9486..75950b6 100644
> --- a/fsck/main.c
> +++ b/fsck/main.c
> @@ -508,8 +508,13 @@ static int erofs_verify_inode_data(struct erofs_inode *inode, int outfd)
>   		if (compressed) {
>   			if (map.m_llen > buffer_size) {
>   				buffer_size = map.m_llen;
> -				buffer = realloc(buffer, buffer_size);
> -				BUG_ON(!buffer);
> +				char *newbuffer = realloc(buffer, buffer_size);
> +
> +				if (!newbuffer) {
> +					ret = -ENOMEM;
> +					goto out;
> +				}
> +				buffer = newbuffer;

I update this as since the variable definition would be better
at the beginning of a block...

diff --git a/fsck/main.c b/fsck/main.c
index 75950b6..fb66967 100644
--- a/fsck/main.c
+++ b/fsck/main.c
@@ -507,9 +507,10 @@ static int erofs_verify_inode_data(struct erofs_inode *inode, int outfd)

                 if (compressed) {
                         if (map.m_llen > buffer_size) {
-                               buffer_size = map.m_llen;
-                               char *newbuffer = realloc(buffer, buffer_size);
+                               char *newbuffer;

+                               buffer_size = map.m_llen;
+                               newbuffer = realloc(buffer, buffer_size);
                                 if (!newbuffer) {
                                         ret = -ENOMEM;
                                         goto out;

Thanks,
Gao Xiang