On Fri Jan 28, 2011 at 11:42:04AM +0100, Roberto Sassu <roberto.sassu@xxxxxxxxx> 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@xxxxxxxxx>
> Reported-by: David Howells <dhowells@xxxxxxxxxx>
> ---
> 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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
