From c1d390d8e6128b050f0f66b1c33d390760deb3f4 Mon Sep 17 00:00:00 2001 From: Xiaotian Feng Date: Tue, 4 Dec 2012 19:33:54 +0800 Subject: [PATCH 1/2] megaraid: fix BUG_ON() from incorrect use of delayed work megaraid use INIT_WORK to declare a hotplug_work, but cast the hotplug_work from work_struct to delayed_work and schedule_delayed_work on it. This is very dangerous, as other part of delayed_work might be kernel memories allocated by others. With commit 8852aac ("workqueue: mod_delayed_work_on() shouldn't queue timer on 0 delay"), schedule_delayed_work() will check dwork->timer before queue_work even when @delay is 0, this causes megaraid code to hit the BUG_ON() in workqueue code. Change megaraid code to use delayed work. Signed-off-by: Xiaotian Feng Signed-off-by: Tejun Heo Cc: Neela Syam Kolli Cc: "James E.J. Bottomley" Cc: linux-scsi@vger.kernel.org --- drivers/scsi/megaraid/megaraid_sas.h | 2 +- drivers/scsi/megaraid/megaraid_sas_base.c | 14 ++++++-------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/drivers/scsi/megaraid/megaraid_sas.h b/drivers/scsi/megaraid/megaraid_sas.h index 16b7a72a70c4..3b2365c8eab2 100644 --- a/drivers/scsi/megaraid/megaraid_sas.h +++ b/drivers/scsi/megaraid/megaraid_sas.h @@ -1276,7 +1276,7 @@ struct megasas_evt_detail { } __attribute__ ((packed)); struct megasas_aen_event { - struct work_struct hotplug_work; + struct delayed_work hotplug_work; struct megasas_instance *instance; }; diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c index d2c5366aff7f..e4f2baacf1e1 100644 --- a/drivers/scsi/megaraid/megaraid_sas_base.c +++ b/drivers/scsi/megaraid/megaraid_sas_base.c @@ -2060,9 +2060,9 @@ megasas_service_aen(struct megasas_instance *instance, struct megasas_cmd *cmd) } else { ev->instance = instance; instance->ev = ev; - INIT_WORK(&ev->hotplug_work, megasas_aen_polling); - schedule_delayed_work( - (struct delayed_work *)&ev->hotplug_work, 0); + INIT_DELAYED_WORK(&ev->hotplug_work, + megasas_aen_polling); + schedule_delayed_work(&ev->hotplug_work, 0); } } } @@ -4352,8 +4352,7 @@ megasas_suspend(struct pci_dev *pdev, pm_message_t state) /* cancel the delayed work if this work still in queue */ if (instance->ev != NULL) { struct megasas_aen_event *ev = instance->ev; - cancel_delayed_work_sync( - (struct delayed_work *)&ev->hotplug_work); + cancel_delayed_work_sync(&ev->hotplug_work); instance->ev = NULL; } @@ -4545,8 +4544,7 @@ static void __devexit megasas_detach_one(struct pci_dev *pdev) /* cancel the delayed work if this work still in queue*/ if (instance->ev != NULL) { struct megasas_aen_event *ev = instance->ev; - cancel_delayed_work_sync( - (struct delayed_work *)&ev->hotplug_work); + cancel_delayed_work_sync(&ev->hotplug_work); instance->ev = NULL; } @@ -5190,7 +5188,7 @@ static void megasas_aen_polling(struct work_struct *work) { struct megasas_aen_event *ev = - container_of(work, struct megasas_aen_event, hotplug_work); + container_of(work, struct megasas_aen_event, hotplug_work.work); struct megasas_instance *instance = ev->instance; union megasas_evt_class_locale class_locale; struct Scsi_Host *host; From fc4b514f2727f74a4587c31db87e0e93465518c3 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Tue, 4 Dec 2012 07:40:39 -0800 Subject: [PATCH 2/2] workqueue: convert BUG_ON()s in __queue_delayed_work() to WARN_ON_ONCE()s 8852aac25e ("workqueue: mod_delayed_work_on() shouldn't queue timer on 0 delay") unexpectedly uncovered a very nasty abuse of delayed_work in megaraid - it allocated work_struct, casted it to delayed_work and then pass that into queue_delayed_work(). Previously, this was okay because 0 @delay short-circuited to queue_work() before doing anything with delayed_work. 8852aac25e moved 0 @delay test into __queue_delayed_work() after sanity check on delayed_work making megaraid trigger BUG_ON(). Although megaraid is already fixed by c1d390d8e6 ("megaraid: fix BUG_ON() from incorrect use of delayed work"), this patch converts BUG_ON()s in __queue_delayed_work() to WARN_ON_ONCE()s so that such abusers, if there are more, trigger warning but don't crash the machine. Signed-off-by: Tejun Heo Cc: Xiaotian Feng --- kernel/workqueue.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/workqueue.c b/kernel/workqueue.c index 084aa47bac82..1dae900df798 100644 --- a/kernel/workqueue.c +++ b/kernel/workqueue.c @@ -1361,8 +1361,8 @@ static void __queue_delayed_work(int cpu, struct workqueue_struct *wq, WARN_ON_ONCE(timer->function != delayed_work_timer_fn || timer->data != (unsigned long)dwork); - BUG_ON(timer_pending(timer)); - BUG_ON(!list_empty(&work->entry)); + WARN_ON_ONCE(timer_pending(timer)); + WARN_ON_ONCE(!list_empty(&work->entry)); /* * If @delay is 0, queue @dwork->work immediately. This is for