• Roland Dreier's avatar
    eCryptfs: Fix lockdep-reported AB-BA mutex issue · aa06117f
    Roland Dreier authored
    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: default avatarRoland Dreier <rolandd@cisco.com>
    Signed-off-by: default avatarTyler Hicks <tyhicks@linux.vnet.ibm.com>
    aa06117f
crypto.c 66.5 KB