[PATCH] fs/squashfs: fix out-of-bounds read while walking the inode table
Richard GENOUD
richard.genoud at bootlin.com
Thu Jun 25 10:11:47 CEST 2026
Hi Hem,
Le 11/06/2026 à 06:21, Hem Parekh a écrit :
> sqfs_find_inode() walks the uncompressed inode table for sblk->inodes
> iterations, advancing 'offset' by sqfs_inode_size() each step. Both the
> inode count and every per-inode size are taken verbatim from the on-disk
> (attacker-controlled) SquashFS image, but the walk was never bounded
> against the end of the heap buffer allocated in sqfs_read_inode_table()
> (metablks_count * SQFS_METADATA_BLOCK_SIZE bytes). A malformed image can
> therefore drive 'base = inode_table + offset' and the field reads in
> sqfs_inode_size() past the buffer, an out-of-bounds read reachable from
> any path lookup (e.g. 'ls'/'load' of a squashfs filesystem).
>
> The SQFS_LDIR_TYPE branch of sqfs_inode_size() makes this worse: it
> loops i_count times reading di->size and advancing 'di' by
> sizeof(*di) + size with no bounds check at all, so a single crafted
> extended-directory inode walks 'di' arbitrarily far past the buffer.
>
> Propagate the inode-table buffer size from sqfs_read_inode_table() into
> the dir stream and on into sqfs_find_inode(), and:
> - reject any base inode whose fixed fields fall outside the table;
> - pass the buffer end to sqfs_inode_size() and bound the LDIR index
> walk against it;
> - reject any computed inode size that pushes 'offset' past the buffer.
>
> Reproduced with a standalone libc + AddressSanitizer harness replicating
> the verbatim sqfs_find_inode()/sqfs_inode_size() logic: the unpatched
> code faults on an OOB read inside the LDIR loop, the patched code rejects
> the malformed inode and returns NULL cleanly.
>
> Signed-off-by: Hem Parekh <hemparekh1596 at gmail.com>
While this patch is nice, IMHO the one from Piyush is addresses more issues
while solving the same problem.
cf https://lore.kernel.org/u-boot/20260612075424.83462-1-piyushthepal@gmail.com/
Thanks!
Regards,
Richard
> ---
> fs/squashfs/sqfs.c | 29 +++++++++++++++++------------
> fs/squashfs/sqfs_filesystem.h | 5 +++--
> fs/squashfs/sqfs_inode.c | 20 ++++++++++++++++----
> 3 files changed, 36 insertions(+), 18 deletions(-)
>
> diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
> index 0768fc4a7b2..d92f8869284 100644
> --- a/fs/squashfs/sqfs.c
> +++ b/fs/squashfs/sqfs.c
> @@ -486,8 +486,9 @@ static int sqfs_search_dir(struct squashfs_dir_stream *dirs, char **token_list,
> dirsp = (struct fs_dir_stream *)dirs;
>
> /* Start by root inode */
> - table = sqfs_find_inode(dirs->inode_table, le32_to_cpu(sblk->inodes),
> - sblk->inodes, sblk->block_size);
> + table = sqfs_find_inode(dirs->inode_table, dirs->inode_table_size,
> + le32_to_cpu(sblk->inodes), sblk->inodes,
> + sblk->block_size);
> if (!table)
> return -EINVAL;
>
> @@ -543,8 +544,9 @@ static int sqfs_search_dir(struct squashfs_dir_stream *dirs, char **token_list,
> dirs->dir_header->inode_number;
>
> /* Get reference to inode in the inode table */
> - table = sqfs_find_inode(dirs->inode_table, new_inode_number,
> - sblk->inodes, sblk->block_size);
> + table = sqfs_find_inode(dirs->inode_table, dirs->inode_table_size,
> + new_inode_number, sblk->inodes,
> + sblk->block_size);
> if (!table)
> return -EINVAL;
> dir = (struct squashfs_dir_inode *)table;
> @@ -719,7 +721,8 @@ static int sqfs_get_metablk_pos(u32 *pos_list, void *table, u32 offset,
> return ret;
> }
>
> -static int sqfs_read_inode_table(unsigned char **inode_table)
> +static int sqfs_read_inode_table(unsigned char **inode_table,
> + size_t *inode_table_size)
> {
> struct squashfs_super_block *sblk = ctxt.sblk;
> u64 start, n_blks, table_offset, table_size;
> @@ -773,6 +776,8 @@ static int sqfs_read_inode_table(unsigned char **inode_table)
> goto free_itb;
> }
>
> + *inode_table_size = (size_t)metablks_count * SQFS_METADATA_BLOCK_SIZE;
> +
> src_table = itb + table_offset + SQFS_HEADER_SIZE;
>
> /* Extract compressed Inode table */
> @@ -933,7 +938,7 @@ static int sqfs_opendir_nest(const char *filename, struct fs_dir_stream **dirsp)
> dirs->inode_table = NULL;
> dirs->dir_table = NULL;
>
> - ret = sqfs_read_inode_table(&inode_table);
> + ret = sqfs_read_inode_table(&inode_table, &dirs->inode_table_size);
> if (ret) {
> ret = -EINVAL;
> goto out;
> @@ -1071,8 +1076,8 @@ static int sqfs_readdir_nest(struct fs_dir_stream *fs_dirs, struct fs_dirent **d
> }
>
> i_number = dirs->dir_header->inode_number + dirs->entry->inode_offset;
> - ipos = sqfs_find_inode(dirs->inode_table, i_number, sblk->inodes,
> - sblk->block_size);
> + ipos = sqfs_find_inode(dirs->inode_table, dirs->inode_table_size,
> + i_number, sblk->inodes, sblk->block_size);
> if (!ipos)
> return -SQFS_STOP_READDIR;
>
> @@ -1430,8 +1435,8 @@ static int sqfs_read_nest(const char *filename, void *buf, loff_t offset,
> }
>
> i_number = dirs->dir_header->inode_number + dirs->entry->inode_offset;
> - ipos = sqfs_find_inode(dirs->inode_table, i_number, sblk->inodes,
> - sblk->block_size);
> + ipos = sqfs_find_inode(dirs->inode_table, dirs->inode_table_size,
> + i_number, sblk->inodes, sblk->block_size);
> if (!ipos) {
> ret = -EINVAL;
> goto out;
> @@ -1699,8 +1704,8 @@ static int sqfs_size_nest(const char *filename, loff_t *size)
> }
>
> i_number = dirs->dir_header->inode_number + dirs->entry->inode_offset;
> - ipos = sqfs_find_inode(dirs->inode_table, i_number, sblk->inodes,
> - sblk->block_size);
> + ipos = sqfs_find_inode(dirs->inode_table, dirs->inode_table_size,
> + i_number, sblk->inodes, sblk->block_size);
>
> if (!ipos) {
> *size = 0;
> diff --git a/fs/squashfs/sqfs_filesystem.h b/fs/squashfs/sqfs_filesystem.h
> index be56498a5e3..2e4224ea78a 100644
> --- a/fs/squashfs/sqfs_filesystem.h
> +++ b/fs/squashfs/sqfs_filesystem.h
> @@ -275,6 +275,7 @@ struct squashfs_dir_stream {
> * sqfs_opendir() and freed in sqfs_closedir().
> */
> unsigned char *inode_table;
> + size_t inode_table_size;
> unsigned char *dir_table;
> };
>
> @@ -293,8 +294,8 @@ struct squashfs_file_info {
> bool comp;
> };
>
> -void *sqfs_find_inode(void *inode_table, int inode_number, __le32 inode_count,
> - __le32 block_size);
> +void *sqfs_find_inode(void *inode_table, size_t inode_table_size,
> + int inode_number, __le32 inode_count, __le32 block_size);
>
> int sqfs_dir_offset(void *dir_i, u32 *m_list, int m_count);
>
> diff --git a/fs/squashfs/sqfs_inode.c b/fs/squashfs/sqfs_inode.c
> index ce9a8ff8e2a..da347becaff 100644
> --- a/fs/squashfs/sqfs_inode.c
> +++ b/fs/squashfs/sqfs_inode.c
> @@ -17,7 +17,8 @@
> #include "sqfs_filesystem.h"
> #include "sqfs_utils.h"
>
> -int sqfs_inode_size(struct squashfs_base_inode *inode, u32 blk_size)
> +int sqfs_inode_size(struct squashfs_base_inode *inode, u32 blk_size,
> + const void *end)
> {
> u16 inode_type = get_unaligned_le16(&inode->inode_type);
>
> @@ -53,9 +54,13 @@ int sqfs_inode_size(struct squashfs_base_inode *inode, u32 blk_size)
>
> di = ldir->index;
> while (l < i_count) {
> + if ((void *)di + sizeof(*di) > end)
> + return -EINVAL;
> sz = get_unaligned_le32(&di->size) + 1;
> index_list_size += sz;
> di = (void *)di + sizeof(*di) + sz;
> + if ((void *)di > end || (void *)di < (void *)ldir)
> + return -EINVAL;
> l++;
> }
>
> @@ -114,9 +119,10 @@ int sqfs_inode_size(struct squashfs_base_inode *inode, u32 blk_size)
> * Given the uncompressed inode table, the inode to be found and the number of
> * inodes in the table, return inode position in case of success.
> */
> -void *sqfs_find_inode(void *inode_table, int inode_number, __le32 inode_count,
> - __le32 block_size)
> +void *sqfs_find_inode(void *inode_table, size_t inode_table_size,
> + int inode_number, __le32 inode_count, __le32 block_size)
> {
> + const void *end = inode_table + inode_table_size;
> struct squashfs_base_inode *base;
> unsigned int offset = 0, k;
> int sz;
> @@ -128,14 +134,20 @@ void *sqfs_find_inode(void *inode_table, int inode_number, __le32 inode_count,
>
> for (k = 0; k < le32_to_cpu(inode_count); k++) {
> base = inode_table + offset;
> + /* Ensure the base inode fields lie within the table */
> + if ((void *)(base + 1) > end)
> + return NULL;
> if (get_unaligned_le32(&base->inode_number) == inode_number)
> return inode_table + offset;
>
> - sz = sqfs_inode_size(base, le32_to_cpu(block_size));
> + sz = sqfs_inode_size(base, le32_to_cpu(block_size), end);
> if (sz < 0)
> return NULL;
>
> offset += sz;
> + /* The computed inode must not run past the table buffer */
> + if (offset > inode_table_size)
> + return NULL;
> }
>
> printf("Inode not found.\n");
--
Richard Genoud, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
More information about the U-Boot
mailing list