From aa06117f19944573cda0c4bee026c916b5256090 Mon Sep 17 00:00:00 2001 From: Roland Dreier Date: Wed, 1 Jul 2009 15:48:18 -0700 Subject: 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: [] ecryptfs_find_global_auth_tok_for_sig+0x2e/0x90 but task is already holding lock: (&crypt_stat->keysig_list_mutex){+.+.+.}, at: [] 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){+.+.+.}: [] check_prev_add+0x2a7/0x370 [] validate_chain+0x661/0x750 [] __lock_acquire+0x237/0x430 [] lock_acquire+0xa5/0x150 [] __mutex_lock_common+0x4d/0x3d0 [] mutex_lock_nested+0x46/0x60 [] ecryptfs_add_keysig+0x5a/0xb0 [] ecryptfs_copy_mount_wide_sigs_to_inode_sigs+0x59/0xb0 [] ecryptfs_new_file_context+0xa6/0x1a0 [] ecryptfs_initialize_file+0x4a/0x140 [] ecryptfs_create+0x2d/0x60 [] vfs_create+0xb4/0xe0 [] __open_namei_create+0xc4/0x110 [] do_filp_open+0xa01/0xae0 [] do_sys_open+0x69/0x140 [] sys_open+0x20/0x30 [] system_call_fastpath+0x16/0x1b [] 0xffffffffffffffff -> #0 (&mount_crypt_stat->global_auth_tok_list_mutex){+.+.+.}: [] check_prev_add+0x85/0x370 [] validate_chain+0x661/0x750 [] __lock_acquire+0x237/0x430 [] lock_acquire+0xa5/0x150 [] __mutex_lock_common+0x4d/0x3d0 [] mutex_lock_nested+0x46/0x60 [] ecryptfs_find_global_auth_tok_for_sig+0x2e/0x90 [] ecryptfs_generate_key_packet_set+0x105/0x2b0 [] ecryptfs_write_headers_virt+0xc9/0x120 [] ecryptfs_write_metadata+0xcd/0x200 [] ecryptfs_initialize_file+0x6b/0x140 [] ecryptfs_create+0x2d/0x60 [] vfs_create+0xb4/0xe0 [] __open_namei_create+0xc4/0x110 [] do_filp_open+0xa01/0xae0 [] do_sys_open+0x69/0x140 [] sys_open+0x20/0x30 [] system_call_fastpath+0x16/0x1b [] 0xffffffffffffffff other info that might help us debug this: 2 locks held by gdm/2640: #0: (&sb->s_type->i_mutex_key#11){+.+.+.}, at: [] do_filp_open+0x3cb/0xae0 #1: (&crypt_stat->keysig_list_mutex){+.+.+.}, at: [] 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: [] print_circular_bug_tail+0xa8/0xf0 [] check_prev_add+0x85/0x370 [] ? __module_text_address+0x12/0x60 [] validate_chain+0x661/0x750 [] ? print_context_stack+0x85/0x140 [] ? find_usage_backwards+0x38/0x160 [] __lock_acquire+0x237/0x430 [] lock_acquire+0xa5/0x150 [] ? ecryptfs_find_global_auth_tok_for_sig+0x2e/0x90 [] ? check_usage_backwards+0x0/0xb0 [] __mutex_lock_common+0x4d/0x3d0 [] ? ecryptfs_find_global_auth_tok_for_sig+0x2e/0x90 [] ? ecryptfs_find_global_auth_tok_for_sig+0x2e/0x90 [] ? mark_held_locks+0x6c/0xa0 [] ? kmem_cache_alloc+0xfd/0x1a0 [] ? trace_hardirqs_on_caller+0x14d/0x190 [] mutex_lock_nested+0x46/0x60 [] ecryptfs_find_global_auth_tok_for_sig+0x2e/0x90 [] ecryptfs_generate_key_packet_set+0x105/0x2b0 [] ecryptfs_write_headers_virt+0xc9/0x120 [] ecryptfs_write_metadata+0xcd/0x200 [] ? ecryptfs_init_persistent_file+0x60/0xe0 [] ecryptfs_initialize_file+0x6b/0x140 [] ecryptfs_create+0x2d/0x60 [] vfs_create+0xb4/0xe0 [] __open_namei_create+0xc4/0x110 [] do_filp_open+0xa01/0xae0 [] ? _raw_spin_unlock+0x5e/0xb0 [] ? _spin_unlock+0x2b/0x40 [] ? getname+0x3b/0x240 [] ? alloc_fd+0xfa/0x140 [] do_sys_open+0x69/0x140 [] ? trace_hardirqs_on_thunk+0x3a/0x3f [] sys_open+0x20/0x30 [] system_call_fastpath+0x16/0x1b Signed-off-by: Roland Dreier Signed-off-by: Tyler Hicks --- fs/ecryptfs/keystore.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) (limited to 'fs/ecryptfs/keystore.c') diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c index 259525c9abb..f9965139c43 100644 --- a/fs/ecryptfs/keystore.c +++ b/fs/ecryptfs/keystore.c @@ -2366,21 +2366,18 @@ struct kmem_cache *ecryptfs_key_sig_cache; int ecryptfs_add_keysig(struct ecryptfs_crypt_stat *crypt_stat, char *sig) { struct ecryptfs_key_sig *new_key_sig; - int rc = 0; new_key_sig = kmem_cache_alloc(ecryptfs_key_sig_cache, GFP_KERNEL); if (!new_key_sig) { - rc = -ENOMEM; printk(KERN_ERR "Error allocating from ecryptfs_key_sig_cache\n"); - goto out; + return -ENOMEM; } memcpy(new_key_sig->keysig, sig, ECRYPTFS_SIG_SIZE_HEX); - mutex_lock(&crypt_stat->keysig_list_mutex); + /* Caller must hold keysig_list_mutex */ list_add(&new_key_sig->crypt_stat_list, &crypt_stat->keysig_list); - mutex_unlock(&crypt_stat->keysig_list_mutex); -out: - return rc; + + return 0; } struct kmem_cache *ecryptfs_global_auth_tok_cache; -- cgit v1.2.3-70-g09d2 From b0105eaefa7cce8f4a941d0fc6354b250d30e745 Mon Sep 17 00:00:00 2001 From: Tyler Hicks Date: Tue, 11 Aug 2009 00:36:32 -0500 Subject: eCryptfs: Handle unrecognized tag 3 cipher codes Returns an error when an unrecognized cipher code is present in a tag 3 packet or an ecryptfs_crypt_stat cannot be initialized. Also sets an crypt_stat->tfm error pointer to NULL to ensure that it will not be incorrectly freed in ecryptfs_destroy_crypt_stat(). Acked-by: Serge Hallyn Cc: ecryptfs-devel@lists.launchpad.net Cc: stable Signed-off-by: Tyler Hicks --- fs/ecryptfs/crypto.c | 1 + fs/ecryptfs/keystore.c | 10 +++++++--- 2 files changed, 8 insertions(+), 3 deletions(-) (limited to 'fs/ecryptfs/keystore.c') diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c index 931f715827a..f92370aa570 100644 --- a/fs/ecryptfs/crypto.c +++ b/fs/ecryptfs/crypto.c @@ -795,6 +795,7 @@ int ecryptfs_init_crypt_ctx(struct ecryptfs_crypt_stat *crypt_stat) kfree(full_alg_name); if (IS_ERR(crypt_stat->tfm)) { rc = PTR_ERR(crypt_stat->tfm); + crypt_stat->tfm = NULL; ecryptfs_printk(KERN_ERR, "cryptfs: init_crypt_ctx(): " "Error initializing cipher [%s]\n", crypt_stat->cipher); diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c index f9965139c43..3a61f056acf 100644 --- a/fs/ecryptfs/keystore.c +++ b/fs/ecryptfs/keystore.c @@ -1316,8 +1316,10 @@ parse_tag_3_packet(struct ecryptfs_crypt_stat *crypt_stat, rc = -EINVAL; goto out_free; } - ecryptfs_cipher_code_to_string(crypt_stat->cipher, - (u16)data[(*packet_size)]); + rc = ecryptfs_cipher_code_to_string(crypt_stat->cipher, + (u16)data[(*packet_size)]); + if (rc) + goto out_free; /* A little extra work to differentiate among the AES key * sizes; see RFC2440 */ switch(data[(*packet_size)++]) { @@ -1328,7 +1330,9 @@ parse_tag_3_packet(struct ecryptfs_crypt_stat *crypt_stat, crypt_stat->key_size = (*new_auth_tok)->session_key.encrypted_key_size; } - ecryptfs_init_crypt_ctx(crypt_stat); + rc = ecryptfs_init_crypt_ctx(crypt_stat); + if (rc) + goto out_free; if (unlikely(data[(*packet_size)++] != 0x03)) { printk(KERN_WARNING "Only S2K ID 3 is currently supported\n"); rc = -ENOSYS; -- cgit v1.2.3-70-g09d2 From df6ad33ba1b9846bd5f0e2b9016c30c20bc2d948 Mon Sep 17 00:00:00 2001 From: Tyler Hicks Date: Fri, 21 Aug 2009 04:27:46 -0500 Subject: eCryptfs: Filename encryption only supports password auth tokens Returns -ENOTSUPP when attempting to use filename encryption with something other than a password authentication token, such as a private token from openssl. Using filename encryption with a userspace eCryptfs key module is a future goal. Until then, this patch handles the situation a little better than simply using a BUG_ON(). Acked-by: Serge Hallyn Cc: ecryptfs-devel@lists.launchpad.net Cc: stable Signed-off-by: Tyler Hicks --- fs/ecryptfs/crypto.c | 4 ++-- fs/ecryptfs/keystore.c | 14 ++++++++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) (limited to 'fs/ecryptfs/keystore.c') diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c index f92370aa570..bae20ad1a50 100644 --- a/fs/ecryptfs/crypto.c +++ b/fs/ecryptfs/crypto.c @@ -1703,7 +1703,7 @@ ecryptfs_encrypt_filename(struct ecryptfs_filename *filename, } else { printk(KERN_ERR "%s: No support for requested filename " "encryption method in this release\n", __func__); - rc = -ENOTSUPP; + rc = -EOPNOTSUPP; goto out; } out: @@ -2168,7 +2168,7 @@ int ecryptfs_encrypt_and_encode_filename( (*encoded_name)[(*encoded_name_size)] = '\0'; (*encoded_name_size)++; } else { - rc = -ENOTSUPP; + rc = -EOPNOTSUPP; } if (rc) { printk(KERN_ERR "%s: Error attempting to encode " diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c index 3a61f056acf..17164e483ab 100644 --- a/fs/ecryptfs/keystore.c +++ b/fs/ecryptfs/keystore.c @@ -612,7 +612,12 @@ ecryptfs_write_tag_70_packet(char *dest, size_t *remaining_bytes, } /* TODO: Support other key modules than passphrase for * filename encryption */ - BUG_ON(s->auth_tok->token_type != ECRYPTFS_PASSWORD); + if (s->auth_tok->token_type != ECRYPTFS_PASSWORD) { + rc = -EOPNOTSUPP; + printk(KERN_INFO "%s: Filename encryption only supports " + "password tokens\n", __func__); + goto out_free_unlock; + } sg_init_one( &s->hash_sg, (u8 *)s->auth_tok->token.password.session_key_encryption_key, @@ -910,7 +915,12 @@ ecryptfs_parse_tag_70_packet(char **filename, size_t *filename_size, } /* TODO: Support other key modules than passphrase for * filename encryption */ - BUG_ON(s->auth_tok->token_type != ECRYPTFS_PASSWORD); + if (s->auth_tok->token_type != ECRYPTFS_PASSWORD) { + rc = -EOPNOTSUPP; + printk(KERN_INFO "%s: Filename encryption only supports " + "password tokens\n", __func__); + goto out_free_unlock; + } rc = crypto_blkcipher_setkey( s->desc.tfm, s->auth_tok->token.password.session_key_encryption_key, -- cgit v1.2.3-70-g09d2 From 3891959846709a19f76628e33478cd85edb0e79f Mon Sep 17 00:00:00 2001 From: Tyler Hicks Date: Wed, 26 Aug 2009 01:54:56 -0500 Subject: eCryptfs: Validate global auth tok keys When searching through the global authentication tokens for a given key signature, verify that a matching key has not been revoked and has not expired. This allows the `keyctl revoke` command to be properly used on keys in use by eCryptfs. Acked-by: Serge Hallyn Cc: ecryptfs-devel@lists.launchpad.net Cc: stable Signed-off-by: Tyler Hicks --- fs/ecryptfs/keystore.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'fs/ecryptfs/keystore.c') diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c index 17164e483ab..a0a7847567e 100644 --- a/fs/ecryptfs/keystore.c +++ b/fs/ecryptfs/keystore.c @@ -416,7 +416,9 @@ ecryptfs_find_global_auth_tok_for_sig( &mount_crypt_stat->global_auth_tok_list, mount_crypt_stat_list) { if (memcmp(walker->sig, sig, ECRYPTFS_SIG_SIZE_HEX) == 0) { - (*global_auth_tok) = walker; + rc = key_validate(walker->global_auth_tok_key); + if (!rc) + (*global_auth_tok) = walker; goto out; } } -- cgit v1.2.3-70-g09d2