linux-kernel - block: Fix oops caused by __blkdev_get() calling disk_unblock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20110309153859.GD27010@htj.dyndns.org>
Date:	Wed, 9 Mar 2011 16:38:59 +0100
From:	Tejun Heo <tj@...nel.org>
To:	Jens Axboe <jaxboe@...ionio.com>
Cc:	linux-kernel@...r.kernel.org, kay.sievers@...y.org,
	hch@...radead.org
Subject: block: Fix oops caused by __blkdev_get() calling
 disk_unblock_events() with invalid @disk

Commit 57c966b8b2 (block: Don't check events while open is in
progress) made __blkdev_get() block events around open calls; however,
it used invalid @disk pointer in the following cases.

* When ->open() returns -ERESTARTSYS, disk_unblock_events() is called
  after @disk is put.  @disk may be invalid by the time unblock is
  called.

  This is fixed by moving references after disk_unblock_events().

* When there are multiple openers, @disk is cleared to %NULL and later
  disk_unblock_disk() is called with %NULL @disk causing oops.

  This is fixed by moving reference putting after open success is
  determined and not clearing @disk to %NULL.  On success, @disk is
  valid because there is another opener holding reference to it.  On
  failure, the references are put after disk_unblock_events() is
  called.

Signed-off-by: Tejun Heo <tj@...nel.org>
Reported-by: Jens Axboe <axboe@...nel.dk>
---
Ah, sorry about that.  My test setup didn't include a device with
multiple partitions on it so the more than one opener case never
triggered.

Thanks.

 fs/block_dev.c |   13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

Index: work/fs/block_dev.c
===================================================================
--- work.orig/fs/block_dev.c
+++ work/fs/block_dev.c
@@ -1109,11 +1109,11 @@ static int __blkdev_get(struct block_dev
 					 */
 					disk_put_part(bdev->bd_part);
 					bdev->bd_part = NULL;
-					module_put(disk->fops->owner);
-					put_disk(disk);
 					bdev->bd_disk = NULL;
 					mutex_unlock(&bdev->bd_mutex);
 					disk_unblock_events(disk);
+					module_put(disk->fops->owner);
+					put_disk(disk);
 					goto restart;
 				}
 				if (ret)
@@ -1150,9 +1150,6 @@ static int __blkdev_get(struct block_dev
 			bd_set_size(bdev, (loff_t)bdev->bd_part->nr_sects << 9);
 		}
 	} else {
-		module_put(disk->fops->owner);
-		put_disk(disk);
-		disk = NULL;
 		if (bdev->bd_contains == bdev) {
 			if (bdev->bd_disk->fops->open) {
 				ret = bdev->bd_disk->fops->open(bdev, mode);
@@ -1162,6 +1159,9 @@ static int __blkdev_get(struct block_dev
 			if (bdev->bd_invalidated)
 				rescan_partitions(bdev->bd_disk, bdev);
 		}
+		/* only one opener holds refs to the module and disk */
+		module_put(disk->fops->owner);
+		put_disk(disk);
 	}
 	bdev->bd_openers++;
 	if (for_part)
@@ -1182,8 +1182,7 @@ static int __blkdev_get(struct block_dev
 	mutex_unlock(&bdev->bd_mutex);
 	disk_unblock_events(disk);
  out:
-	if (disk)
-		module_put(disk->fops->owner);
+	module_put(disk->fops->owner);
 	put_disk(disk);
 	bdput(bdev);
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/