From 27365bb52a0b752a9fa733cac7104aef071579a7 Mon Sep 17 00:00:00 2001 From: Sasha Levin Date: Mon, 26 Sep 2022 00:58:03 -0400 Subject: [PATCH] Fixes for 5.4 Signed-off-by: Sasha Levin --- queue-5.4/series | 1 + ...skip-lockdep-work-dependency-in-canc.patch | 98 +++++++++++++++++++ 2 files changed, 99 insertions(+) create mode 100644 queue-5.4/workqueue-don-t-skip-lockdep-work-dependency-in-canc.patch diff --git a/queue-5.4/series b/queue-5.4/series index 038387473f7..ebab4e10c01 100644 --- a/queue-5.4/series +++ b/queue-5.4/series @@ -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 index 00000000000..3626884c6c6 --- /dev/null +++ b/queue-5.4/workqueue-don-t-skip-lockdep-work-dependency-in-canc.patch @@ -0,0 +1,98 @@ +From e1e5db5c04e995e4857d8aeb3f6660e6859e0956 Mon Sep 17 00:00:00 2001 +From: Sasha Levin +Date: Fri, 29 Jul 2022 13:30:23 +0900 +Subject: workqueue: don't skip lockdep work dependency in cancel_work_sync() + +From: Tetsuo Handa + +[ 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 + #include + 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 +Suggested-by: Lai Jiangshan +Fixes: 87915adc3f0acdf0 ("workqueue: re-add lockdep dependencies for flushing") +Cc: Johannes Berg +Signed-off-by: Tetsuo Handa +Signed-off-by: Tejun Heo +Signed-off-by: Sasha Levin +--- + 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 + -- 2.47.3