linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Phillip Lougher <phillip@squashfs.org.uk>,
	syzbot+2ccea6339d368360800d@syzkaller.appspotmail.com,
	Andrew Morton <akpm@linux-foundation.org>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: [PATCH 5.4 24/24] squashfs: add more sanity checks in xattr id lookup
Date: Thu, 11 Feb 2021 16:02:47 +0100	[thread overview]
Message-ID: <20210211150149.577145041@linuxfoundation.org> (raw)
In-Reply-To: <20210211150148.516371325@linuxfoundation.org>

From: Phillip Lougher <phillip@squashfs.org.uk>

commit 506220d2ba21791314af569211ffd8870b8208fa upstream.

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>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 fs/squashfs/xattr_id.c |   66 ++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 57 insertions(+), 9 deletions(-)

--- a/fs/squashfs/xattr_id.c
+++ b/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;
 }



  parent reply	other threads:[~2021-02-11 15:49 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-11 15:02 [PATCH 5.4 00/24] 5.4.98-rc1 review Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 01/24] tracing/kprobe: Fix to support kretprobe events on unloaded modules Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 02/24] af_key: relax availability checks for skb size calculation Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 03/24] regulator: core: avoid regulator_resolve_supply() race condition Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 04/24] mac80211: 160MHz with extended NSS BW in CSA Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 05/24] ASoC: Intel: Skylake: Zero snd_ctl_elem_value Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 06/24] chtls: Fix potential resource leak Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 07/24] pNFS/NFSv4: Try to return invalid layout in pnfs_layout_process() Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 08/24] ASoC: ak4458: correct reset polarity Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 09/24] iwlwifi: mvm: skip power command when unbinding vif during CSA Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 10/24] iwlwifi: mvm: take mutex for calling iwl_mvm_get_sync_time() Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 11/24] iwlwifi: pcie: add a NULL check in iwl_pcie_txq_unmap Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 12/24] iwlwifi: pcie: fix context info memory leak Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 13/24] iwlwifi: mvm: invalidate IDs of internal stations at mvm start Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 14/24] iwlwifi: mvm: guard against device removal in reprobe Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 15/24] SUNRPC: Move simple_get_bytes and simple_get_netobj into private header Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 16/24] SUNRPC: Handle 0 length opaque XDR object data properly Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 17/24] i2c: mediatek: Move suspend and resume handling to NOIRQ phase Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 18/24] blk-cgroup: Use cond_resched() when destroy blkgs Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 19/24] regulator: Fix lockdep warning resolving supplies Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 20/24] bpf: Fix 32 bit src register truncation on div/mod Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 21/24] Fix unsynchronized access to sev members through svm_register_enc_region Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 22/24] squashfs: add more sanity checks in id lookup Greg Kroah-Hartman
2021-02-11 15:02 ` [PATCH 5.4 23/24] squashfs: add more sanity checks in inode lookup Greg Kroah-Hartman
2021-02-11 15:02 ` Greg Kroah-Hartman [this message]
2021-02-12  4:24 ` [PATCH 5.4 00/24] 5.4.98-rc1 review Naresh Kamboju
2021-02-12 15:20 ` Naresh Kamboju
2021-02-12 16:17 ` Shuah Khan
2021-02-12 18:08 ` Guenter Roeck
2021-02-12 19:02 ` Florian Fainelli
2021-02-13  3:17 ` Ross Schmidt

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=20210211150149.577145041@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=phillip@squashfs.org.uk \
    --cc=stable@vger.kernel.org \
    --cc=syzbot+2ccea6339d368360800d@syzkaller.appspotmail.com \
    --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).