On Tue, Aug 08, 2023 at 01:08:02PM -0400, Sweet Tea Dorminy wrote:
> At present, setup_file_encryption_key() does several things: it finds
> and locks the master key, and then calls into the appropriate functions
> to setup the prepared key for the fscrypt_info. The code is clearer to
> follow if these functions are divided.
>
> Thus, move calling the appropriate file key setup function into a new
> fscrypt_setup_file_key() function. After the file key setup functions
> are moved, the remaining function can take a const fscrypt_info, and is
> renamed find_and_lock_master_key() to precisely describe its action.
>
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@xxxxxxxxxx>
> ---
> fs/crypto/keysetup.c | 77 ++++++++++++++++++++++++++++++--------------
> 1 file changed, 52 insertions(+), 25 deletions(-)
>
> diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
> index b89c32ad19fb..727d473b6b03 100644
> --- a/fs/crypto/keysetup.c
> +++ b/fs/crypto/keysetup.c
> @@ -386,6 +386,43 @@ static int fscrypt_setup_v2_file_key(struct fscrypt_info *ci,
> return 0;
> }
>
> +/*
> + * Find or create the appropriate prepared key for an info.
> + */
> +static int fscrypt_setup_file_key(struct fscrypt_info *ci,
> + struct fscrypt_master_key *mk,
> + bool need_dirhash_key)
> +{
> + int err;
> +
> + if (!mk) {
> + if (ci->ci_policy.version != FSCRYPT_POLICY_V1)
> + return -ENOKEY;
> +
> + /*
> + * As a legacy fallback for v1 policies, search for the key in
> + * the current task's subscribed keyrings too. Don't move this
> + * to before the search of ->s_master_keys, since users
> + * shouldn't be able to override filesystem-level keys.
> + */
> + return fscrypt_setup_v1_file_key_via_subscribed_keyrings(ci);
> + }
> +
> + switch (ci->ci_policy.version) {
> + case FSCRYPT_POLICY_V1:
> + err = fscrypt_setup_v1_file_key(ci, mk->mk_secret.raw);
> + break;
> + case FSCRYPT_POLICY_V2:
> + err = fscrypt_setup_v2_file_key(ci, mk, need_dirhash_key);
> + break;
> + default:
> + WARN_ON_ONCE(1);
> + err = -EINVAL;
> + break;
> + }
> + return err;
> +}
> +
> /*
> * Check whether the size of the given master key (@mk) is appropriate for the
> * encryption settings which a particular file will use (@ci).
> @@ -426,7 +463,7 @@ static bool fscrypt_valid_master_key_size(const struct fscrypt_master_key *mk,
> }
>
> /*
> - * Find the master key, then set up the inode's actual encryption key.
> + * Find and lock the master key.
> *
> * If the master key is found in the filesystem-level keyring, then it is
> * returned in *mk_ret with its semaphore read-locked. This is needed to ensure
> @@ -434,9 +471,8 @@ static bool fscrypt_valid_master_key_size(const struct fscrypt_master_key *mk,
> * multiple tasks may race to create an fscrypt_info for the same inode), and to
> * synchronize the master key being removed with a new inode starting to use it.
> */
> -static int setup_file_encryption_key(struct fscrypt_info *ci,
> - bool need_dirhash_key,
> - struct fscrypt_master_key **mk_ret)
> +static int find_and_lock_master_key(const struct fscrypt_info *ci,
> + struct fscrypt_master_key **mk_ret)
> {
> struct super_block *sb = ci->ci_inode->i_sb;
> struct fscrypt_key_specifier mk_spec;
> @@ -466,17 +502,19 @@ static int setup_file_encryption_key(struct fscrypt_info *ci,
> mk = fscrypt_find_master_key(sb, &mk_spec);
> }
> }
> +
Random newline, you can add
Reviewed-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
once you fix it up. Thanks,
Josef
|