Sweet Tea Dorminy <sweettea-kernel@xxxxxxxxxx> writes:
> Right now fscrypt_infos have two fields dedicated solely to recording
> what type of prepared key the info has: whether it solely owns the
> prepared key, or has borrowed it from a master key, or from a direct
> key.
>
> The ci_direct_key field is only used for v1 direct key policies,
> recording the direct key that needs to have its refcount reduced when
> the crypt_info is freed. However, now that crypt_info->ci_enc_key is a
> pointer to the authoritative prepared key -- embedded in the direct key,
> in this case, we no longer need to keep a full pointer to the direct key
> -- we can use container_of() to go from the prepared key to its
> surrounding direct key.
>
> The key ownership information doesn't change during the lifetime of a
> prepared key. Since at worst there's a prepared key per info, and at
> best many infos share a single prepared key, it can be slightly more
> efficient to store this ownership info in the prepared key instead of in
> the fscrypt_info, especially since we can squash both fields down into
> a single enum.
>
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@xxxxxxxxxx>
> ---
> fs/crypto/fscrypt_private.h | 31 +++++++++++++++++++++++--------
> fs/crypto/keysetup.c | 21 +++++++++++++--------
> fs/crypto/keysetup_v1.c | 7 +++++--
> 3 files changed, 41 insertions(+), 18 deletions(-)
>
> diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
> index 5011737b60b3..e726a1fb9f7e 100644
> --- a/fs/crypto/fscrypt_private.h
> +++ b/fs/crypto/fscrypt_private.h
> @@ -174,18 +174,39 @@ struct fscrypt_symlink_data {
> char encrypted_path[1];
> } __packed;
>
> +/**
> + * enum fscrypt_prepared_key_type - records a prepared key's ownership
> + *
> + * @FSCRYPT_KEY_PER_INFO: this prepared key is allocated for a specific info
> + * and is never shared.
> + * @FSCRYPT_KEY_DIRECT_V1: this prepared key is embedded in a fscrypt_direct_key
> + * used in v1 direct key policies.
> + * @FSCRYPT_KEY_MASTER_KEY: this prepared key is a per-mode and policy key,
> + * part of a fscrypt_master_key, shared between all
> + * users of this master key having this mode and
> + * policy.
> + */
> +enum fscrypt_prepared_key_type {
> + FSCRYPT_KEY_PER_INFO = 1,
> + FSCRYPT_KEY_DIRECT_V1,
> + FSCRYPT_KEY_MASTER_KEY,
> +} __packed;
> +
> /**
> * struct fscrypt_prepared_key - a key prepared for actual encryption/decryption
> * @tfm: crypto API transform object
> * @blk_key: key for blk-crypto
> + * @type: records the ownership type of the prepared key
> *
> - * Normally only one of the fields will be non-NULL.
> + * Normally only one of @tfm and @blk_key will be non-NULL, although it is
> + * possible if @type is FSCRYPT_KEY_MASTER_KEY.
> */
> struct fscrypt_prepared_key {
> struct crypto_skcipher *tfm;
> #ifdef CONFIG_FS_ENCRYPTION_INLINE_CRYPT
> struct blk_crypto_key *blk_key;
> #endif
> + enum fscrypt_prepared_key_type type;
> };
>
> /*
> @@ -233,12 +254,6 @@ struct fscrypt_info {
> */
> struct list_head ci_master_key_link;
>
> - /*
> - * If non-NULL, then encryption is done using the master key directly
> - * and ci_enc_key will equal ci_direct_key->dk_key.
> - */
> - struct fscrypt_direct_key *ci_direct_key;
> -
> /*
> * This inode's hash key for filenames. This is a 128-bit SipHash-2-4
> * key. This is only set for directories that use a keyed dirhash over
> @@ -641,7 +656,7 @@ static inline int fscrypt_require_key(struct inode *inode)
>
> /* keysetup_v1.c */
>
> -void fscrypt_put_direct_key(struct fscrypt_direct_key *dk);
> +void fscrypt_put_direct_key(struct fscrypt_prepared_key *prep_key);
>
> int fscrypt_setup_v1_file_key(struct fscrypt_info *ci,
> const u8 *raw_master_key);
> diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
> index 4f04999ecfd1..a19650f954e2 100644
> --- a/fs/crypto/keysetup.c
> +++ b/fs/crypto/keysetup.c
> @@ -191,11 +191,11 @@ void fscrypt_destroy_prepared_key(struct super_block *sb,
> /* Given a per-file encryption key, set up the file's crypto transform object */
> int fscrypt_set_per_file_enc_key(struct fscrypt_info *ci, const u8 *raw_key)
> {
> - ci->ci_owns_key = true;
> ci->ci_enc_key = kzalloc(sizeof(*ci->ci_enc_key), GFP_KERNEL);
> if (!ci->ci_enc_key)
> return -ENOMEM;
>
> + ci->ci_enc_key->type = FSCRYPT_KEY_PER_INFO;
> return fscrypt_prepare_key(ci->ci_enc_key, raw_key, ci);
> }
>
> @@ -290,7 +290,8 @@ static int setup_new_mode_prepared_key(struct fscrypt_master_key *mk,
> hkdf_context, hkdf_info, hkdf_infolen,
> mode_key, mode->keysize);
> if (err)
> - goto out_unlock;
> + return err;
Is this change really intended? I guess it's a mistake, because if we
simply return we'll leave keysetup mutex locked, which is probably not
what we want here.
Cheers,
--
Luís
> + prep_key->type = FSCRYPT_KEY_MASTER_KEY;
> err = fscrypt_prepare_key(prep_key, mode_key, ci);
> memzero_explicit(mode_key, mode->keysize);
>
|