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 <[email protected]> >> --- >> 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.
