On Mon, Apr 10, 2023 at 03:39:55PM -0400, Sweet Tea Dorminy wrote:
> /*
> - * 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,13 @@ static int setup_file_encryption_key(struct fscrypt_info *ci,
> mk = fscrypt_find_master_key(sb, &mk_spec);
> }
> }
> +
> if (unlikely(!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);
> + *mk_ret = NULL;
> + return 0;
While this change may be a benefit overall, it does split the code that handles
the legacy case of "v1 policy using process-subscribed keyrings" into two
places. That makes it a little more difficult to understand. I think a comment
would be helpful here, at least?
- Eric
|