| 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
| ||
|
Date: Mon, 5 Feb 2018 16:49:03 -0800
From: Joel Fernandes <joelaf@...gle.com>
To: linux-kernel@...r.kernel.org
Cc: Joel Fernandes <joelaf@...gle.com>,
Peter Zilstra <peterz@...radead.org>, mhocko@...nel.org,
minchan@...nel.org, linux-mm@...ck.org
Subject: [PATCH RFC] ashmem: Fix lockdep RECLAIM_FS false positive
During invocation of ashmem shrinker under memory pressure, ashmem
calls into VFS code via vfs_fallocate. We however make sure we
don't enter it if the allocation was GFP_FS to prevent looping
into filesystem code. However lockdep doesn't know this and prints
a lockdep splat as below.
This patch fixes the issue by releasing the reclaim_fs lock after
checking for GFP_FS but before calling into the VFS path, and
reacquiring it after so that lockdep can continue reporting any
reclaim issues later.
[ 2115.359650] -(1)[106:kswapd0]=================================
[ 2115.359665] -(1)[106:kswapd0][ INFO: inconsistent lock state ]
[ 2115.359684] -(1)[106:kswapd0]4.9.60+ #2 Tainted: G W O
[ 2115.359699] -(1)[106:kswapd0]---------------------------------
[ 2115.359715] -(1)[106:kswapd0]inconsistent {RECLAIM_FS-ON-W} ->
{IN-RECLAIM_FS-W} usage.
[ 2115.359732] -(1)[106:kswapd0]kswapd0/106 [HC0[0]:SC0[0]:HE1:SE1]
takes:
[ 2115.359748] (&sb->s_type->i_mutex_key#9){++++?+}, at:
[<ffffff9008470644>] shmem_fallocate+0x104/0x880
[ 2115.359809] -(1)[106:kswapd0]{RECLAIM_FS-ON-W} state was registered
at:
[ 2115.359828] -(1)[106:kswapd0] mark_lock+0x2a4/0x10c0
[ 2115.359845] -(1)[106:kswapd0] mark_held_locks+0xc0/0x128
[ 2115.359862] -(1)[106:kswapd0] lockdep_trace_alloc+0x284/0x368
[ 2115.359881] -(1)[106:kswapd0] kmem_cache_alloc+0x3c/0x368
[ 2115.359900] -(1)[106:kswapd0] __d_alloc+0x3c/0x7a8
[ 2115.359917] -(1)[106:kswapd0] d_alloc+0x34/0x140
[ 2115.359934] -(1)[106:kswapd0] d_alloc_parallel+0xfc/0x1480
[ 2115.359953] -(1)[106:kswapd0] lookup_open+0x3c4/0x12a8
[ 2115.359971] -(1)[106:kswapd0] path_openat+0xb40/0x1dc0
[ 2115.359987] -(1)[106:kswapd0] do_filp_open+0x170/0x258
[ 2115.360006] -(1)[106:kswapd0] do_sys_open+0x1b8/0x2f0
[ 2115.360023] -(1)[106:kswapd0] SyS_openat+0x10/0x18
[ 2115.360041] -(1)[106:kswapd0] el0_svc_naked+0x24/0x28
[ 2115.360056] -(1)[106:kswapd0]irq event stamp: 2437365
[ 2115.360079] -(1)[106:kswapd0]hardirqs last enabled at (2437365):
[<ffffff900a0ec6ec>] mutex_trylock+0x224/0x460
[ 2115.360098] -(1)[106:kswapd0]hardirqs last disabled at (2437364):
[<ffffff900a0ec5b0>] mutex_trylock+0xe8/0x460
[ 2115.360116] -(1)[106:kswapd0]softirqs last enabled at (2436534):
[<ffffff90080822a0>] __do_softirq+0xc38/0x1190
[ 2115.360138] -(1)[106:kswapd0]softirqs last disabled at (2436515):
[<ffffff90080de05c>] irq_exit+0x1ac/0x228
[ 2115.360153] -(1)[106:kswapd0]\x0aother info that might help us debug
this:
[ 2115.360169] -(1)[106:kswapd0] Possible unsafe locking scenario:\x0a
[ 2115.360184] -(1)[106:kswapd0] CPU0
[ 2115.360198] -(1)[106:kswapd0] ----
[ 2115.360211] -(1)[106:kswapd0] lock(&sb->s_type->i_mutex_key#9);
[ 2115.360252] -(1)[106:kswapd0] <Interrupt>
[ 2115.360265] -(1)[106:kswapd0] lock(&sb->s_type->i_mutex_key#9);
[ 2115.360304] -(1)[106:kswapd0]\x0a *** DEADLOCK ***\x0a
[ 2115.360322] -(1)[106:kswapd0] #0: (shrinker_rwsem){++++..}, at:
[<ffffff9008459e08>] shrink_slab.part.15.constprop.28+0xb0/0xe10
[ 2115.360382] -(1)[106:kswapd0] #1: (ashmem_mutex){+.+.+.}, at:
[<ffffff90098c4ba0>] ashmem_shrink_scan+0x80/0x308
[ 2115.360439] -(1)[106:kswapd0]\x0astack backtrace:
[ 2115.360462] -(1)[106:kswapd0]CPU: 1 PID: 106 Comm: kswapd0 Tainted: G
W O 4.9.60+ #2
[ 2115.360478] -(1)[106:kswapd0]Hardware name: MT6765 (DT)
[ 2115.360494] -(1)[106:kswapd0]Call trace:
[ 2115.360515] -(1)[106:kswapd0][<ffffff9008092938>]
dump_backtrace+0x0/0x400
[ 2115.360533] -(1)[106:kswapd0][<ffffff900809302c>]
show_stack+0x14/0x20
[ 2115.360555] -(1)[106:kswapd0][<ffffff9008988bc0>]
dump_stack+0xb0/0xe8
[ 2115.360576] -(1)[106:kswapd0][<ffffff900841e8b0>]
print_usage_bug.part.24+0x548/0x568
[ 2115.360595] -(1)[106:kswapd0][<ffffff90082398bc>]
mark_lock+0x494/0x10c0
[ 2115.360613] -(1)[106:kswapd0][<ffffff900823bf1c>]
__lock_acquire+0xc94/0x58e8
[ 2115.360630] -(1)[106:kswapd0][<ffffff9008241ba0>]
lock_acquire+0x1d0/0x708
[ 2115.360650] -(1)[106:kswapd0][<ffffff900a0f27a8>]
down_write+0x48/0xd0
[ 2115.360669] -(1)[106:kswapd0][<ffffff9008470644>]
shmem_fallocate+0x104/0x880
[ 2115.360688] -(1)[106:kswapd0][<ffffff90098c4cc4>]
ashmem_shrink_scan+0x1a4/0x308
[ 2115.360709] -(1)[106:kswapd0][<ffffff900845a088>]
shrink_slab.part.15.constprop.28+0x330/0xe10
[ 2115.360729] -(1)[106:kswapd0][<ffffff9008463b34>]
shrink_node+0x1b4/0x588
[ 2115.360747] -(1)[106:kswapd0][<ffffff90084655f4>] kswapd+0x774/0x1640
[ 2115.360767] -(1)[106:kswapd0][<ffffff9008128a4c>] kthread+0x28c/0x310
[ 2115.360786] -(1)[106:kswapd0][<ffffff9008083f00>]
ret_from_fork+0x10/0x50
Cc: Peter Zilstra <peterz@...radead.org>
Cc: mhocko@...nel.org
Cc: minchan@...nel.org
Cc: linux-mm@...ck.org
Signed-off-by: Joel Fernandes <joelaf@...gle.com>
---
drivers/staging/android/ashmem.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/android/ashmem.c b/drivers/staging/android/ashmem.c
index 372ce9913e6d..7e060f32aaa8 100644
--- a/drivers/staging/android/ashmem.c
+++ b/drivers/staging/android/ashmem.c
@@ -32,6 +32,7 @@
#include <linux/bitops.h>
#include <linux/mutex.h>
#include <linux/shmem_fs.h>
+#include <linux/sched/mm.h>
#include "ashmem.h"
#define ASHMEM_NAME_PREFIX "dev/ashmem/"
@@ -446,8 +447,17 @@ ashmem_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
if (!(sc->gfp_mask & __GFP_FS))
return SHRINK_STOP;
- if (!mutex_trylock(&ashmem_mutex))
+ /*
+ * Release reclaim-fs marking since we've already checked GFP_FS, This
+ * will prevent lockdep's reclaim recursion deadlock false positives.
+ * We'll renable it before returning from this function.
+ */
+ fs_reclaim_release(sc->gfp_mask);
+
+ if (!mutex_trylock(&ashmem_mutex)) {
+ fs_reclaim_acquire(sc->gfp_mask);
return -1;
+ }
list_for_each_entry_safe(range, next, &ashmem_lru_list, lru) {
loff_t start = range->pgstart * PAGE_SIZE;
@@ -464,6 +474,8 @@ ashmem_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
break;
}
mutex_unlock(&ashmem_mutex);
+
+ fs_reclaim_acquire(sc->gfp_mask);
return freed;
}
--
2.16.0.rc1.238.g530d649a79-goog
Powered by blists - more mailing lists