aboutsummaryrefslogtreecommitdiffstats
path: root/fs/ecryptfs
diff options
context:
space:
mode:
authorRoland Dreier <rdreier@cisco.com>2009-07-01 18:48:18 -0400
committerTyler Hicks <tyhicks@linux.vnet.ibm.com>2009-09-23 10:10:30 -0400
commitaa06117f19944573cda0c4bee026c916b5256090 (patch)
treee5cb24f500431a5201e7a50b1e9df62fbd24e1bb /fs/ecryptfs
parent05dafedb906425fe935199f4c92700d87285e3e9 (diff)
eCryptfs: Fix lockdep-reported AB-BA mutex issue
Lockdep reports the following valid-looking possible AB-BA deadlock with global_auth_tok_list_mutex and keysig_list_mutex: ecryptfs_new_file_context() -> ecryptfs_copy_mount_wide_sigs_to_inode_sigs() -> mutex_lock(&mount_crypt_stat->global_auth_tok_list_mutex); -> ecryptfs_add_keysig() -> mutex_lock(&crypt_stat->keysig_list_mutex); vs ecryptfs_generate_key_packet_set() -> mutex_lock(&crypt_stat->keysig_list_mutex); -> ecryptfs_find_global_auth_tok_for_sig() -> mutex_lock(&mount_crypt_stat->global_auth_tok_list_mutex); ie the two mutexes are taken in opposite orders in the two different code paths. I'm not sure if this is a real bug where two threads could actually hit the two paths in parallel and deadlock, but it at least makes lockdep impossible to use with ecryptfs since this report triggers every time and disables future lockdep reporting. Since ecryptfs_add_keysig() is called only from the single callsite in ecryptfs_copy_mount_wide_sigs_to_inode_sigs(), the simplest fix seems to be to move the lock of keysig_list_mutex back up outside of the where global_auth_tok_list_mutex is taken. This patch does that, and fixes the lockdep report on my system (and ecryptfs still works OK). The full output of lockdep fixed by this patch is: ======================================================= [ INFO: possible circular locking dependency detected ] 2.6.31-2-generic #14~rbd2 ------------------------------------------------------- gdm/2640 is trying to acquire lock: (&mount_crypt_stat->global_auth_tok_list_mutex){+.+.+.}, at: [<ffffffff8121591e>] ecryptfs_find_global_auth_tok_for_sig+0x2e/0x90 but task is already holding lock: (&crypt_stat->keysig_list_mutex){+.+.+.}, at: [<ffffffff81217728>] ecryptfs_generate_key_packet_set+0x58/0x2b0 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (&crypt_stat->keysig_list_mutex){+.+.+.}: [<ffffffff8108c897>] check_prev_add+0x2a7/0x370 [<ffffffff8108cfc1>] validate_chain+0x661/0x750 [<ffffffff8108d2e7>] __lock_acquire+0x237/0x430 [<ffffffff8108d585>] lock_acquire+0xa5/0x150 [<ffffffff815526cd>] __mutex_lock_common+0x4d/0x3d0 [<ffffffff81552b56>] mutex_lock_nested+0x46/0x60 [<ffffffff8121526a>] ecryptfs_add_keysig+0x5a/0xb0 [<ffffffff81213299>] ecryptfs_copy_mount_wide_sigs_to_inode_sigs+0x59/0xb0 [<ffffffff81214b06>] ecryptfs_new_file_context+0xa6/0x1a0 [<ffffffff8120e42a>] ecryptfs_initialize_file+0x4a/0x140 [<ffffffff8120e54d>] ecryptfs_create+0x2d/0x60 [<ffffffff8113a7d4>] vfs_create+0xb4/0xe0 [<ffffffff8113a8c4>] __open_namei_create+0xc4/0x110 [<ffffffff8113d1c1>] do_filp_open+0xa01/0xae0 [<ffffffff8112d8d9>] do_sys_open+0x69/0x140 [<ffffffff8112d9f0>] sys_open+0x20/0x30 [<ffffffff81013132>] system_call_fastpath+0x16/0x1b [<ffffffffffffffff>] 0xffffffffffffffff -> #0 (&mount_crypt_stat->global_auth_tok_list_mutex){+.+.+.}: [<ffffffff8108c675>] check_prev_add+0x85/0x370 [<ffffffff8108cfc1>] validate_chain+0x661/0x750 [<ffffffff8108d2e7>] __lock_acquire+0x237/0x430 [<ffffffff8108d585>] lock_acquire+0xa5/0x150 [<ffffffff815526cd>] __mutex_lock_common+0x4d/0x3d0 [<ffffffff81552b56>] mutex_lock_nested+0x46/0x60 [<ffffffff8121591e>] ecryptfs_find_global_auth_tok_for_sig+0x2e/0x90 [<ffffffff812177d5>] ecryptfs_generate_key_packet_set+0x105/0x2b0 [<ffffffff81212f49>] ecryptfs_write_headers_virt+0xc9/0x120 [<ffffffff8121306d>] ecryptfs_write_metadata+0xcd/0x200 [<ffffffff8120e44b>] ecryptfs_initialize_file+0x6b/0x140 [<ffffffff8120e54d>] ecryptfs_create+0x2d/0x60 [<ffffffff8113a7d4>] vfs_create+0xb4/0xe0 [<ffffffff8113a8c4>] __open_namei_create+0xc4/0x110 [<ffffffff8113d1c1>] do_filp_open+0xa01/0xae0 [<ffffffff8112d8d9>] do_sys_open+0x69/0x140 [<ffffffff8112d9f0>] sys_open+0x20/0x30 [<ffffffff81013132>] system_call_fastpath+0x16/0x1b [<ffffffffffffffff>] 0xffffffffffffffff other info that might help us debug this: 2 locks held by gdm/2640: #0: (&sb->s_type->i_mutex_key#11){+.+.+.}, at: [<ffffffff8113cb8b>] do_filp_open+0x3cb/0xae0 #1: (&crypt_stat->keysig_list_mutex){+.+.+.}, at: [<ffffffff81217728>] ecryptfs_generate_key_packet_set+0x58/0x2b0 stack backtrace: Pid: 2640, comm: gdm Tainted: G C 2.6.31-2-generic #14~rbd2 Call Trace: [<ffffffff8108b988>] print_circular_bug_tail+0xa8/0xf0 [<ffffffff8108c675>] check_prev_add+0x85/0x370 [<ffffffff81094912>] ? __module_text_address+0x12/0x60 [<ffffffff8108cfc1>] validate_chain+0x661/0x750 [<ffffffff81017275>] ? print_context_stack+0x85/0x140 [<ffffffff81089c68>] ? find_usage_backwards+0x38/0x160 [<ffffffff8108d2e7>] __lock_acquire+0x237/0x430 [<ffffffff8108d585>] lock_acquire+0xa5/0x150 [<ffffffff8121591e>] ? ecryptfs_find_global_auth_tok_for_sig+0x2e/0x90 [<ffffffff8108b0b0>] ? check_usage_backwards+0x0/0xb0 [<ffffffff815526cd>] __mutex_lock_common+0x4d/0x3d0 [<ffffffff8121591e>] ? ecryptfs_find_global_auth_tok_for_sig+0x2e/0x90 [<ffffffff8121591e>] ? ecryptfs_find_global_auth_tok_for_sig+0x2e/0x90 [<ffffffff8108c02c>] ? mark_held_locks+0x6c/0xa0 [<ffffffff81125b0d>] ? kmem_cache_alloc+0xfd/0x1a0 [<ffffffff8108c34d>] ? trace_hardirqs_on_caller+0x14d/0x190 [<ffffffff81552b56>] mutex_lock_nested+0x46/0x60 [<ffffffff8121591e>] ecryptfs_find_global_auth_tok_for_sig+0x2e/0x90 [<ffffffff812177d5>] ecryptfs_generate_key_packet_set+0x105/0x2b0 [<ffffffff81212f49>] ecryptfs_write_headers_virt+0xc9/0x120 [<ffffffff8121306d>] ecryptfs_write_metadata+0xcd/0x200 [<ffffffff81210240>] ? ecryptfs_init_persistent_file+0x60/0xe0 [<ffffffff8120e44b>] ecryptfs_initialize_file+0x6b/0x140 [<ffffffff8120e54d>] ecryptfs_create+0x2d/0x60 [<ffffffff8113a7d4>] vfs_create+0xb4/0xe0 [<ffffffff8113a8c4>] __open_namei_create+0xc4/0x110 [<ffffffff8113d1c1>] do_filp_open+0xa01/0xae0 [<ffffffff8129a93e>] ? _raw_spin_unlock+0x5e/0xb0 [<ffffffff8155410b>] ? _spin_unlock+0x2b/0x40 [<ffffffff81139e9b>] ? getname+0x3b/0x240 [<ffffffff81148a5a>] ? alloc_fd+0xfa/0x140 [<ffffffff8112d8d9>] do_sys_open+0x69/0x140 [<ffffffff81553b8f>] ? trace_hardirqs_on_thunk+0x3a/0x3f [<ffffffff8112d9f0>] sys_open+0x20/0x30 [<ffffffff81013132>] system_call_fastpath+0x16/0x1b Signed-off-by: Roland Dreier <rolandd@cisco.com> Signed-off-by: Tyler Hicks <tyhicks@linux.vnet.ibm.com>
Diffstat (limited to 'fs/ecryptfs')
-rw-r--r--fs/ecryptfs/crypto.c8
-rw-r--r--fs/ecryptfs/keystore.c11
2 files changed, 9 insertions, 10 deletions
diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
index 4610fd6afcef..520783b205a1 100644
--- a/fs/ecryptfs/crypto.c
+++ b/fs/ecryptfs/crypto.c
@@ -923,7 +923,9 @@ static int ecryptfs_copy_mount_wide_sigs_to_inode_sigs(
923 struct ecryptfs_global_auth_tok *global_auth_tok; 923 struct ecryptfs_global_auth_tok *global_auth_tok;
924 int rc = 0; 924 int rc = 0;
925 925
926 mutex_lock(&crypt_stat->keysig_list_mutex);
926 mutex_lock(&mount_crypt_stat->global_auth_tok_list_mutex); 927 mutex_lock(&mount_crypt_stat->global_auth_tok_list_mutex);
928
927 list_for_each_entry(global_auth_tok, 929 list_for_each_entry(global_auth_tok,
928 &mount_crypt_stat->global_auth_tok_list, 930 &mount_crypt_stat->global_auth_tok_list,
929 mount_crypt_stat_list) { 931 mount_crypt_stat_list) {
@@ -932,13 +934,13 @@ static int ecryptfs_copy_mount_wide_sigs_to_inode_sigs(
932 rc = ecryptfs_add_keysig(crypt_stat, global_auth_tok->sig); 934 rc = ecryptfs_add_keysig(crypt_stat, global_auth_tok->sig);
933 if (rc) { 935 if (rc) {
934 printk(KERN_ERR "Error adding keysig; rc = [%d]\n", rc); 936 printk(KERN_ERR "Error adding keysig; rc = [%d]\n", rc);
935 mutex_unlock(
936 &mount_crypt_stat->global_auth_tok_list_mutex);
937 goto out; 937 goto out;
938 } 938 }
939 } 939 }
940 mutex_unlock(&mount_crypt_stat->global_auth_tok_list_mutex); 940
941out: 941out:
942 mutex_unlock(&mount_crypt_stat->global_auth_tok_list_mutex);
943 mutex_unlock(&crypt_stat->keysig_list_mutex);
942 return rc; 944 return rc;
943} 945}
944 946
diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c
index 259525c9abb8..f9965139c430 100644
--- a/fs/ecryptfs/keystore.c
+++ b/fs/ecryptfs/keystore.c
@@ -2366,21 +2366,18 @@ struct kmem_cache *ecryptfs_key_sig_cache;
2366int ecryptfs_add_keysig(struct ecryptfs_crypt_stat *crypt_stat, char *sig) 2366int ecryptfs_add_keysig(struct ecryptfs_crypt_stat *crypt_stat, char *sig)
2367{ 2367{
2368 struct ecryptfs_key_sig *new_key_sig; 2368 struct ecryptfs_key_sig *new_key_sig;
2369 int rc = 0;
2370 2369
2371 new_key_sig = kmem_cache_alloc(ecryptfs_key_sig_cache, GFP_KERNEL); 2370 new_key_sig = kmem_cache_alloc(ecryptfs_key_sig_cache, GFP_KERNEL);
2372 if (!new_key_sig) { 2371 if (!new_key_sig) {
2373 rc = -ENOMEM;
2374 printk(KERN_ERR 2372 printk(KERN_ERR
2375 "Error allocating from ecryptfs_key_sig_cache\n"); 2373 "Error allocating from ecryptfs_key_sig_cache\n");
2376 goto out; 2374 return -ENOMEM;
2377 } 2375 }
2378 memcpy(new_key_sig->keysig, sig, ECRYPTFS_SIG_SIZE_HEX); 2376 memcpy(new_key_sig->keysig, sig, ECRYPTFS_SIG_SIZE_HEX);
2379 mutex_lock(&crypt_stat->keysig_list_mutex); 2377 /* Caller must hold keysig_list_mutex */
2380 list_add(&new_key_sig->crypt_stat_list, &crypt_stat->keysig_list); 2378 list_add(&new_key_sig->crypt_stat_list, &crypt_stat->keysig_list);
2381 mutex_unlock(&crypt_stat->keysig_list_mutex); 2379
2382out: 2380 return 0;
2383 return rc;
2384} 2381}
2385 2382
2386struct kmem_cache *ecryptfs_global_auth_tok_cache; 2383struct kmem_cache *ecryptfs_global_auth_tok_cache;