[PATCH v2] fs/squashfs: fix missing error checks causing Synchronous Abort

[Allan ELKAIM]

sqfs_find_inode() returns NULL on failure, and sqfs_dir_offset()
returns -EINVAL on failure. At 8 call sites in sqfs.c neither return
value was validated before the result was cast to a struct pointer or
used as an array index.

On squashfs images with large inode/directory tables (e.g. containing
the tzdata timezone database), sqfs_dir_offset() fails to locate a
start_block in the metadata-block position list and returns -EINVAL.
The caller then computes &dir_table[-22], producing a faulting address
of 0xffffffffffffffea and a Synchronous Abort on ARM64:

  "Synchronous Abort" handler, esr 0x96000004, far 0xffffffffffffffea

The "Error: invalid inode reference to directory table." message
printed just before the abort confirms the sqfs_dir_offset() path.
Additionally, sqfs_find_inode() can return NULL when the inode table
scan encounters an unknown or malformed inode type, after which any
dereference of the cast pointer crashes.

Add NULL guards after every sqfs_find_inode() call and negative-value
guards after every sqfs_dir_offset() call so that any lookup failure
propagates as -EINVAL rather than crashing.

Signed-off-by: Allan Elkaim <allan.elkaim at gmail.com>
---
 fs/squashfs/sqfs.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index 1430e671..da8fbb5a 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -472,12 +472,16 @@ static int sqfs_search_dir(struct squashfs_dir_stream *dirs, char **token_list,
 	/* Start by root inode */
 	table = sqfs_find_inode(dirs->inode_table, le32_to_cpu(sblk->inodes),
 				sblk->inodes, sblk->block_size);
+	if (!table)
+		return -EINVAL;
 
 	dir = (struct squashfs_dir_inode *)table;
 	ldir = (struct squashfs_ldir_inode *)table;
 
 	/* 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 */
@@ -527,6 +531,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) {
+			free(dirs->entry);
+			dirs->entry = NULL;
+			ret = -EINVAL;
+			goto out;
+		}
 		dir = (struct squashfs_dir_inode *)table;
 
 		/* Check for symbolic link and inode type sanity */
@@ -599,6 +609,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 */
@@ -623,6 +639,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)
@@ -1023,6 +1045,8 @@ int sqfs_readdir(struct fs_dir_stream *fs_dirs, struct fs_dirent **dentp)
 	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);
+	if (!ipos)
+		return -SQFS_STOP_READDIR;
 
 	base = (struct squashfs_base_inode *)ipos;
 
@@ -1379,6 +1403,10 @@ int sqfs_read(const char *filename, void *buf, loff_t offset, loff_t len,
 	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);
+	if (!ipos) {
+		ret = -EINVAL;
+		goto out;
+	}
 
 	base = (struct squashfs_base_inode *)ipos;
 	switch (get_unaligned_le16(&base->inode_type)) {
@@ -1629,6 +1657,11 @@ int sqfs_size(const char *filename, loff_t *size)
 			       sblk->block_size);
 	free(dirs->entry);
 	dirs->entry = NULL;
+	if (!ipos) {
+		*size = 0;
+		ret = -EINVAL;
+		goto free_strings;
+	}
 
 	base = (struct squashfs_base_inode *)ipos;
 	switch (get_unaligned_le16(&base->inode_type)) {
-- 
2.53.0