Le 11/06/2026 à 11:43, Allan ELKAIM a écrit :
In sqfs_search_dir(), when sqfs_find_inode() fails to locate the inode
of a directory entry just returned by sqfs_readdir_nest(), the function
returns directly while dirs->entry still holds the entry allocation,
leaking it. The bare return also bypasses the regular error path.
Free the entry and leave through the out label instead, consistent
with the other error paths in this function.
Signed-off-by: Allan ELKAIM <[email protected]>
---
Changes in v2:
- New patch, fixing a pre-existing leak of the same kind as the ones
addressed in patch 2 (suggested by Richard Genoud's review)
fs/squashfs/sqfs.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index af32d008..df988774 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -547,8 +547,12 @@ static int sqfs_search_dir(struct squashfs_dir_stream
*dirs, char **token_list,
/* Get reference to inode in the inode table */
table = sqfs_find_inode(dirs->inode_table, new_inode_number,
sblk->inodes, sblk->block_size);
- if (!table)
- return -EINVAL;
+ if (!table) {
+ free(dirs->entry);
+ dirs->entry = NULL;
+ ret = -EINVAL;
+ goto out;
+ }
dir = (struct squashfs_dir_inode *)table;
/* Check for symbolic link and inode type sanity */
Having a 2nd look at the whole function, it seems that there a lot of:
ret = -Exxx;
goto out;
without freeing dirs->entry
Like here:
[...]
if (++symlinknest == MAX_SYMLINK_NEST) {
ret = -ELOOP;
goto out;
}
sym = (struct squashfs_symlink_inode *)table;
/* Get first j + 1 tokens */
path = sqfs_concat_tokens(token_list, j + 1);
if (!path) {
ret = -ENOMEM;
goto out;
}
/* Resolve for these tokens */
target = sqfs_resolve_symlink(sym, path);
if (!target) {
ret = -ENOMEM;
goto out;
}
[...]
It seems to me that we could keep free(dirs->entry):
- in the while (!sqfs_readdir_nest(dirsp, &dent)) loop
- at the end of the for (j = 0; j < token_count; j++) loop
- and before the recursive call to sqfs_search_dir()
And remove all the rest to add something like:
@@ -677,6 +677,10 @@ static int sqfs_search_dir(struct squashfs_dir_stream
*dirs, char **token_list,
memcpy(&dirs->i_ldir, ldir, sizeof(*ldir));
out:
+ if (ret < 0) {
+ free(dirs->entry);
+ dirs->entry = NULL;
+ }
free(res);
free(rem);
free(path);
(not tested)
What do you think?
Regards,
Richard
