[PATCH] fs/erofs: Fix realloc error handling

[Gao Xiang]

Hi,

On Mon, Dec 01, 2025 at 10:55:02AM -0600, Tom Rini wrote:
> On Sat, Nov 29, 2025 at 12:12:23PM +0100, Francois Berder wrote:
> > Hello Tom,
> > 
> > On 11/18/25 22:33, Tom Rini wrote:
> > > 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 <fberder at 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.
> > > 
> > 
> > I found this issue (and most bug fixes I submitted recently) by running
> > cppcheck and scan-build against u-boot source code and triaging warnings reported by these tools.
> > 
> > I think my fix should still be applied. Looking at z_erofs_map_blocks_iter and z_erofs_do_map_blocks, I cannot guarantee that map.m_plen does not increase during the loop execution. Hence, realloc could be called several times and we need to properly handle realloc failure.
> 
> Thanks for explaining more. Yes, the loop part was what I missed on my
> earlier look over the code.
> 

Just saw this, actually this issue has been fixed in erofs-utils
commit a3a75f7af7b2 ("erofs-utils: misc: Fix potential memory leak
in realloc failure path")

I think it's fine to merge this fix now, and the mid-term plan is to
sync the erofs-utils codebase again for recent features and bugfixes.

Thanks,
Gao Xiang

> -- 
> Tom