lkml.org 
[lkml]   [2012]   [Feb]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[RFC] block: fix __blkdev_get and add_disk race condition
The following situation might occur:

__blkdev_get: add_disk:

register_disk()
get_gendisk()

disk_block_events()
disk->ev == NULL

disk_add_events()

__disk_unblock_events()
disk->ev != NULL
--ev->block

Then we unblock events, when they are suppose to be blocked. This
can trigger the warning, but also can crash in sd_check_events()
or other places.

I'm able to reproduce crashes with the following scripts (with
conneted usb dongle as sdb disk).

<snip>
DEV=/dev/sdb
ENABLE=/sys/bus/usb/devices/1-2/bConfigurationValue

function stop_me()
{
for i in `jobs -p` ; do kill $i 2> /dev/null ; done
exit
}

trap stop_me SIGHUP SIGINT SIGTERM

for ((i = 0; i < 10; i++)) ; do
while true; do fdisk -l $DEV 2>&1 > /dev/null ; done &
done

while true ; do
echo 1 > $ENABLE
sleep 1
echo 0 > $ENABLE
done
</snip>

I use the script to verify patch fixing oops in sd_revalidate_disk
http://marc.info/?l=linux-scsi&m=132935572512352&w=2
Without Jun'ichi patch or this one, script easily crash kernel
within a few seconds. With both patches applied I do not observe
crash. Unfortunately after some time (dozen of minutes), script
will hung in:

[ 1563.906432] [<c08354f5>] schedule_timeout_uninterruptible+0x15/0x20
[ 1563.906437] [<c04532d5>] msleep+0x15/0x20
[ 1563.906443] [<c05d60b2>] blk_drain_queue+0x32/0xd0
[ 1563.906447] [<c05d6e00>] blk_cleanup_queue+0xd0/0x170
[ 1563.906454] [<c06d278f>] scsi_free_queue+0x3f/0x60
[ 1563.906459] [<c06d7e6e>] __scsi_remove_device+0x6e/0xb0
[ 1563.906463] [<c06d4aff>] scsi_forget_host+0x4f/0x60
[ 1563.906468] [<c06cd84a>] scsi_remove_host+0x5a/0xf0
[ 1563.906482] [<f7f030fb>] quiesce_and_remove_host+0x5b/0xa0 [usb_storage]
[ 1563.906490] [<f7f03203>] usb_stor_disconnect+0x13/0x20 [usb_storage]

Anyway I think this patch is some step forward.

As drawback, I do not teardown on sysfs file create error, because I do
not know how to nullify disk->ev (since it can be used).

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
block/genhd.c | 32 +++++++++++++++++++-------------
1 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 23b4f70..b26c408 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -35,6 +35,7 @@ static DEFINE_IDR(ext_devt_idr);

static struct device_type disk_type;

+static void disk_alloc_events(struct gendisk *disk);
static void disk_add_events(struct gendisk *disk);
static void disk_del_events(struct gendisk *disk);
static void disk_release_events(struct gendisk *disk);
@@ -601,6 +602,8 @@ void add_disk(struct gendisk *disk)
disk->major = MAJOR(devt);
disk->first_minor = MINOR(devt);

+ disk_alloc_events(disk);
+
/* Register BDI before referencing it from bdev */
bdi = &disk->queue->backing_dev_info;
bdi_register_dev(bdi, disk_devt(disk));
@@ -1733,9 +1736,9 @@ module_param_cb(events_dfl_poll_msecs, &disk_events_dfl_poll_msecs_param_ops,
&disk_events_dfl_poll_msecs, 0644);

/*
- * disk_{add|del|release}_events - initialize and destroy disk_events.
+ * disk_{alloc|add|del|release}_events - initialize and destroy disk_events.
*/
-static void disk_add_events(struct gendisk *disk)
+static void disk_alloc_events(struct gendisk *disk)
{
struct disk_events *ev;

@@ -1748,16 +1751,6 @@ static void disk_add_events(struct gendisk *disk)
return;
}

- if (sysfs_create_files(&disk_to_dev(disk)->kobj,
- disk_events_attrs) < 0) {
- pr_warn("%s: failed to create sysfs files for events\n",
- disk->disk_name);
- kfree(ev);
- return;
- }
-
- disk->ev = ev;
-
INIT_LIST_HEAD(&ev->node);
ev->disk = disk;
spin_lock_init(&ev->lock);
@@ -1766,8 +1759,21 @@ static void disk_add_events(struct gendisk *disk)
ev->poll_msecs = -1;
INIT_DELAYED_WORK(&ev->dwork, disk_events_workfn);

+ disk->ev = ev;
+}
+
+static void disk_add_events(struct gendisk *disk)
+{
+ if (!disk->ev)
+ return;
+
+ /* FIXME: error handling */
+ if (sysfs_create_files(&disk_to_dev(disk)->kobj, disk_events_attrs) < 0)
+ pr_warn("%s: failed to create sysfs files for events\n",
+ disk->disk_name);
+
mutex_lock(&disk_events_mutex);
- list_add_tail(&ev->node, &disk_events);
+ list_add_tail(&disk->ev->node, &disk_events);
mutex_unlock(&disk_events_mutex);

/*
--
1.7.1


\
 
 \ /
  Last update: 2012-02-18 18:29    [W:0.035 / U:0.180 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site