All of lore.kernel.org
 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, Yoon Jungyeon <jungyeon@gatech.edu>,
	Nikolay Borisov <nborisov@suse.com>, Qu Wenruo <wqu@suse.com>,
	David Sterba <dsterba@suse.com>,
	Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Subject: [PATCH 4.4 02/24] btrfs: inode: Verify inode mode to avoid NULL pointer dereference
Date: Tue,  1 Dec 2020 09:53:08 +0100	[thread overview]
Message-ID: <20201201084637.885690100@linuxfoundation.org> (raw)
In-Reply-To: <20201201084637.754785180@linuxfoundation.org>

From: Qu Wenruo <wqu@suse.com>

commit 6bf9e4bd6a277840d3fe8c5d5d530a1fbd3db592 upstream

[BUG]
When accessing a file on a crafted image, btrfs can crash in block layer:

  BUG: unable to handle kernel NULL pointer dereference at 0000000000000008
  PGD 136501067 P4D 136501067 PUD 124519067 PMD 0
  CPU: 3 PID: 0 Comm: swapper/3 Not tainted 5.0.0-rc8-default #252
  RIP: 0010:end_bio_extent_readpage+0x144/0x700
  Call Trace:
   <IRQ>
   blk_update_request+0x8f/0x350
   blk_mq_end_request+0x1a/0x120
   blk_done_softirq+0x99/0xc0
   __do_softirq+0xc7/0x467
   irq_exit+0xd1/0xe0
   call_function_single_interrupt+0xf/0x20
   </IRQ>
  RIP: 0010:default_idle+0x1e/0x170

[CAUSE]
The crafted image has a tricky corruption, the INODE_ITEM has a
different type against its parent dir:

        item 20 key (268 INODE_ITEM 0) itemoff 2808 itemsize 160
                generation 13 transid 13 size 1048576 nbytes 1048576
                block group 0 mode 121644 links 1 uid 0 gid 0 rdev 0
                sequence 9 flags 0x0(none)

This mode number 0120000 means it's a symlink.

But the dir item think it's still a regular file:

        item 8 key (264 DIR_INDEX 5) itemoff 3707 itemsize 32
                location key (268 INODE_ITEM 0) type FILE
                transid 13 data_len 0 name_len 2
                name: f4
        item 40 key (264 DIR_ITEM 51821248) itemoff 1573 itemsize 32
                location key (268 INODE_ITEM 0) type FILE
                transid 13 data_len 0 name_len 2
                name: f4

For symlink, we don't set BTRFS_I(inode)->io_tree.ops and leave it
empty, as symlink is only designed to have inlined extent, all handled
by tree block read.  Thus no need to trigger btrfs_submit_bio_hook() for
inline file extent.

However end_bio_extent_readpage() expects tree->ops populated, as it's
reading regular data extent.  This causes NULL pointer dereference.

[FIX]
This patch fixes the problem in two ways:

- Verify inode mode against its dir item when looking up inode
  So in btrfs_lookup_dentry() if we find inode mode mismatch with dir
  item, we error out so that corrupted inode will not be accessed.

- Verify inode mode when getting extent mapping
  Only regular file should have regular or preallocated extent.
  If we found regular/preallocated file extent for symlink or
  the rest, we error out before submitting the read bio.

With this fix that crafted image can be rejected gracefully:

  BTRFS critical (device loop0): inode mode mismatch with dir: inode mode=0121644 btrfs type=7 dir type=1

Reported-by: Yoon Jungyeon <jungyeon@gatech.edu>
Link: https://bugzilla.kernel.org/show_bug.cgi?id=202763
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
[sudip: use original btrfs_inode_type(), btrfs_crit with root->fs_info,
ISREG with inode->i_mode and adjust context]
Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 fs/btrfs/inode.c             |   41 +++++++++++++++++++++++++++++++++--------
 fs/btrfs/tests/inode-tests.c |    1 +
 2 files changed, 34 insertions(+), 8 deletions(-)

