All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anand Jain <anand.jain@oracle.com>
To: linux-btrfs@vger.kernel.org
Subject: [PATCH v2] btrfs: btrfs_shrink_device should call commit transaction
Date: Mon,  6 Aug 2018 18:12:37 +0800	[thread overview]
Message-ID: <20180806101237.20507-1-anand.jain@oracle.com> (raw)
In-Reply-To: <20180725132727.642-1-anand.jain@oracle.com>

test case btrfs/164 reported UAF..

[ 6712.084324] general protection fault: 0000 [#1] PREEMPT SMP
::
[ 6712.195423]  btrfs_update_commit_device_size+0x75/0xf0 [btrfs]
[ 6712.201424]  btrfs_commit_transaction+0x57d/0xa90 [btrfs]
[ 6712.206999]  btrfs_rm_device+0x627/0x850 [btrfs]
[ 6712.211800]  btrfs_ioctl+0x2b03/0x3120 [btrfs]
::

reason for this is that btrfs_shrink_device() adds the device resized to
the fs_devices::resized_devices after it has called the last commit
transaction.
So the list fs_devices::resized_devices is not empty when
btrfs_shrink_device() returns.
Now the parent function btrfs_rm_device() calls
        btrfs_close_bdev(device);
        call_rcu(&device->rcu, free_device_rcu);
and then does the commit transaction.
The commit transaction goes through the fs_devices::resized_devices
in btrfs_update_commit_device_size() and leads to UAF.

Fix this by making sure btrfs_shrink_device() calls the last needed
btrfs_commit_transaction() before the return.

Reported-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
Signed-off-by: Anand Jain <anand.jain@oracle.com>
Tested-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
---
v1->v2: cleanup the dirty and new meta block group before ending
the transaction when btrfs_update_device fails.

 fs/btrfs/volumes.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index da86706123ff..f4405e430da6 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -4491,7 +4491,12 @@ int btrfs_shrink_device(struct btrfs_device *device, u64 new_size)
 
 	/* Now btrfs_update_device() will change the on-disk size. */
 	ret = btrfs_update_device(trans, device);
-	btrfs_end_transaction(trans);
+	if (ret < 0) {
+		btrfs_abort_transaction(trans, ret);
+		btrfs_end_transaction(trans);
+	} else {
+		ret = btrfs_commit_transaction(trans);
+	}
 done:
 	btrfs_free_path(path);
 	if (ret) {
-- 
2.7.0


  parent reply	other threads:[~2018-08-06 12:17 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-25 13:27 [PATCH] btrfs: btrfs_shrink_device should call commit transaction Anand Jain
2018-07-25 16:52 ` Lu Fengqi
2018-07-30 14:18 ` Nikolay Borisov
2018-08-03 11:27   ` Anand Jain
2018-08-03 11:34     ` Nikolay Borisov
2018-08-06 10:12 ` Anand Jain [this message]
2018-08-17 14:04   ` [PATCH v2] " David Sterba

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=20180806101237.20507-1-anand.jain@oracle.com \
    --to=anand.jain@oracle.com \
    --cc=linux-btrfs@vger.kernel.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 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.