[PATCH] fs/erofs: Fix realloc error handling

[Tom Rini]

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.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20251201/dd6755dd/attachment.sig>