Re: [PATCH v6 5/8] fscrypt: reduce special-casing of IV_INO_LBLK_32

[Eric Biggers <ebiggers@kernel.org>]

On Tue, Aug 08, 2023 at 01:08:05PM -0400, Sweet Tea Dorminy wrote:
> Right now, the IV_INO_LBLK_32 policy is handled by its own function
> called in fscrypt_setup_v2_file_key(), different from all other policies
> which just call find_mode_prepared_key() with various parameters. The
> function additionally sets up the relevant inode hashing key in the
> master key, and uses it to hash the inode number if possible. This is
> not particularly relevant to setting up a prepared key, so this change
> tries to make it clear that every non-default policy uses basically the
> same setup mechanism for its prepared key. The other setup is moved to
> be called from the top crypt_info setup function.
> 
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>

It seems the goal of this patch is to finish the refactoring started by patches
2 and 4 of making fscrypt_setup_file_key() only set up the I/O key ("prepared
key").  The title and description don't make it very clear, though.  I think a
better title would be the following which is analogous to patch 4:

    fscrypt: move ino hashing setup away from IO key setup

BTW, it seems patch 3 should not be where it is in the series, since 2, 4, and 5
seem to be on one topic.

> +static int fscrypt_setup_ino_hash_key(struct fscrypt_master_key *mk)
>  {
>  	int err;

err needs to be initialized to 0.

> +	/*
> +	 * The IV_INO_LBLK_32 policy needs a hashed inode number, but new
> +	 * inodes may not have an inode number assigned yet.
> +	 */
> +	if (policy->version == FSCRYPT_POLICY_V2 &&
> +	    (policy->v2.flags & FSCRYPT_POLICY_FLAG_IV_INO_LBLK_32)) {
> +		res = fscrypt_setup_ino_hash_key(mk);
> +		if (res)
> +			goto out;
> +
> +		if (inode->i_ino)
> +			fscrypt_hash_inode_number(crypt_info, mk);
> +	}

This seems to be another case where a comment was copied but it doesn't make as
much sense in the new context.  How about the following:

	/* 
	 * If the file is using an IV_INO_LBLK_32 policy, derive the inode hash
	 * key if it wasn't done already.  Then hash the inode number and cache
         * the resulting hash.  New inodes might not have an inode number
         * assigned yet; hashing their inode number is delayed until later.
	 */

- Eric