linux-kernel - [PATCH] locking: Fix runtime warning in ww mutex selftest

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <1538516929-9734-1-git-send-email-linux@roeck-us.net>
Date:   Tue,  2 Oct 2018 14:48:49 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     linux-kernel@...r.kernel.org, Guenter Roeck <linux@...ck-us.net>,
        Will Deacon <will.deacon@....com>,
        Chris Wilson <chris@...is-wilson.co.uk>,
        Ingo Molnar <mingo@...nel.org>
Subject: [PATCH] locking: Fix runtime warning in ww mutex selftest

If CONFIG_WW_MUTEX_SELFTEST is enabled, booting an image
in an arm64 virtual machine results in the following
traceback if 8 CPUs are enabled.

DEBUG_LOCKS_WARN_ON(__owner_task(owner) != current)
WARNING: CPU: 2 PID: 537 at kernel/locking/mutex.c:1033 __mutex_unlock_slowpath+0x1a8/0x2e0
Modules linked in:
CPU: 2 PID: 537 Comm: kworker/u16:11 Not tainted 4.18.12-rc1-00133-g74ba23ae47e5 #1
Hardware name: linux,dummy-virt (DT)
Workqueue: test-ww_mutex test_cycle_work
pstate: 40000005 (nZcv daif -PAN -UAO)
pc : __mutex_unlock_slowpath+0x1a8/0x2e0
lr : __mutex_unlock_slowpath+0x1a8/0x2e0
sp : ffff00000acbbc50
x29: ffff00000acbbc50 x28: 0000000000000000
x27: 0000000000000000 x26: ffff000009fc1d80
x25: ffff000009f86558 x24: ffff000008f8d410
x23: ffff000009f86000 x22: ffff00000926f000
x21: ffff00000acbbcb8 x20: ffff80001be208c8
x19: 0000000000000000 x18: ffffffffffffffff
x17: 0000000000000000 x16: ffff80001c2a0000
x15: ffff00000926f808 x14: ffff000089f85d3f
x13: ffff000009f85d4d x12: ffff00000928a000
x11: 0000000005f5e0ff x10: ffff00000acbb8a0
x9 : 0000000000000000 x8 : ffff00000926f808
x7 : ffff000008149180 x6 : 0000000000000000
x5 : 0000000000000000 x4 : 0000000000000000
x3 : ffffffffffffffff x2 : ffff00000928ab40
x1 : 6172e063a21fe300 x0 : 0000000000000000
Call trace:
 __mutex_unlock_slowpath+0x1a8/0x2e0
 ww_mutex_unlock+0x48/0xa0
 test_cycle_work+0xf8/0x200
 process_one_work+0x258/0x410
 worker_thread+0x40/0x458
 kthread+0x128/0x130
 ret_from_fork+0x10/0x18
irq event stamp: 1305
_raw_spin_unlock_irq+0x2c/0x60
__schedule+0xbc/0x780
__do_softirq+0x1e8/0x260
irq_exit+0x144/0x150

If requesting b_mutex fails with -EDEADLK, the error variable
is reassigned to the return value from calling ww_mutex_lock
on a_mutex again. If this call fails, a_mutex is not locked.
It is, however, unconditionally unlocked subsequently, causing
the reported warning. Fix the problem by using two error variables.

With this change, the selftest still fails as follows.

cyclic deadlock not resolved, ret[7/8] = -35

However, the traceback is gone.

Fixes: d1b42b800e5d0 ("locking/ww_mutex: Add kselftests for resolving
ww_mutex cyclic deadlocks")
Cc: Will Deacon <will.deacon@....com>
Cc: Chris Wilson <chris@...is-wilson.co.uk>
Cc: Peter Zijlstra <peterz@...radead.org>
Cc: Ingo Molnar <mingo@...nel.org>
Signed-off-by: Guenter Roeck <linux@...ck-us.net>
---
 kernel/locking/test-ww_mutex.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/kernel/locking/test-ww_mutex.c b/kernel/locking/test-ww_mutex.c
index 0be047dbd897..65a3b7e55b9f 100644
--- a/kernel/locking/test-ww_mutex.c
+++ b/kernel/locking/test-ww_mutex.c
@@ -260,7 +260,7 @@ static void test_cycle_work(struct work_struct *work)
 {
 	struct test_cycle *cycle = container_of(work, typeof(*cycle), work);
 	struct ww_acquire_ctx ctx;
-	int err;
+	int err, erra = 0;
 
 	ww_acquire_init(&ctx, &ww_class);
 	ww_mutex_lock(&cycle->a_mutex, &ctx);
@@ -270,17 +270,19 @@ static void test_cycle_work(struct work_struct *work)
 
 	err = ww_mutex_lock(cycle->b_mutex, &ctx);
 	if (err == -EDEADLK) {
+		err = 0;
 		ww_mutex_unlock(&cycle->a_mutex);
 		ww_mutex_lock_slow(cycle->b_mutex, &ctx);
-		err = ww_mutex_lock(&cycle->a_mutex, &ctx);
+		erra = ww_mutex_lock(&cycle->a_mutex, &ctx);
 	}
 
 	if (!err)
 		ww_mutex_unlock(cycle->b_mutex);
-	ww_mutex_unlock(&cycle->a_mutex);
+	if (!erra)
+		ww_mutex_unlock(&cycle->a_mutex);
 	ww_acquire_fini(&ctx);
 
-	cycle->result = err;
+	cycle->result = err ?: erra;
 }
 
 static int __test_cycle(unsigned int nthreads)
-- 
2.7.4