summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDave Jones <davej@redhat.com>2012-02-20 20:32:37 -0500
committerDave Jones <davej@redhat.com>2012-02-20 20:32:37 -0500
commit43a44e9595f4ff57fb3c7ce29b75a4563dc56ad0 (patch)
tree0001227dffe1602bd33adf690e4c049292a5aa45
parentc10e53789bf2d500b01ff7f9cb94ac4d9c22ef5f (diff)
downloadkernel-43a44e9595f4ff57fb3c7ce29b75a4563dc56ad0.tar.gz
kernel-43a44e9595f4ff57fb3c7ce29b75a4563dc56ad0.tar.xz
kernel-43a44e9595f4ff57fb3c7ce29b75a4563dc56ad0.zip
Do not call drivers when invalidating partitions for -ENOMEDIUM
-rw-r--r--kernel.spec15
-rw-r--r--scsi-fix-sd_revalidate_disk-oops.patch187
2 files changed, 197 insertions, 5 deletions
diff --git a/kernel.spec b/kernel.spec
index 80fc7b95a..c4bf4b5dc 100644
--- a/kernel.spec
+++ b/kernel.spec
@@ -762,18 +762,19 @@ Patch21234: Bluetooth-Remove-bogus-inline-decl-from-l2cap_chan_connect.patch
#rhbz 754518
Patch21235: scsi-sd_revalidate_disk-prevent-NULL-ptr-deref.patch
+Patch21236: scsi-fix-sd_revalidate_disk-oops.patch
#rhbz 714828
-Patch21236: autofs4-lockdep.patch
+Patch21240: autofs4-lockdep.patch
-Patch21237: mcelog-rcu-splat.patch
-Patch21238: x86-Avoid-invoking-RCU-when-CPU-is-idle.patch
+Patch21250: mcelog-rcu-splat.patch
+Patch21260: x86-Avoid-invoking-RCU-when-CPU-is-idle.patch
#rhbz 790367
-Patch21239: s390x-enable-keys-compat.patch
+Patch21270: s390x-enable-keys-compat.patch
#rhbz 795544
-Patch21240: ums_realtek-do-not-use-stack-memory-for-DMA-in-__do_.patch
+Patch21280: ums_realtek-do-not-use-stack-memory-for-DMA-in-__do_.patch
# compat-wireless patches
Patch50000: compat-wireless-config-fixups.patch
@@ -1485,6 +1486,7 @@ ApplyPatch Bluetooth-Remove-bogus-inline-decl-from-l2cap_chan_connect.patch
#rhbz 754518
ApplyPatch scsi-sd_revalidate_disk-prevent-NULL-ptr-deref.patch
+ApplyPatch scsi-fix-sd_revalidate_disk-oops.patch
#rhbz 714828
ApplyPatch autofs4-lockdep.patch
@@ -2376,6 +2378,9 @@ fi
# ||----w |
# || ||
%changelog
+* Mon Feb 20 2012 Dave Jones <davej@redhat.com>
+- Do not call drivers when invalidating partitions for -ENOMEDIUM
+
* Mon Feb 20 2012 Josh Boyer <jwboyer@redhat.com>
- Avoid using stack variables in ums_realtek (again) (rhbz 795544)
diff --git a/scsi-fix-sd_revalidate_disk-oops.patch b/scsi-fix-sd_revalidate_disk-oops.patch
new file mode 100644
index 000000000..efc657c7f
--- /dev/null
+++ b/scsi-fix-sd_revalidate_disk-oops.patch
@@ -0,0 +1,187 @@
+Hi,
+
+Thank you for review and comments.
+
+On 02/16/12 02:26, Tejun Heo wrote:
+> On Wed, Feb 15, 2012 at 11:56:19AM +0900, Jun'ichi Nomura wrote:
+>> +int invalidate_partitions(struct gendisk *disk, struct block_device *bdev)
+>> +{
+>> + int res;
+>> +
+>> + res = drop_partitions(disk, bdev);
+>> + if (res)
+>> + return res;
+>> +
+>
+> Hmmm... shouldn't we have set_capacity(disk, 0) here?
+
+Added.
+I wasn't sure whether I should leave it to drivers.
+But it seems capacity 0 for ENOMEDIUM device is reasonable.
+
+>> + check_disk_size_change(disk, bdev);
+>> + bdev->bd_invalidated = 0;
+>> + /* tell userspace that the media / partition table may have changed */
+>> + kobject_uevent(&disk_to_dev(disk)->kobj, KOBJ_CHANGE);
+>
+> Also, we really shouldn't be generating KOBJ_CHANGE after every
+> -ENOMEDIUM open. This can easily lead to infinite loop. We should
+> generate this iff we actually dropped partitions && modified the size.
+
+invalidate_partitions() is called only when bd_invalidated is set.
+So KOBJ_CHANGE is not raised for every ENOMEDIUM open.
+
+I put it explicit in the function to make it safer for
+possible misuse.
+
+How about this?
+
+---------------------------------------------------------
+Do not call drivers when invalidating partitions for -ENOMEDIUM
+
+When a scsi driver returns -ENOMEDIUM for open(),
+__blkdev_get() calls rescan_partitions(), which ends up calling
+sd_revalidate_disk() without getting a refcount of scsi_device.
+
+That could lead to oops like this:
+
+ process A process B
+ ----------------------------------------------
+ sys_open
+ __blkdev_get
+ sd_open
+ returns -ENOMEDIUM
+ scsi_remove_device
+ <scsi_device torn down>
+ rescan_partitions
+ sd_revalidate_disk
+ <oops>
+
+Oopses are reported here:
+http://marc.info/?l=linux-scsi&m=132388619710052
+
+This patch separates the partition invalidation from rescan_partitions()
+and use it for -ENOMEDIUM case.
+
+Index: linux-3.3/block/partition-generic.c
+===================================================================
+--- linux-3.3.orig/block/partition-generic.c 2012-02-15 09:00:25.147293790 +0900
++++ linux-3.3/block/partition-generic.c 2012-02-16 10:48:22.257680685 +0900
+@@ -389,17 +389,11 @@ static bool disk_unlock_native_capacity(
+ }
+ }
+
+-int rescan_partitions(struct gendisk *disk, struct block_device *bdev)
++static int drop_partitions(struct gendisk *disk, struct block_device *bdev)
+ {
+- struct parsed_partitions *state = NULL;
+ struct disk_part_iter piter;
+ struct hd_struct *part;
+- int p, highest, res;
+-rescan:
+- if (state && !IS_ERR(state)) {
+- kfree(state);
+- state = NULL;
+- }
++ int res;
+
+ if (bdev->bd_part_count)
+ return -EBUSY;
+@@ -412,6 +406,24 @@ rescan:
+ delete_partition(disk, part->partno);
+ disk_part_iter_exit(&piter);
+
++ return 0;
++}
++
++int rescan_partitions(struct gendisk *disk, struct block_device *bdev)
++{
++ struct parsed_partitions *state = NULL;
++ struct hd_struct *part;
++ int p, highest, res;
++rescan:
++ if (state && !IS_ERR(state)) {
++ kfree(state);
++ state = NULL;
++ }
++
++ res = drop_partitions(disk, bdev);
++ if (res)
++ return res;
++
+ if (disk->fops->revalidate_disk)
+ disk->fops->revalidate_disk(disk);
+ check_disk_size_change(disk, bdev);
+@@ -515,6 +527,26 @@ rescan:
+ return 0;
+ }
+
++int invalidate_partitions(struct gendisk *disk, struct block_device *bdev)
++{
++ int res;
++
++ if (!bdev->bd_invalidated)
++ return 0;
++
++ res = drop_partitions(disk, bdev);
++ if (res)
++ return res;
++
++ set_capacity(disk, 0);
++ check_disk_size_change(disk, bdev);
++ bdev->bd_invalidated = 0;
++ /* tell userspace that the media / partition table may have changed */
++ kobject_uevent(&disk_to_dev(disk)->kobj, KOBJ_CHANGE);
++
++ return 0;
++}
++
+ unsigned char *read_dev_sector(struct block_device *bdev, sector_t n, Sector *p)
+ {
+ struct address_space *mapping = bdev->bd_inode->i_mapping;
+Index: linux-3.3/include/linux/genhd.h
+===================================================================
+--- linux-3.3.orig/include/linux/genhd.h 2012-02-09 12:21:53.000000000 +0900
++++ linux-3.3/include/linux/genhd.h 2012-02-16 10:47:43.783681813 +0900
+@@ -596,6 +596,7 @@ extern char *disk_name (struct gendisk *
+
+ extern int disk_expand_part_tbl(struct gendisk *disk, int target);
+ extern int rescan_partitions(struct gendisk *disk, struct block_device *bdev);
++extern int invalidate_partitions(struct gendisk *disk, struct block_device *bdev);
+ extern struct hd_struct * __must_check add_partition(struct gendisk *disk,
+ int partno, sector_t start,
+ sector_t len, int flags,
+Index: linux-3.3/fs/block_dev.c
+===================================================================
+--- linux-3.3.orig/fs/block_dev.c 2012-02-09 12:21:53.000000000 +0900
++++ linux-3.3/fs/block_dev.c 2012-02-16 10:47:52.602681441 +0900
+@@ -1183,8 +1183,12 @@ static int __blkdev_get(struct block_dev
+ * The latter is necessary to prevent ghost
+ * partitions on a removed medium.
+ */
+- if (bdev->bd_invalidated && (!ret || ret == -ENOMEDIUM))
+- rescan_partitions(disk, bdev);
++ if (bdev->bd_invalidated) {
++ if (!ret)
++ rescan_partitions(disk, bdev);
++ else if (ret == -ENOMEDIUM)
++ invalidate_partitions(disk, bdev);
++ }
+ if (ret)
+ goto out_clear;
+ } else {
+@@ -1214,8 +1218,12 @@ static int __blkdev_get(struct block_dev
+ if (bdev->bd_disk->fops->open)
+ ret = bdev->bd_disk->fops->open(bdev, mode);
+ /* the same as first opener case, read comment there */
+- if (bdev->bd_invalidated && (!ret || ret == -ENOMEDIUM))
+- rescan_partitions(bdev->bd_disk, bdev);
++ if (bdev->bd_invalidated) {
++ if (!ret)
++ rescan_partitions(bdev->bd_disk, bdev);
++ else if (ret == -ENOMEDIUM)
++ invalidate_partitions(bdev->bd_disk, bdev);
++ }
+ if (ret)
+ goto out_unlock_bdev;
+ }