Patchwork [3.5.y.z,extended,stable] Patch "loop: prevent bdev freeing while device in use" has been added to staging queue

mail settings
Submitter Luis Henriques
Date April 4, 2013, 1:26 p.m.
Message ID <>
Download mbox | patch
Permalink /patch/233816/
State New
Headers show


Luis Henriques - April 4, 2013, 1:26 p.m.
This is a note to let you know that I have just added a patch titled

    loop: prevent bdev freeing while device in use

to the linux-3.5.y-queue branch of the 3.5.y.z extended stable tree 
which can be found at:;a=shortlog;h=refs/heads/linux-3.5.y-queue

If you, or anyone else, feels it should not be added to this tree, please 
reply to this email.

For more information about the 3.5.y.z tree, see



From 3cf0f498b6aa73ecfeafdb57758e067b8c267f33 Mon Sep 17 00:00:00 2001
From: Anatol Pomozov <>
Date: Mon, 1 Apr 2013 09:47:56 -0700
Subject: [PATCH] loop: prevent bdev freeing while device in use

commit c1681bf8a7b1b98edee8b862a42c19c4e53205fd upstream.

struct block_device lifecycle is defined by its inode (see fs/block_dev.c) -
block_device allocated first time we access /dev/loopXX and deallocated on
bdev_destroy_inode. When we create the device "losetup /dev/loopXX afile"
we want that block_device stay alive until we destroy the loop device
with "losetup -d".

But because we do not hold /dev/loopXX inode its counter goes 0, and
inode/bdev can be destroyed at any moment. Usually it happens at memory
pressure or when user drops inode cache (like in the test below). When later in
loop_clr_fd() we want to use bdev we have use-after-free error with following

BUG: unable to handle kernel NULL pointer dereference at 0000000000000280
  loop_clr_fd+0x1f8/0x420 [loop]
  lo_ioctl+0x200/0x7e0 [loop]
  lo_compat_ioctl+0x47/0xe0 [loop]

To prevent use-after-free we need to grab the device in loop_set_fd()
and put it later in loop_clr_fd().

The issue is reprodusible on current Linus head and v3.3. Here is the test:

  dd if=/dev/zero of=loop.file bs=1M count=1
  while [ true ]; do
    losetup /dev/loop0 loop.file
    echo 2 > /proc/sys/vm/drop_caches
    losetup -d /dev/loop0

[ Doing bdgrab/bput in loop_set_fd/loop_clr_fd is safe, because every
  time we call loop_set_fd() we check that loop_device->lo_state is
  Lo_unbound and set it to Lo_bound If somebody will try to set_fd again
  it will get EBUSY.  And if we try to loop_clr_fd() on unbound loop
  device we'll get ENXIO.

  loop_set_fd/loop_clr_fd (and any other loop ioctl) is called under
  loop_device->lo_ctl_mutex. ]

Signed-off-by: Anatol Pomozov <>
Cc: Al Viro <>
Signed-off-by: Linus Torvalds <>
Signed-off-by: Luis Henriques <>
 drivers/block/loop.c | 9 ++++++++-
 fs/block_dev.c       | 1 +
 2 files changed, 9 insertions(+), 1 deletion(-)



diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index a77d9a5..c5e0f73 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -908,6 +908,11 @@  static int loop_set_fd(struct loop_device *lo, fmode_t mode,
 		lo->lo_flags |= LO_FLAGS_PARTSCAN;
 	if (lo->lo_flags & LO_FLAGS_PARTSCAN)
 		ioctl_by_bdev(bdev, BLKRRPART, 0);
+	/* Grab the block_device to prevent its destruction after we
+	 * put /dev/loopXX inode. Later in loop_clr_fd() we bdput(bdev).
+	 */
+	bdgrab(bdev);
 	return 0;

@@ -1004,8 +1009,10 @@  static int loop_clr_fd(struct loop_device *lo)
 	memset(lo->lo_encrypt_key, 0, LO_KEY_SIZE);
 	memset(lo->lo_crypt_name, 0, LO_NAME_SIZE);
 	memset(lo->lo_file_name, 0, LO_NAME_SIZE);
-	if (bdev)
+	if (bdev) {
+		bdput(bdev);
+	}
 	set_capacity(lo->lo_disk, 0);
 	if (bdev) {
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 77e86b3..1244094 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -604,6 +604,7 @@  struct block_device *bdgrab(struct block_device *bdev)
 	return bdev;

 long nr_blockdev_pages(void)