diff options
author | Anatol Pomozov <anatol.pomozov@gmail.com> | 2013-04-01 12:47:56 -0400 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2013-04-01 18:48:47 -0400 |
commit | c1681bf8a7b1b98edee8b862a42c19c4e53205fd (patch) | |
tree | e97d3b2bfcdc7e681a682fb28a091402dc2e4b77 /drivers/block/loop.c | |
parent | aae92db9f0f67559198f6ecde922f15e2aeea7e0 (diff) |
loop: prevent bdev freeing while device in use
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
stack:
BUG: unable to handle kernel NULL pointer dereference at 0000000000000280
bd_set_size+0x10/0xa0
loop_clr_fd+0x1f8/0x420 [loop]
lo_ioctl+0x200/0x7e0 [loop]
lo_compat_ioctl+0x47/0xe0 [loop]
compat_blkdev_ioctl+0x341/0x1290
do_filp_open+0x42/0xa0
compat_sys_ioctl+0xc1/0xf20
do_sys_open+0x16e/0x1d0
sysenter_dispatch+0x7/0x1a
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
done
[ 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 <anatol.pomozov@gmail.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'drivers/block/loop.c')
-rw-r--r-- | drivers/block/loop.c | 9 |
1 files changed, 8 insertions, 1 deletions
diff --git a/drivers/block/loop.c b/drivers/block/loop.c index fe5f6403417f..2c127f9c3f3b 100644 --- a/drivers/block/loop.c +++ b/drivers/block/loop.c | |||
@@ -922,6 +922,11 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode, | |||
922 | lo->lo_flags |= LO_FLAGS_PARTSCAN; | 922 | lo->lo_flags |= LO_FLAGS_PARTSCAN; |
923 | if (lo->lo_flags & LO_FLAGS_PARTSCAN) | 923 | if (lo->lo_flags & LO_FLAGS_PARTSCAN) |
924 | ioctl_by_bdev(bdev, BLKRRPART, 0); | 924 | ioctl_by_bdev(bdev, BLKRRPART, 0); |
925 | |||
926 | /* Grab the block_device to prevent its destruction after we | ||
927 | * put /dev/loopXX inode. Later in loop_clr_fd() we bdput(bdev). | ||
928 | */ | ||
929 | bdgrab(bdev); | ||
925 | return 0; | 930 | return 0; |
926 | 931 | ||
927 | out_clr: | 932 | out_clr: |
@@ -1031,8 +1036,10 @@ static int loop_clr_fd(struct loop_device *lo) | |||
1031 | memset(lo->lo_encrypt_key, 0, LO_KEY_SIZE); | 1036 | memset(lo->lo_encrypt_key, 0, LO_KEY_SIZE); |
1032 | memset(lo->lo_crypt_name, 0, LO_NAME_SIZE); | 1037 | memset(lo->lo_crypt_name, 0, LO_NAME_SIZE); |
1033 | memset(lo->lo_file_name, 0, LO_NAME_SIZE); | 1038 | memset(lo->lo_file_name, 0, LO_NAME_SIZE); |
1034 | if (bdev) | 1039 | if (bdev) { |
1040 | bdput(bdev); | ||
1035 | invalidate_bdev(bdev); | 1041 | invalidate_bdev(bdev); |
1042 | } | ||
1036 | set_capacity(lo->lo_disk, 0); | 1043 | set_capacity(lo->lo_disk, 0); |
1037 | loop_sysfs_exit(lo); | 1044 | loop_sysfs_exit(lo); |
1038 | if (bdev) { | 1045 | if (bdev) { |