]> git.ipfire.org Git - thirdparty/kernel/stable-queue.git/commitdiff
Fixes for 5.4
authorSasha Levin <sashal@kernel.org>
Mon, 26 Sep 2022 04:58:03 +0000 (00:58 -0400)
committerSasha Levin <sashal@kernel.org>
Mon, 26 Sep 2022 04:58:03 +0000 (00:58 -0400)
Signed-off-by: Sasha Levin <sashal@kernel.org>
queue-5.4/series
queue-5.4/workqueue-don-t-skip-lockdep-work-dependency-in-canc.patch [new file with mode: 0644]

index 038387473f727b4ae886df656c84922308e21c7e..ebab4e10c011c1d8594de2865d5b3abd163e1aaa 100644 (file)
@@ -98,3 +98,4 @@ gpio-ixp4xx-make-irqchip-immutable.patch
 drm-amdgpu-use-dirty-framebuffer-helper.patch
 drm-amd-display-limit-user-regamma-to-a-valid-value.patch
 drm-rockchip-fix-return-type-of-cdn_dp_connector_mod.patch
+workqueue-don-t-skip-lockdep-work-dependency-in-canc.patch
diff --git a/queue-5.4/workqueue-don-t-skip-lockdep-work-dependency-in-canc.patch b/queue-5.4/workqueue-don-t-skip-lockdep-work-dependency-in-canc.patch
new file mode 100644 (file)
index 0000000..3626884
--- /dev/null
@@ -0,0 +1,98 @@
+From e1e5db5c04e995e4857d8aeb3f6660e6859e0956 Mon Sep 17 00:00:00 2001
+From: Sasha Levin <sashal@kernel.org>
+Date: Fri, 29 Jul 2022 13:30:23 +0900
+Subject: workqueue: don't skip lockdep work dependency in cancel_work_sync()
+
+From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
+
+[ Upstream commit c0feea594e058223973db94c1c32a830c9807c86 ]
+
+Like Hillf Danton mentioned
+
+  syzbot should have been able to catch cancel_work_sync() in work context
+  by checking lockdep_map in __flush_work() for both flush and cancel.
+
+in [1], being unable to report an obvious deadlock scenario shown below is
+broken. From locking dependency perspective, sync version of cancel request
+should behave as if flush request, for it waits for completion of work if
+that work has already started execution.
+
+  ----------
+  #include <linux/module.h>
+  #include <linux/sched.h>
+  static DEFINE_MUTEX(mutex);
+  static void work_fn(struct work_struct *work)
+  {
+    schedule_timeout_uninterruptible(HZ / 5);
+    mutex_lock(&mutex);
+    mutex_unlock(&mutex);
+  }
+  static DECLARE_WORK(work, work_fn);
+  static int __init test_init(void)
+  {
+    schedule_work(&work);
+    schedule_timeout_uninterruptible(HZ / 10);
+    mutex_lock(&mutex);
+    cancel_work_sync(&work);
+    mutex_unlock(&mutex);
+    return -EINVAL;
+  }
+  module_init(test_init);
+  MODULE_LICENSE("GPL");
+  ----------
+
+The check this patch restores was added by commit 0976dfc1d0cd80a4
+("workqueue: Catch more locking problems with flush_work()").
+
+Then, lockdep's crossrelease feature was added by commit b09be676e0ff25bd
+("locking/lockdep: Implement the 'crossrelease' feature"). As a result,
+this check was once removed by commit fd1a5b04dfb899f8 ("workqueue: Remove
+now redundant lock acquisitions wrt. workqueue flushes").
+
+But lockdep's crossrelease feature was removed by commit e966eaeeb623f099
+("locking/lockdep: Remove the cross-release locking checks"). At this
+point, this check should have been restored.
+
+Then, commit d6e89786bed977f3 ("workqueue: skip lockdep wq dependency in
+cancel_work_sync()") introduced a boolean flag in order to distinguish
+flush_work() and cancel_work_sync(), for checking "struct workqueue_struct"
+dependency when called from cancel_work_sync() was causing false positives.
+
+Then, commit 87915adc3f0acdf0 ("workqueue: re-add lockdep dependencies for
+flushing") tried to restore "struct work_struct" dependency check, but by
+error checked this boolean flag. Like an example shown above indicates,
+"struct work_struct" dependency needs to be checked for both flush_work()
+and cancel_work_sync().
+
+Link: https://lkml.kernel.org/r/20220504044800.4966-1-hdanton@sina.com [1]
+Reported-by: Hillf Danton <hdanton@sina.com>
+Suggested-by: Lai Jiangshan <jiangshanlai@gmail.com>
+Fixes: 87915adc3f0acdf0 ("workqueue: re-add lockdep dependencies for flushing")
+Cc: Johannes Berg <johannes.berg@intel.com>
+Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
+Signed-off-by: Tejun Heo <tj@kernel.org>
+Signed-off-by: Sasha Levin <sashal@kernel.org>
+---
+ kernel/workqueue.c | 6 ++----
+ 1 file changed, 2 insertions(+), 4 deletions(-)
+
+diff --git a/kernel/workqueue.c b/kernel/workqueue.c
+index e90f37e22202..dd96391b44de 100644
+--- a/kernel/workqueue.c
++++ b/kernel/workqueue.c
+@@ -3049,10 +3049,8 @@ static bool __flush_work(struct work_struct *work, bool from_cancel)
+       if (WARN_ON(!work->func))
+               return false;
+-      if (!from_cancel) {
+-              lock_map_acquire(&work->lockdep_map);
+-              lock_map_release(&work->lockdep_map);
+-      }
++      lock_map_acquire(&work->lockdep_map);
++      lock_map_release(&work->lockdep_map);
+       if (start_flush_work(work, &barr, from_cancel)) {
+               wait_for_completion(&barr.done);
+-- 
+2.35.1
+