[v2,(RESEND)] block: genhd: fix double kfree() in __alloc_disk_node()

Commit Message

Tetsuo Handa Oct. 2, 2021, 9:23 a.m. UTC

syzbot is reporting use-after-free read at bdev_free_inode() [1], for
kfree() from __alloc_disk_node() is called before bdev_free_inode()
(which is called after RCU grace period) reads bdev->bd_disk and calls
kfree(bdev->bd_disk).

Fix use-after-free read followed by double kfree() problem
by making sure that bdev->bd_disk is NULL when calling iput().

Link: https://syzkaller.appspot.com/bug?extid=8281086e8a6fbfbd952a [1]
Reported-by: syzbot <[e-mail]>
Signed-off-by: Tetsuo Handa <[e-mail]>
Reviewed-by: Christoph Hellwig <[e-mail]>
---
Changes in v2:
  Defer bdev->bd_disk assignment instead of resetting to NULL in bdev_alloc().

 block/bdev.c  | 2 +-
 block/genhd.c | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

Comments

Jens Axboe Oct. 2, 2021, 1:29 p.m. UTC | #1

On 10/2/21 3:23 AM, Tetsuo Handa wrote:
> syzbot is reporting use-after-free read at bdev_free_inode() [1], for
> kfree() from __alloc_disk_node() is called before bdev_free_inode()
> (which is called after RCU grace period) reads bdev->bd_disk and calls
> kfree(bdev->bd_disk).
> 
> Fix use-after-free read followed by double kfree() problem
> by making sure that bdev->bd_disk is NULL when calling iput().

Applied for 5.15, thanks.

Patch

diff --git a/block/bdev.c b/block/bdev.c
index cf2780cb44a7..485a258b0ab3 100644
--- a/block/bdev.c
+++ b/block/bdev.c
@@ -490,7 +490,6 @@  struct block_device *bdev_alloc(struct gendisk *disk, u8 partno)
 	bdev = I_BDEV(inode);
 	mutex_init(&bdev->bd_fsfreeze_mutex);
 	spin_lock_init(&bdev->bd_size_lock);
-	bdev->bd_disk = disk;
 	bdev->bd_partno = partno;
 	bdev->bd_inode = inode;
 	bdev->bd_stats = alloc_percpu(struct disk_stats);
@@ -498,6 +497,7 @@  struct block_device *bdev_alloc(struct gendisk *disk, u8 partno)
 		iput(inode);
 		return NULL;
 	}
+	bdev->bd_disk = disk;
 	return bdev;
 }
 
diff --git a/block/genhd.c b/block/genhd.c
index 7b6e5e1cf956..496e8458c357 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1268,6 +1268,7 @@  struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
 
 out_destroy_part_tbl:
 	xa_destroy(&disk->part_tbl);
+	disk->part0->bd_disk = NULL;
 	iput(disk->part0->bd_inode);
 out_free_bdi:
 	bdi_put(disk->bdi);