--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -5370,11 +5370,13 @@ no_delete:
 }
 
 /*
- * this returns the key found in the dir entry in the location pointer.
+ * Return the key found in the dir entry in the location pointer, fill @type
+ * with BTRFS_FT_*, and return 0.
+ *
  * If no dir entries were found, location->objectid is 0.
  */
 static int btrfs_inode_by_name(struct inode *dir, struct dentry *dentry,
-			       struct btrfs_key *location)
+			       struct btrfs_key *location, u8 *type)
 {
 	const char *name = dentry->d_name.name;
 	int namelen = dentry->d_name.len;
@@ -5396,6 +5398,8 @@ static int btrfs_inode_by_name(struct in
 		goto out_err;
 
 	btrfs_dir_item_key_to_cpu(path->nodes[0], di, location);
+	if (!ret)
+		*type = btrfs_dir_type(path->nodes[0], di);
 out:
 	btrfs_free_path(path);
 	return ret;
@@ -5681,19 +5685,25 @@ static struct inode *new_simple_dir(stru
 	return inode;
 }
 
+static inline u8 btrfs_inode_type(struct inode *inode)
+{
+	return btrfs_type_by_mode[(inode->i_mode & S_IFMT) >> S_SHIFT];
+}
+
 struct inode *btrfs_lookup_dentry(struct inode *dir, struct dentry *dentry)
 {
 	struct inode *inode;
 	struct btrfs_root *root = BTRFS_I(dir)->root;
 	struct btrfs_root *sub_root = root;
 	struct btrfs_key location;
+	u8 di_type = 0;
 	int index;
 	int ret = 0;
 
 	if (dentry->d_name.len > BTRFS_NAME_LEN)
 		return ERR_PTR(-ENAMETOOLONG);
 
-	ret = btrfs_inode_by_name(dir, dentry, &location);
+	ret = btrfs_inode_by_name(dir, dentry, &location, &di_type);
 	if (ret < 0)
 		return ERR_PTR(ret);
 
@@ -5702,6 +5712,18 @@ struct inode *btrfs_lookup_dentry(struct
 
 	if (location.type == BTRFS_INODE_ITEM_KEY) {
 		inode = btrfs_iget(dir->i_sb, &location, root, NULL);
+		if (IS_ERR(inode))
+			return inode;
+
+		/* Do extra check against inode mode with di_type */
+		if (btrfs_inode_type(inode) != di_type) {
+			btrfs_crit(root->fs_info,
+"inode mode mismatch with dir: inode mode=0%o btrfs type=%u dir type=%u",
+				  inode->i_mode, btrfs_inode_type(inode),
+				  di_type);
+			iput(inode);
+			return ERR_PTR(-EUCLEAN);
+		}
 		return inode;
 	}
 
@@ -6315,11 +6337,6 @@ fail:
 	return ERR_PTR(ret);
 }
 
-static inline u8 btrfs_inode_type(struct inode *inode)
-{
-	return btrfs_type_by_mode[(inode->i_mode & S_IFMT) >> S_SHIFT];
-}
-
 /*
  * utility function to add 'inode' into 'parent_inode' with
  * a give name and a given sequence number.
@@ -6904,6 +6921,14 @@ again:
 	extent_start = found_key.offset;
 	if (found_type == BTRFS_FILE_EXTENT_REG ||
 	    found_type == BTRFS_FILE_EXTENT_PREALLOC) {
+		/* Only regular file could have regular/prealloc extent */
+		if (!S_ISREG(inode->i_mode)) {
+			ret = -EUCLEAN;
+			btrfs_crit(root->fs_info,
+		"regular/prealloc extent found for non-regular inode %llu",
+				   btrfs_ino(inode));
+			goto out;
+		}
 		extent_end = extent_start +
 		       btrfs_file_extent_num_bytes(leaf, item);
 	} else if (found_type == BTRFS_FILE_EXTENT_INLINE) {
--- a/fs/btrfs/tests/inode-tests.c
+++ b/fs/btrfs/tests/inode-tests.c
@@ -235,6 +235,7 @@ static noinline int test_btrfs_get_exten
 		return ret;
 	}
 
+	inode->i_mode = S_IFREG;
 	BTRFS_I(inode)->location.type = BTRFS_INODE_ITEM_KEY;
 	BTRFS_I(inode)->location.objectid = BTRFS_FIRST_FREE_OBJECTID;
 	BTRFS_I(inode)->location.offset = 0;



  parent reply	other threads:[~2020-12-01  9:31 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-01  8:53 [PATCH 4.4 00/24] 4.4.247-rc1 review Greg Kroah-Hartman
2020-12-01  8:53 ` [PATCH 4.4 01/24] btrfs: tree-checker: Enhance chunk checker to validate chunk profile Greg Kroah-Hartman
2020-12-01  8:53 ` Greg Kroah-Hartman [this message]
2020-12-01  8:53 ` [PATCH 4.4 03/24] KVM: x86: Fix split-irqchip vs interrupt injection window request Greg Kroah-Hartman
2020-12-01  8:53 ` [PATCH 4.4 04/24] HID: cypress: Support Varmilo Keyboards media hotkeys Greg Kroah-Hartman
2020-12-01  8:53 ` [PATCH 4.4 05/24] Input: i8042 - allow insmod to succeed on devices without an i8042 controller Greg Kroah-Hartman
2020-12-01  8:53 ` [PATCH 4.4 06/24] HID: hid-sensor-hub: Fix issue with devices with no report ID Greg Kroah-Hartman
2020-12-01  8:53 ` [PATCH 4.4 07/24] x86/xen: dont unbind uninitialized lock_kicker_irq Greg Kroah-Hartman
2020-12-01  8:53 ` [PATCH 4.4 08/24] proc: dont allow async path resolution of /proc/self components Greg Kroah-Hartman
2020-12-01  8:53 ` [PATCH 4.4 09/24] dmaengine: pl330: _prep_dma_memcpy: Fix wrong burst size Greg Kroah-Hartman
2020-12-01  8:53 ` [PATCH 4.4 10/24] scsi: libiscsi: Fix NOP race condition Greg Kroah-Hartman
2020-12-01  8:53 ` [PATCH 4.4 11/24] scsi: target: iscsi: Fix cmd abort fabric stop race Greg Kroah-Hartman
2020-12-01  8:53 ` [PATCH 4.4 12/24] scsi: ufs: Fix race between shutdown and runtime resume flow Greg Kroah-Hartman
2020-12-01  8:53 ` [PATCH 4.4 13/24] bnxt_en: fix error return code in bnxt_init_board() Greg Kroah-Hartman
2020-12-01  8:53 ` [PATCH 4.4 14/24] video: hyperv_fb: Fix the cache type when mapping the VRAM Greg Kroah-Hartman
2020-12-01  8:53 ` [PATCH 4.4 15/24] bnxt_en: Release PCI regions when DMA mask setup fails during probe Greg Kroah-Hartman
2020-12-01  8:53 ` [PATCH 4.4 16/24] IB/mthca: fix return value of error branch in mthca_init_cq() Greg Kroah-Hartman
2020-12-01  8:53 ` [PATCH 4.4 17/24] nfc: s3fwrn5: use signed integer for parsing GPIO numbers Greg Kroah-Hartman
2020-12-01  8:53 ` [PATCH 4.4 18/24] efivarfs: revert "fix memory leak in efivarfs_create()" Greg Kroah-Hartman
2020-12-01  8:53 ` [PATCH 4.4 19/24] perf probe: Fix to die_entrypc() returns error correctly Greg Kroah-Hartman
2020-12-01  8:53 ` [PATCH 4.4 20/24] USB: core: Change %pK for __user pointers to %px Greg Kroah-Hartman
2020-12-01  8:53 ` [PATCH 4.4 21/24] x86/speculation: Fix prctl() when spectre_v2_user={seccomp,prctl},ibpb Greg Kroah-Hartman
2020-12-01  8:53 ` [PATCH 4.4 22/24] USB: core: add endpoint-blacklist quirk Greg Kroah-Hartman
2020-12-01  8:53 ` [PATCH 4.4 23/24] USB: core: Fix regression in Hercules audio card Greg Kroah-Hartman
2020-12-01  8:53 ` [PATCH 4.4 24/24] btrfs: fix lockdep splat when reading qgroup config on mount Greg Kroah-Hartman
2020-12-01 13:41 ` [PATCH 4.4 00/24] 4.4.247-rc1 review Jon Hunter
2020-12-01 15:59 ` Pavel Machek
2020-12-01 21:38 ` Guenter Roeck
2020-12-02  6:17 ` Naresh Kamboju
2020-12-02 17:04 ` Shuah Khan

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=20201201084637.885690100@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=dsterba@suse.com \
    --cc=jungyeon@gatech.edu \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nborisov@suse.com \
    --cc=stable@vger.kernel.org \
    --cc=sudipm.mukherjee@gmail.com \
    --cc=wqu@suse.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.