From: Andrew Morton <akpm@linux-foundation.org>
To: akpm@linux-foundation.org, linux-mm@kvack.org,
mm-commits@vger.kernel.org, phillip@squashfs.org.uk,
stable@vger.kernel.org, torvalds@linux-foundation.org
Subject: [patch 04/14] squashfs: add more sanity checks in xattr id lookup
Date: Tue, 09 Feb 2021 13:42:00 -0800 [thread overview]
Message-ID: <20210209214200.eB2f3Dmb5%akpm@linux-foundation.org> (raw)
In-Reply-To: <20210209134115.4d933d446165cd0ed8977b03@linux-foundation.org>
From: Phillip Lougher <phillip@squashfs.org.uk>
Subject: squashfs: add more sanity checks in xattr id lookup
Sysbot has reported a warning where a kmalloc() attempt exceeds the
maximum limit. This has been identified as corruption of the xattr_ids
count when reading the xattr id lookup table.
This patch adds a number of additional sanity checks to detect this
corruption and others.
1. It checks for a corrupted xattr index read from the inode. This could
be because the metadata block is uncompressed, or because the
"compression" bit has been corrupted (turning a compressed block
into an uncompressed block). This would cause an out of bounds read.
2. It checks against corruption of the xattr_ids count. This can either
lead to the above kmalloc failure, or a smaller than expected
table to be read.
3. It checks the contents of the index table for corruption.
[phillip@squashfs.org.uk: fix checkpatch issue]
Link: https://lkml.kernel.org/r/270245655.754655.1612770082682@webmail.123-reg.co.uk
Link: https://lkml.kernel.org/r/20210204130249.4495-5-phillip@squashfs.org.uk
Signed-off-by: Phillip Lougher <phillip@squashfs.org.uk>
Reported-by: syzbot+2ccea6339d368360800d@syzkaller.appspotmail.com
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
fs/squashfs/xattr_id.c | 66 +++++++++++++++++++++++++++++++++------
1 file changed, 57 insertions(+), 9 deletions(-)
--- a/fs/squashfs/xattr_id.c~squashfs-add-more-sanity-checks-in-xattr-id-lookup
+++ a/fs/squashfs/xattr_id.c
@@ -31,10 +31,15 @@ int squashfs_xattr_lookup(struct super_b
struct squashfs_sb_info *msblk = sb->s_fs_info;
int block = SQUASHFS_XATTR_BLOCK(index);
int offset = SQUASHFS_XATTR_BLOCK_OFFSET(index);
- u64 start_block = le64_to_cpu(msblk->xattr_id_table[block]);
+ u64 start_block;
struct squashfs_xattr_id id;
int err;
+ if (index >= msblk->xattr_ids)
+ return -EINVAL;
+
+ start_block = le64_to_cpu(msblk->xattr_id_table[block]);
+
err = squashfs_read_metadata(sb, &id, &start_block, &offset,
sizeof(id));
if (err < 0)
@@ -50,13 +55,17 @@ int squashfs_xattr_lookup(struct super_b
/*
* Read uncompressed xattr id lookup table indexes from disk into memory
*/
-__le64 *squashfs_read_xattr_id_table(struct super_block *sb, u64 start,
+__le64 *squashfs_read_xattr_id_table(struct super_block *sb, u64 table_start,
u64 *xattr_table_start, int *xattr_ids)
{
- unsigned int len;
+ struct squashfs_sb_info *msblk = sb->s_fs_info;
+ unsigned int len, indexes;
struct squashfs_xattr_id_table *id_table;
+ __le64 *table;
+ u64 start, end;
+ int n;
- id_table = squashfs_read_table(sb, start, sizeof(*id_table));
+ id_table = squashfs_read_table(sb, table_start, sizeof(*id_table));
if (IS_ERR(id_table))
return (__le64 *) id_table;
@@ -70,13 +79,52 @@ __le64 *squashfs_read_xattr_id_table(str
if (*xattr_ids == 0)
return ERR_PTR(-EINVAL);
- /* xattr_table should be less than start */
- if (*xattr_table_start >= start)
+ len = SQUASHFS_XATTR_BLOCK_BYTES(*xattr_ids);
+ indexes = SQUASHFS_XATTR_BLOCKS(*xattr_ids);
+
+ /*
+ * The computed size of the index table (len bytes) should exactly
+ * match the table start and end points
+ */
+ start = table_start + sizeof(*id_table);
+ end = msblk->bytes_used;
+
+ if (len != (end - start))
return ERR_PTR(-EINVAL);
- len = SQUASHFS_XATTR_BLOCK_BYTES(*xattr_ids);
+ table = squashfs_read_table(sb, start, len);
+ if (IS_ERR(table))
+ return table;
+
+ /* table[0], table[1], ... table[indexes - 1] store the locations
+ * of the compressed xattr id blocks. Each entry should be less than
+ * the next (i.e. table[0] < table[1]), and the difference between them
+ * should be SQUASHFS_METADATA_SIZE or less. table[indexes - 1]
+ * should be less than table_start, and again the difference
+ * shouls be SQUASHFS_METADATA_SIZE or less.
+ *
+ * Finally xattr_table_start should be less than table[0].
+ */
+ for (n = 0; n < (indexes - 1); n++) {
+ start = le64_to_cpu(table[n]);
+ end = le64_to_cpu(table[n + 1]);
+
+ if (start >= end || (end - start) > SQUASHFS_METADATA_SIZE) {
+ kfree(table);
+ return ERR_PTR(-EINVAL);
+ }
+ }
+
+ start = le64_to_cpu(table[indexes - 1]);
+ if (start >= table_start || (table_start - start) > SQUASHFS_METADATA_SIZE) {
+ kfree(table);
+ return ERR_PTR(-EINVAL);
+ }
- TRACE("In read_xattr_index_table, length %d\n", len);
+ if (*xattr_table_start >= le64_to_cpu(table[0])) {
+ kfree(table);
+ return ERR_PTR(-EINVAL);
+ }
- return squashfs_read_table(sb, start + sizeof(*id_table), len);
+ return table;
}
_
next prev parent reply other threads:[~2021-02-09 21:42 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-09 21:41 incoming Andrew Morton
2021-02-09 21:41 ` [patch 01/14] squashfs: avoid out of bounds writes in decompressors Andrew Morton
2021-02-09 21:41 ` [patch 02/14] squashfs: add more sanity checks in id lookup Andrew Morton
2021-02-09 21:41 ` [patch 03/14] squashfs: add more sanity checks in inode lookup Andrew Morton
2021-02-09 21:42 ` Andrew Morton [this message]
2021-02-09 21:42 ` [patch 05/14] kasan: fix stack traces dependency for HW_TAGS Andrew Morton
2021-02-09 21:42 ` [patch 06/14] firmware_loader: align .builtin_fw to 8 Andrew Morton
2021-02-09 21:42 ` [patch 07/14] mm/mremap: fix BUILD_BUG_ON() error in get_extent Andrew Morton
2021-02-09 21:42 ` [patch 08/14] tmpfs: disallow CONFIG_TMPFS_INODE64 on s390 Andrew Morton
2021-02-09 21:42 ` [patch 09/14] tmpfs: disallow CONFIG_TMPFS_INODE64 on alpha Andrew Morton
2021-02-09 22:03 ` Linus Torvalds
2021-02-10 13:34 ` Heiko Carstens
2021-02-10 17:27 ` Heiko Carstens
2021-02-10 19:17 ` Linus Torvalds
2021-02-10 19:55 ` Arnd Bergmann
2021-02-11 18:45 ` Heiko Carstens
2021-02-09 21:42 ` [patch 10/14] selftests/vm: rename file run_vmtests to run_vmtests.sh Andrew Morton
2021-02-09 21:42 ` [patch 11/14] MAINTAINERS: update Andrey Ryabinin's email address Andrew Morton
2021-02-09 21:42 ` [patch 12/14] Revert "mm: memcontrol: avoid workload stalls when lowering memory.high" Andrew Morton
2021-02-09 21:42 ` [patch 13/14] mm, slub: better heuristic for number of cpus when calculating slab order Andrew Morton
2021-02-10 14:34 ` Vlastimil Babka
2021-02-10 19:22 ` Linus Torvalds
2021-02-09 21:42 ` [patch 14/14] nilfs2: make splice write available again Andrew Morton
2021-02-10 19:30 ` incoming Linus Torvalds
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210209214200.eB2f3Dmb5%akpm@linux-foundation.org \
--to=akpm@linux-foundation.org \
--cc=linux-mm@kvack.org \
--cc=mm-commits@vger.kernel.org \
--cc=phillip@squashfs.org.uk \
--cc=stable@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).