Several error paths in sqfs_search_dir() return through 'goto out' while a directory entry obtained from sqfs_readdir_nest() is still held, leaking dirs->entry: the inode lookup failure, the symlink nesting limit check, every allocation/tokenization failure during symlink resolution, and the case where readdir aborts after an entry was already read.
Instead of freeing dirs->entry at each error site, centralize the cleanup at the 'out' label: on error, no valid entry may be handed back to the caller, so it can be freed unconditionally there. On success, dirs->entry is already NULL: it is freed at the end of each token iteration and before recursing into a symlink target, and the root directory path never allocates it. Explicit frees remain only where a success path needs them: between reads in the readdir loop, at the end of each token iteration, and before the recursive call. The now-redundant frees on individual error paths are removed. Suggested-by: Richard Genoud <[email protected]> Signed-off-by: Allan ELKAIM <[email protected]> --- Changes in v3: - Centralize dirs->entry cleanup at the 'out' label instead of freeing only on the inode lookup failure path, covering the remaining leaky error paths (suggested by Richard Genoud) fs/squashfs/sqfs.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c index af32d008..471ea7d9 100644 --- a/fs/squashfs/sqfs.c +++ b/fs/squashfs/sqfs.c @@ -547,8 +547,10 @@ 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) { + ret = -EINVAL; + goto out; + } dir = (struct squashfs_dir_inode *)table; /* Check for symbolic link and inode type sanity */ @@ -617,8 +619,6 @@ static int sqfs_search_dir(struct squashfs_dir_stream *dirs, char **token_list, goto out; } else if (!sqfs_is_dir(get_unaligned_le16(&dir->inode_type))) { printf("** Cannot find directory. **\n"); - free(dirs->entry); - dirs->entry = NULL; ret = -EINVAL; goto out; } @@ -630,8 +630,6 @@ static int sqfs_search_dir(struct squashfs_dir_stream *dirs, char **token_list, /* Get dir. offset into the directory table */ offset = sqfs_dir_offset(table, m_list, m_count); if (offset < 0) { - free(dirs->entry); - dirs->entry = NULL; ret = offset; goto out; } @@ -644,8 +642,6 @@ static int sqfs_search_dir(struct squashfs_dir_stream *dirs, char **token_list, /* Check for empty directory */ if (sqfs_is_empty_dir(table)) { printf("Empty directory.\n"); - free(dirs->entry); - dirs->entry = NULL; ret = SQFS_EMPTY_DIR; goto out; } @@ -660,8 +656,6 @@ static int sqfs_search_dir(struct squashfs_dir_stream *dirs, char **token_list, offset = sqfs_dir_offset(table, m_list, m_count); if (offset < 0) { - free(dirs->entry); - dirs->entry = NULL; ret = offset; goto out; } @@ -673,6 +667,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); -- 2.53.0
