[PATCH v3 3/3] fs/squashfs: fix dirs->entry leaks on sqfs_search_dir() error paths

[Allan ELKAIM]

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 <richard.genoud at bootlin.com>
Signed-off-by: Allan ELKAIM <allan.elkaim at gmail.com>
---

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