Re: [PATCH] fs/erofs: Fix realloc error handling

[Tom Rini]

On Tue, Nov 11, 2025 at 01:49:30PM +0100, Francois Berder wrote:

> If realloc failed, raw was not freed and thus memory
> was leaked.
> 
> Signed-off-by: Francois Berder <fber...@outlook.fr>
> ---
>  fs/erofs/data.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/erofs/data.c b/fs/erofs/data.c
> index 95b609d8ea8..b58ec6fcc66 100644
> --- a/fs/erofs/data.c
> +++ b/fs/erofs/data.c
> @@ -319,12 +319,15 @@ static int z_erofs_read_data(struct erofs_inode *inode, 
> char *buffer,
>               }
>  
>               if (map.m_plen > bufsize) {
> +                     char *tmp;
> +
>                       bufsize = map.m_plen;
> -                     raw = realloc(raw, bufsize);
> -                     if (!raw) {
> +                     tmp = realloc(raw, bufsize);
> +                     if (!tmp) {
>                               ret = -ENOMEM;
>                               break;
>                       }
> +                     raw = tmp;
>               }
>  
>               ret = z_erofs_read_one_data(inode, &map, raw,

I'm not sure how this changes anything? The function is currently
(snipped for clarity):
static int z_erofs_read_data(struct erofs_inode *inode, char *buffer,
                             erofs_off_t size, erofs_off_t offset)
{
[snip]
        char *raw = NULL;
[snip]
                if (map.m_plen > bufsize) {
                        bufsize = map.m_plen;
                        raw = realloc(raw, bufsize);
                        if (!raw) {
                                ret = -ENOMEM;
                                break;
                        }
                }

                ret = z_erofs_read_one_data(inode, &map, raw,
                                            buffer + end - offset, skip, length,
                                            trimmed);
                if (ret < 0)
                        break;
        }
        if (raw)
                free(raw);
        return ret < 0 ? ret : 0;
}

And per include/malloc.h, calling realloc with a null pointer is the
same as calling malloc. So we had nothing previously allocated to free
later when this failed. How did you find this particular issue? Thanks.

-- 
Tom

signature.asc
Description: PGP signature