From: Sasha Levin Date: Mon, 26 Sep 2022 04:58:03 +0000 (-0400) Subject: Fixes for 4.19 X-Git-Tag: v4.9.330~35 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=abd919cf53b54dbbbb253a845d1b6763734bccf5;p=thirdparty%2Fkernel%2Fstable-queue.git Fixes for 4.19 Signed-off-by: Sasha Levin --- diff --git a/queue-4.19/series b/queue-4.19/series index 20789282887..f25ed724246 100644 --- a/queue-4.19/series +++ b/queue-4.19/series @@ -53,3 +53,4 @@ s390-dasd-fix-oops-in-dasd_alias_get_start_dev-due-to-missing-pavgroup.patch drivers-hv-never-allocate-anything-besides-framebuff.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-4.19/workqueue-don-t-skip-lockdep-work-dependency-in-canc.patch b/queue-4.19/workqueue-don-t-skip-lockdep-work-dependency-in-canc.patch new file mode 100644 index 00000000000..ef1101de528 --- /dev/null +++ b/queue-4.19/workqueue-don-t-skip-lockdep-work-dependency-in-canc.patch @@ -0,0 +1,98 @@ +From 921efbbdce521651f9aa1982b7191ae6729e65db 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 b1bb6cb5802e..4ea2f7fd20ce 100644 +--- a/kernel/workqueue.c ++++ b/kernel/workqueue.c +@@ -2917,10 +2917,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 +