* [PATCH 1/2] eCryptfs: ecryptfs_keyring_auth_tok_for_sig() bug fix @ 2011-01-28 10:42 Roberto Sassu 2011-01-28 10:42 ` [PATCH 2/2] eCryptfs: lock requested keys for writing Roberto Sassu 0 siblings, 1 reply; 3+ messages in thread From: Roberto Sassu @ 2011-01-28 10:42 UTC (permalink / raw) To: tyhicks Cc: kirkland, dhowells, keyrings, linux-fsdevel, linux-kernel, Roberto Sassu [-- Attachment #1: Type: text/plain, Size: 779 bytes --] The pointer '(*auth_tok_key)' must be set to NULL if the requested key cannot be retrieved in order to prevent the use by calling functions. Signed-off-by: Roberto Sassu <roberto.sassu@polito.it> --- fs/ecryptfs/keystore.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c index c1436cf..4feb78c 100644 --- a/fs/ecryptfs/keystore.c +++ b/fs/ecryptfs/keystore.c @@ -1563,6 +1563,7 @@ int ecryptfs_keyring_auth_tok_for_sig(struct key **auth_tok_key, printk(KERN_ERR "Could not find key with description: [%s]\n", sig); rc = process_request_key_err(PTR_ERR(*auth_tok_key)); + (*auth_tok_key) = NULL; goto out; } (*auth_tok) = ecryptfs_get_key_payload_data(*auth_tok_key); -- 1.7.3.4 [-- Attachment #2: smime.p7s --] [-- Type: application/x-pkcs7-signature, Size: 2061 bytes --] ^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH 2/2] eCryptfs: lock requested keys for writing 2011-01-28 10:42 [PATCH 1/2] eCryptfs: ecryptfs_keyring_auth_tok_for_sig() bug fix Roberto Sassu @ 2011-01-28 10:42 ` Roberto Sassu 2011-02-01 23:16 ` Tyler Hicks 0 siblings, 1 reply; 3+ messages in thread From: Roberto Sassu @ 2011-01-28 10:42 UTC (permalink / raw) To: tyhicks Cc: kirkland, dhowells, keyrings, linux-fsdevel, linux-kernel, Roberto Sassu [-- Attachment #1: Type: text/plain, Size: 2829 bytes --] Keys requested by eCryptfs are exclusively locked in order to prevent modifications by other subjects. In particular those which description is specified at mount time are locked until the filesystem is unmounted, these required to decrypt a single file are unlocked immediately after the open. Signed-off-by: Roberto Sassu <roberto.sassu@polito.it> Reported-by: David Howells <dhowells@redhat.com> --- fs/ecryptfs/crypto.c | 4 +++- fs/ecryptfs/keystore.c | 15 ++++++++++++--- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c index bfd8b68..6b32776 100644 --- a/fs/ecryptfs/crypto.c +++ b/fs/ecryptfs/crypto.c @@ -268,8 +268,10 @@ void ecryptfs_destroy_mount_crypt_stat( list_del(&auth_tok->mount_crypt_stat_list); mount_crypt_stat->num_global_auth_toks--; if (auth_tok->global_auth_tok_key - && !(auth_tok->flags & ECRYPTFS_AUTH_TOK_INVALID)) + && !(auth_tok->flags & ECRYPTFS_AUTH_TOK_INVALID)) { + up_write(&(auth_tok->global_auth_tok_key->sem)); key_put(auth_tok->global_auth_tok_key); + } kmem_cache_free(ecryptfs_global_auth_tok_cache, auth_tok); } mutex_unlock(&mount_crypt_stat->global_auth_tok_list_mutex); diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c index 4feb78c..c90456a 100644 --- a/fs/ecryptfs/keystore.c +++ b/fs/ecryptfs/keystore.c @@ -765,8 +765,10 @@ out_free_unlock: out_unlock: mutex_unlock(s->tfm_mutex); out: - if (auth_tok_key) + if (auth_tok_key) { + up_write(&(auth_tok_key->sem)); key_put(auth_tok_key); + } kfree(s); return rc; } @@ -1002,8 +1004,10 @@ out: (*filename_size) = 0; (*filename) = NULL; } - if (auth_tok_key) + if (auth_tok_key) { + up_write(&(auth_tok_key->sem)); key_put(auth_tok_key); + } kfree(s); return rc; } @@ -1566,6 +1570,7 @@ int ecryptfs_keyring_auth_tok_for_sig(struct key **auth_tok_key, (*auth_tok_key) = NULL; goto out; } + down_write(&((*auth_tok_key)->sem)); (*auth_tok) = ecryptfs_get_key_payload_data(*auth_tok_key); if (ecryptfs_verify_version((*auth_tok)->version)) { printk(KERN_ERR @@ -1587,6 +1592,7 @@ int ecryptfs_keyring_auth_tok_for_sig(struct key **auth_tok_key, } out_release_key: if (rc) { + up_write(&((*auth_tok_key)->sem)); key_put(*auth_tok_key); (*auth_tok_key) = NULL; } @@ -1810,6 +1816,7 @@ int ecryptfs_parse_packet_set(struct ecryptfs_crypt_stat *crypt_stat, find_next_matching_auth_tok: found_auth_tok = 0; if (auth_tok_key) { + up_write(&(auth_tok_key->sem)); key_put(auth_tok_key); auth_tok_key = NULL; } @@ -1896,8 +1903,10 @@ found_matching_auth_tok: out_wipe_list: wipe_auth_tok_list(&auth_tok_list); out: - if (auth_tok_key) + if (auth_tok_key) { + up_write(&(auth_tok_key->sem)); key_put(auth_tok_key); + } return rc; } -- 1.7.3.4 [-- Attachment #2: smime.p7s --] [-- Type: application/x-pkcs7-signature, Size: 2061 bytes --] ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 2/2] eCryptfs: lock requested keys for writing 2011-01-28 10:42 ` [PATCH 2/2] eCryptfs: lock requested keys for writing Roberto Sassu @ 2011-02-01 23:16 ` Tyler Hicks 0 siblings, 0 replies; 3+ messages in thread From: Tyler Hicks @ 2011-02-01 23:16 UTC (permalink / raw) To: Roberto Sassu; +Cc: kirkland, dhowells, keyrings, linux-fsdevel, linux-kernel On Fri Jan 28, 2011 at 11:42:04AM +0100, Roberto Sassu <roberto.sassu@polito.it> wrote: > Keys requested by eCryptfs are exclusively locked in order to prevent > modifications by other subjects. In particular those which description is > specified at mount time are locked until the filesystem is unmounted, these I don't think the approach taken for the mount wide keys will work for a couple reasons. The first is that lockdep reports this after a mount: ================================================ [ BUG: lock held when returning to user space! ] ------------------------------------------------ mount/1019 is leaving the kernel with locks still held! 1 lock held by mount/1019: #0: (&key->sem){+.+.+.}, at: [<ffffffffa021ff12>] ecryptfs_keyring_auth_tok_for_sig+0xe6/0x195 [ecryptfs] The second is that while this does prevent key_update() from updating the key underneath us, it just results in `keyctl update` on a mount key to become a hung task until unmounting the eCryptfs mount. It looks like we'll have to have more efficient locking for mount keys. Tyler > required to decrypt a single file are unlocked immediately after the open. > > Signed-off-by: Roberto Sassu <roberto.sassu@polito.it> > Reported-by: David Howells <dhowells@redhat.com> > --- > fs/ecryptfs/crypto.c | 4 +++- > fs/ecryptfs/keystore.c | 15 ++++++++++++--- > 2 files changed, 15 insertions(+), 4 deletions(-) > > diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c > index bfd8b68..6b32776 100644 > --- a/fs/ecryptfs/crypto.c > +++ b/fs/ecryptfs/crypto.c > @@ -268,8 +268,10 @@ void ecryptfs_destroy_mount_crypt_stat( > list_del(&auth_tok->mount_crypt_stat_list); > mount_crypt_stat->num_global_auth_toks--; > if (auth_tok->global_auth_tok_key > - && !(auth_tok->flags & ECRYPTFS_AUTH_TOK_INVALID)) > + && !(auth_tok->flags & ECRYPTFS_AUTH_TOK_INVALID)) { > + up_write(&(auth_tok->global_auth_tok_key->sem)); > key_put(auth_tok->global_auth_tok_key); > + } > kmem_cache_free(ecryptfs_global_auth_tok_cache, auth_tok); > } > mutex_unlock(&mount_crypt_stat->global_auth_tok_list_mutex); > diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c > index 4feb78c..c90456a 100644 > --- a/fs/ecryptfs/keystore.c > +++ b/fs/ecryptfs/keystore.c > @@ -765,8 +765,10 @@ out_free_unlock: > out_unlock: > mutex_unlock(s->tfm_mutex); > out: > - if (auth_tok_key) > + if (auth_tok_key) { > + up_write(&(auth_tok_key->sem)); > key_put(auth_tok_key); > + } > kfree(s); > return rc; > } > @@ -1002,8 +1004,10 @@ out: > (*filename_size) = 0; > (*filename) = NULL; > } > - if (auth_tok_key) > + if (auth_tok_key) { > + up_write(&(auth_tok_key->sem)); > key_put(auth_tok_key); > + } > kfree(s); > return rc; > } > @@ -1566,6 +1570,7 @@ int ecryptfs_keyring_auth_tok_for_sig(struct key **auth_tok_key, > (*auth_tok_key) = NULL; > goto out; > } > + down_write(&((*auth_tok_key)->sem)); > (*auth_tok) = ecryptfs_get_key_payload_data(*auth_tok_key); > if (ecryptfs_verify_version((*auth_tok)->version)) { > printk(KERN_ERR > @@ -1587,6 +1592,7 @@ int ecryptfs_keyring_auth_tok_for_sig(struct key **auth_tok_key, > } > out_release_key: > if (rc) { > + up_write(&((*auth_tok_key)->sem)); > key_put(*auth_tok_key); > (*auth_tok_key) = NULL; > } > @@ -1810,6 +1816,7 @@ int ecryptfs_parse_packet_set(struct ecryptfs_crypt_stat *crypt_stat, > find_next_matching_auth_tok: > found_auth_tok = 0; > if (auth_tok_key) { > + up_write(&(auth_tok_key->sem)); > key_put(auth_tok_key); > auth_tok_key = NULL; > } > @@ -1896,8 +1903,10 @@ found_matching_auth_tok: > out_wipe_list: > wipe_auth_tok_list(&auth_tok_list); > out: > - if (auth_tok_key) > + if (auth_tok_key) { > + up_write(&(auth_tok_key->sem)); > key_put(auth_tok_key); > + } > return rc; > } > > -- > 1.7.3.4 > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-02-01 23:16 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-01-28 10:42 [PATCH 1/2] eCryptfs: ecryptfs_keyring_auth_tok_for_sig() bug fix Roberto Sassu 2011-01-28 10:42 ` [PATCH 2/2] eCryptfs: lock requested keys for writing Roberto Sassu 2011-02-01 23:16 ` Tyler Hicks
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.