diff options
author | John Johansen <john.johansen@canonical.com> | 2017-11-20 23:24:09 -0800 |
---|---|---|
committer | John Johansen <john.johansen@canonical.com> | 2017-11-21 02:17:16 -0800 |
commit | feb3c766a3ab32d233aaff7db13afd9ba5bc142d (patch) | |
tree | 59b48447feb2bf23a53df1b2ead7d02fd440c957 /security/apparmor/policy_ns.c | |
parent | 5d7c44ef5e4f0149c9fb99faeae41e930485a1ec (diff) |
apparmor: fix possible recursive lock warning in __aa_create_ns
Use mutex_lock_nested to provide lockdep the parent child lock ordering of
the tree.
This fixes the lockdep Warning
[ 305.275177] ============================================
[ 305.275178] WARNING: possible recursive locking detected
[ 305.275179] 4.14.0-rc7+ #320 Not tainted
[ 305.275180] --------------------------------------------
[ 305.275181] apparmor_parser/1339 is trying to acquire lock:
[ 305.275182] (&ns->lock){+.+.}, at: [<ffffffff970544dd>] __aa_create_ns+0x6d/0x1e0
[ 305.275187]
but task is already holding lock:
[ 305.275187] (&ns->lock){+.+.}, at: [<ffffffff97054b5d>] aa_prepare_ns+0x3d/0xd0
[ 305.275190]
other info that might help us debug this:
[ 305.275191] Possible unsafe locking scenario:
[ 305.275192] CPU0
[ 305.275193] ----
[ 305.275193] lock(&ns->lock);
[ 305.275194] lock(&ns->lock);
[ 305.275195]
*** DEADLOCK ***
[ 305.275196] May be due to missing lock nesting notation
[ 305.275198] 2 locks held by apparmor_parser/1339:
[ 305.275198] #0: (sb_writers#10){.+.+}, at: [<ffffffff96e9c6b7>] vfs_write+0x1a7/0x1d0
[ 305.275202] #1: (&ns->lock){+.+.}, at: [<ffffffff97054b5d>] aa_prepare_ns+0x3d/0xd0
[ 305.275205]
stack backtrace:
[ 305.275207] CPU: 1 PID: 1339 Comm: apparmor_parser Not tainted 4.14.0-rc7+ #320
[ 305.275208] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.1-1ubuntu1 04/01/2014
[ 305.275209] Call Trace:
[ 305.275212] dump_stack+0x85/0xcb
[ 305.275214] __lock_acquire+0x141c/0x1460
[ 305.275216] ? __aa_create_ns+0x6d/0x1e0
[ 305.275218] ? ___slab_alloc+0x183/0x540
[ 305.275219] ? ___slab_alloc+0x183/0x540
[ 305.275221] lock_acquire+0xed/0x1e0
[ 305.275223] ? lock_acquire+0xed/0x1e0
[ 305.275224] ? __aa_create_ns+0x6d/0x1e0
[ 305.275227] __mutex_lock+0x89/0x920
[ 305.275228] ? __aa_create_ns+0x6d/0x1e0
[ 305.275230] ? trace_hardirqs_on_caller+0x11f/0x190
[ 305.275231] ? __aa_create_ns+0x6d/0x1e0
[ 305.275233] ? __lockdep_init_map+0x57/0x1d0
[ 305.275234] ? lockdep_init_map+0x9/0x10
[ 305.275236] ? __rwlock_init+0x32/0x60
[ 305.275238] mutex_lock_nested+0x1b/0x20
[ 305.275240] ? mutex_lock_nested+0x1b/0x20
[ 305.275241] __aa_create_ns+0x6d/0x1e0
[ 305.275243] aa_prepare_ns+0xc2/0xd0
[ 305.275245] aa_replace_profiles+0x168/0xf30
[ 305.275247] ? __might_fault+0x85/0x90
[ 305.275250] policy_update+0xb9/0x380
[ 305.275252] profile_load+0x7e/0x90
[ 305.275254] __vfs_write+0x28/0x150
[ 305.275256] ? rcu_read_lock_sched_held+0x72/0x80
[ 305.275257] ? rcu_sync_lockdep_assert+0x2f/0x60
[ 305.275259] ? __sb_start_write+0xdc/0x1c0
[ 305.275261] ? vfs_write+0x1a7/0x1d0
[ 305.275262] vfs_write+0xca/0x1d0
[ 305.275264] ? trace_hardirqs_on_caller+0x11f/0x190
[ 305.275266] SyS_write+0x49/0xa0
[ 305.275268] entry_SYSCALL_64_fastpath+0x23/0xc2
[ 305.275271] RIP: 0033:0x7fa6b22e8c74
[ 305.275272] RSP: 002b:00007ffeaaee6288 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
[ 305.275273] RAX: ffffffffffffffda RBX: 00007ffeaaee62a4 RCX: 00007fa6b22e8c74
[ 305.275274] RDX: 0000000000000a51 RSI: 00005566a8198c10 RDI: 0000000000000004
[ 305.275275] RBP: 0000000000000a39 R08: 0000000000000a51 R09: 0000000000000000
[ 305.275276] R10: 0000000000000000 R11: 0000000000000246 R12: 00005566a8198c10
[ 305.275277] R13: 0000000000000004 R14: 00005566a72ecb88 R15: 00005566a72ec3a8
Fixes: 73688d1ed0b8 ("apparmor: refactor prepare_ns() and make usable from different views")
Signed-off-by: John Johansen <john.johansen@canonical.com>
Diffstat (limited to 'security/apparmor/policy_ns.c')
-rw-r--r-- | security/apparmor/policy_ns.c | 8 |
1 files changed, 4 insertions, 4 deletions
diff --git a/security/apparmor/policy_ns.c b/security/apparmor/policy_ns.c index 62a3589c62ab..b1e629cba70b 100644 --- a/security/apparmor/policy_ns.c +++ b/security/apparmor/policy_ns.c @@ -256,7 +256,8 @@ static struct aa_ns *__aa_create_ns(struct aa_ns *parent, const char *name, ns = alloc_ns(parent->base.hname, name); if (!ns) return NULL; - mutex_lock(&ns->lock); + ns->level = parent->level + 1; + mutex_lock_nested(&ns->lock, ns->level); error = __aafs_ns_mkdir(ns, ns_subns_dir(parent), name, dir); if (error) { AA_ERROR("Failed to create interface for ns %s\n", @@ -266,7 +267,6 @@ static struct aa_ns *__aa_create_ns(struct aa_ns *parent, const char *name, return ERR_PTR(error); } ns->parent = aa_get_ns(parent); - ns->level = parent->level + 1; list_add_rcu(&ns->base.list, &parent->sub_ns); /* add list ref */ aa_get_ns(ns); @@ -313,7 +313,7 @@ struct aa_ns *aa_prepare_ns(struct aa_ns *parent, const char *name) { struct aa_ns *ns; - mutex_lock(&parent->lock); + mutex_lock_nested(&parent->lock, parent->level); /* try and find the specified ns and if it doesn't exist create it */ /* released by caller */ ns = aa_get_ns(__aa_find_ns(&parent->sub_ns, name)); @@ -336,7 +336,7 @@ static void destroy_ns(struct aa_ns *ns) if (!ns) return; - mutex_lock(&ns->lock); + mutex_lock_nested(&ns->lock, ns->level); /* release all profiles in this namespace */ __aa_profile_list_release(&ns->base.profiles); |