diff options
author | NeilBrown <neilb@suse.de> | 2011-09-21 01:30:20 -0400 |
---|---|---|
committer | NeilBrown <neilb@suse.de> | 2011-09-21 01:30:20 -0400 |
commit | 01f96c0a9922cd9919baf9d16febdf7016177a12 (patch) | |
tree | a877fe509c4ef0db5252b7192df56009c1d06d6f /drivers/md/md.c | |
parent | 27a7b260f71439c40546b43588448faac01adb93 (diff) |
md: Avoid waking up a thread after it has been freed.
Two related problems:
1/ some error paths call "md_unregister_thread(mddev->thread)"
without subsequently clearing ->thread. A subsequent call
to mddev_unlock will try to wake the thread, and crash.
2/ Most calls to md_wakeup_thread are protected against the thread
disappeared either by:
- holding the ->mutex
- having an active request, so something else must be keeping
the array active.
However mddev_unlock calls md_wakeup_thread after dropping the
mutex and without any certainty of an active request, so the
->thread could theoretically disappear.
So we need a spinlock to provide some protections.
So change md_unregister_thread to take a pointer to the thread
pointer, and ensure that it always does the required locking, and
clears the pointer properly.
Reported-by: "Moshe Melnikov" <moshe@zadarastorage.com>
Signed-off-by: NeilBrown <neilb@suse.de>
cc: stable@kernel.org
Diffstat (limited to 'drivers/md/md.c')
-rw-r--r-- | drivers/md/md.c | 22 |
1 files changed, 19 insertions, 3 deletions
diff --git a/drivers/md/md.c b/drivers/md/md.c index 5404b2295820..5c95ccb59500 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c | |||
@@ -61,6 +61,11 @@ | |||
61 | static void autostart_arrays(int part); | 61 | static void autostart_arrays(int part); |
62 | #endif | 62 | #endif |
63 | 63 | ||
64 | /* pers_list is a list of registered personalities protected | ||
65 | * by pers_lock. | ||
66 | * pers_lock does extra service to protect accesses to | ||
67 | * mddev->thread when the mutex cannot be held. | ||
68 | */ | ||
64 | static LIST_HEAD(pers_list); | 69 | static LIST_HEAD(pers_list); |
65 | static DEFINE_SPINLOCK(pers_lock); | 70 | static DEFINE_SPINLOCK(pers_lock); |
66 | 71 | ||
@@ -739,7 +744,12 @@ static void mddev_unlock(mddev_t * mddev) | |||
739 | } else | 744 | } else |
740 | mutex_unlock(&mddev->reconfig_mutex); | 745 | mutex_unlock(&mddev->reconfig_mutex); |
741 | 746 | ||
747 | /* was we've dropped the mutex we need a spinlock to | ||
748 | * make sur the thread doesn't disappear | ||
749 | */ | ||
750 | spin_lock(&pers_lock); | ||
742 | md_wakeup_thread(mddev->thread); | 751 | md_wakeup_thread(mddev->thread); |
752 | spin_unlock(&pers_lock); | ||
743 | } | 753 | } |
744 | 754 | ||
745 | static mdk_rdev_t * find_rdev_nr(mddev_t *mddev, int nr) | 755 | static mdk_rdev_t * find_rdev_nr(mddev_t *mddev, int nr) |
@@ -6429,11 +6439,18 @@ mdk_thread_t *md_register_thread(void (*run) (mddev_t *), mddev_t *mddev, | |||
6429 | return thread; | 6439 | return thread; |
6430 | } | 6440 | } |
6431 | 6441 | ||
6432 | void md_unregister_thread(mdk_thread_t *thread) | 6442 | void md_unregister_thread(mdk_thread_t **threadp) |
6433 | { | 6443 | { |
6444 | mdk_thread_t *thread = *threadp; | ||
6434 | if (!thread) | 6445 | if (!thread) |
6435 | return; | 6446 | return; |
6436 | dprintk("interrupting MD-thread pid %d\n", task_pid_nr(thread->tsk)); | 6447 | dprintk("interrupting MD-thread pid %d\n", task_pid_nr(thread->tsk)); |
6448 | /* Locking ensures that mddev_unlock does not wake_up a | ||
6449 | * non-existent thread | ||
6450 | */ | ||
6451 | spin_lock(&pers_lock); | ||
6452 | *threadp = NULL; | ||
6453 | spin_unlock(&pers_lock); | ||
6437 | 6454 | ||
6438 | kthread_stop(thread->tsk); | 6455 | kthread_stop(thread->tsk); |
6439 | kfree(thread); | 6456 | kfree(thread); |
@@ -7340,8 +7357,7 @@ static void reap_sync_thread(mddev_t *mddev) | |||
7340 | mdk_rdev_t *rdev; | 7357 | mdk_rdev_t *rdev; |
7341 | 7358 | ||
7342 | /* resync has finished, collect result */ | 7359 | /* resync has finished, collect result */ |
7343 | md_unregister_thread(mddev->sync_thread); | 7360 | md_unregister_thread(&mddev->sync_thread); |
7344 | mddev->sync_thread = NULL; | ||
7345 | if (!test_bit(MD_RECOVERY_INTR, &mddev->recovery) && | 7361 | if (!test_bit(MD_RECOVERY_INTR, &mddev->recovery) && |
7346 | !test_bit(MD_RECOVERY_REQUESTED, &mddev->recovery)) { | 7362 | !test_bit(MD_RECOVERY_REQUESTED, &mddev->recovery)) { |
7347 | /* success...*/ | 7363 | /* success...*/ |