From 9a9d46cb5e1a8f600c52f3ddaeaca8d4f28f66ee Mon Sep 17 00:00:00 2001 From: Allan ELKAIM Date: Mon, 13 Jul 2026 16:22:43 +0200 Subject: fs/squashfs: fix heap exhaustion during symlink resolution When sqfs_read_nest() encounters a symlink it resolves it by calling itself recursively. In the unfixed code this looks like: // dirsp is open: inode_table + dir_table still on heap resolved = sqfs_resolve_symlink(symlink, filename); ret = sqfs_read_nest(resolved, ...); // recursive: allocates a new // inode_table + dir_table pair free(resolved); goto out; // out: sqfs_closedir(dirsp) <- parent tables freed HERE, too late There is no permanent leak: the parent's tables are freed at the out: label once the recursive call returns. However, for the entire duration of the recursive call both the parent's inode_table + dir_table and the child's inode_table + dir_table are live on the heap simultaneously. On large squashfs images these tables can be significant in size, and this temporary double allocation may exhaust the heap budget. A superficial workaround would be to increase CONFIG_SYS_MALLOC_LEN, but that wastes memory on all boards and does not address the structural problem. The correct fix is to change the freeing order: release the parent directory's resources before recursing. This way only one set of inode and directory tables is live at any given time, halving the peak heap usage during symlink resolution. When heap exhaustion does occur and malloc returns NULL for dir_table or pos_list inside sqfs_read_directory_table(), the failure is currently silent and cascading: - metablks_count is not reset to -1 before the goto out, so the function returns a positive block count alongside a NULL pointer. - sqfs_opendir_nest() does not detect the failure (it only checks metablks_count < 1) and calls sqfs_search_dir() with m_list=NULL. - sqfs_dir_offset() iterates over m_list[0..n], reading from addresses 0x0, 0x4, 0x8, ... None of those values match the inode's start_block, so the function returns -EINVAL. - The error propagates up as a load failure with no indication that the root cause was heap exhaustion: Error: invalid inode reference to directory table. Failed to load '' Two fixes: 1. In sqfs_read_directory_table(), set metablks_count = -1 whenever malloc fails after sqfs_count_metablks() returns a positive value, so that the caller's "metablks_count < 1" check correctly detects the failure and avoids calling sqfs_search_dir() with a NULL pos_list. 2. In sqfs_read_nest() and sqfs_size_nest(), call sqfs_closedir() on the parent dirsp before the recursive call so that the parent's inode and directory tables are freed before the child allocates its own. Only one set of tables is then live at any given time, halving peak heap usage during symlink resolution. Link: https://lists.denx.de/pipermail/u-boot/2026-May/618533.html Reviewed-by: Richard Genoud Acked-by: Miquel Raynal Signed-off-by: Allan ELKAIM --- fs/squashfs/sqfs.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c index 9cb8b4afcdd..07e2bd82561 100644 --- a/fs/squashfs/sqfs.c +++ b/fs/squashfs/sqfs.c @@ -853,12 +853,16 @@ static int sqfs_read_directory_table(unsigned char **dir_table, u32 **pos_list) goto out; *dir_table = malloc(metablks_count * SQFS_METADATA_BLOCK_SIZE); - if (!*dir_table) + if (!*dir_table) { + metablks_count = -1; goto out; + } *pos_list = malloc(metablks_count * sizeof(u32)); - if (!*pos_list) + if (!*pos_list) { + metablks_count = -1; goto out; + } ret = sqfs_get_metablk_pos(*pos_list, dtb, table_offset, metablks_count); @@ -1473,6 +1477,15 @@ static int sqfs_read_nest(const char *filename, void *buf, loff_t offset, symlink = (struct squashfs_symlink_inode *)ipos; resolved = sqfs_resolve_symlink(symlink, filename); + /* + * Free the parent directory resources before recursing so that + * the recursive call can allocate its own inode and directory + * tables without exhausting the heap. + */ + free(dirs->entry); + dirs->entry = NULL; + sqfs_closedir(dirsp); + dirsp = NULL; ret = sqfs_read_nest(resolved, buf, offset, len, actread); free(resolved); goto out; @@ -1731,6 +1744,11 @@ static int sqfs_size_nest(const char *filename, loff_t *size) symlink = (struct squashfs_symlink_inode *)ipos; resolved = sqfs_resolve_symlink(symlink, filename); + /* + * Free the parent directory resources before recursing. + */ + sqfs_closedir(dirsp); + dirsp = NULL; ret = sqfs_size(resolved, size); free(resolved); break; -- cgit v1.3.1 From 57e0bb7bf00dadd7537f93609afb955108ce22c7 Mon Sep 17 00:00:00 2001 From: Allan ELKAIM Date: Mon, 13 Jul 2026 16:22:45 +0200 Subject: fs/squashfs: add sqfs_dir_offset() error checks sqfs_dir_offset() returns a negative errno on failure, but three call sites in sqfs_search_dir() use the return value as an array index without checking for errors first. If the lookup fails, dirs->table is set to an invalid address, leading to undefined behavior. Add negative-value guards after each sqfs_dir_offset() call so that any lookup failure propagates cleanly as an error rather than producing incorrect results. Note: the corresponding sqfs_find_inode() NULL checks and the heap exhaustion fix during symlink resolution are applied in separate patches. Acked-by: Miquel Raynal Reviewed-by: Richard Genoud Signed-off-by: Allan ELKAIM --- fs/squashfs/sqfs.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c index 07e2bd82561..af32d008e30 100644 --- a/fs/squashfs/sqfs.c +++ b/fs/squashfs/sqfs.c @@ -496,6 +496,8 @@ static int sqfs_search_dir(struct squashfs_dir_stream *dirs, char **token_list, /* get directory offset in directory table */ offset = sqfs_dir_offset(table, m_list, m_count); + if (offset < 0) + return offset; dirs->table = &dirs->dir_table[offset]; /* Setup directory header */ @@ -627,6 +629,12 @@ 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; + } dirs->table = &dirs->dir_table[offset]; /* Copy directory header */ @@ -651,6 +659,12 @@ 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; + } dirs->table = &dirs->dir_table[offset]; if (get_unaligned_le16(&dir->inode_type) == SQFS_DIR_TYPE) -- cgit v1.3.1 From 171b604888537dd7112ffddaa13abb16932eabd0 Mon Sep 17 00:00:00 2001 From: Allan ELKAIM Date: Mon, 13 Jul 2026 16:22:47 +0200 Subject: fs/squashfs: fix dirs->entry leaks on sqfs_search_dir() error paths 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 Signed-off-by: Allan ELKAIM --- 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 af32d008e30..471ea7d9df8 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); -- cgit v1.3.1