Re: [PATCH v6] block: Fix use-after-free in blkdev_get()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Dan,

在 2020/6/16 18:20, Dan Carpenter 写道:
On Tue, Jun 16, 2020 at 11:40:02AM +0800, Jason Yan wrote:

Fixes: e525fd89d380 ("block: make blkdev_get/put() handle exclusive access")

I still don't understand how this is the correct fixes tag...  :/

git show e525fd89d380:fs/block_dev.c | cat -n
   1208  int blkdev_get(struct block_device *bdev, fmode_t mode, void *holder)
   1209  {
   1210          struct block_device *whole = NULL;
   1211          int res;
   1212
   1213          WARN_ON_ONCE((mode & FMODE_EXCL) && !holder);
   1214
   1215          if ((mode & FMODE_EXCL) && holder) {
   1216                  whole = bd_start_claiming(bdev, holder);
   1217                  if (IS_ERR(whole)) {
   1218                          bdput(bdev);
   1219                          return PTR_ERR(whole);
   1220                  }
   1221          }
   1222
   1223          res = __blkdev_get(bdev, mode, 0);
   1224
   1225          if (whole) {
   1226                  if (res == 0)
                             ^^^^^^^^

   1227                          bd_finish_claiming(bdev, whole, holder);
   1228                  else
   1229                          bd_abort_claiming(whole, holder);
                                                   ^^^^^^^^^^^^^
If __blkdev_get() then this doesn't dereference "bdev" so it's not a
use after free bug.

   1230          }
   1231
   1232          return res;
   1233  }

So far as I can see the Fixes tag should be what I said earlier.

Fixes: 89e524c04fa9 ("loop: Fix mount(2) failure due to race with LOOP_SET_FD")


I tried kernel before this commit and can still reproduce this issue.

After some digging, at last I found this one:
77ea887e433a "implement in-kernel gendisk events handling"

@@ -1158,9 +1159,10 @@ int blkdev_get(struct block_device *bdev, fmode_t mode, void *holder)

        if (whole) {
                /* finish claiming */
+               mutex_lock(&bdev->bd_mutex);
                spin_lock(&bdev_lock);

-               if (res == 0) {
+               if (!res) {
                        BUG_ON(!bd_may_claim(bdev, whole, holder));
                        /*
                         * Note that for a whole device bd_holders


This commit added accessing bdev->bd_mutex before checking res, which will cause use-after-free. So I think the fixes tag should be:

Fixes: 77ea887e433a ("implement in-kernel gendisk events handling")

Otherwise the patch looks good to me.

Reviewed-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>

regards,
dan carpenter

.





[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux