Re: [PATCH v5 7/9] fscrypt: add inline encryption support

[Christoph Hellwig <hch@infradead.org>]

> diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c
> index 1f4b8a277060..956798debf71 100644
> --- a/fs/crypto/bio.c
> +++ b/fs/crypto/bio.c
> @@ -46,26 +46,38 @@ int fscrypt_zeroout_range(const struct inode *inode, pgoff_t lblk,
>  {
>  	const unsigned int blockbits = inode->i_blkbits;
>  	const unsigned int blocksize = 1 << blockbits;
> +	const bool inlinecrypt = fscrypt_inode_uses_inline_crypto(inode);
>  	struct page *ciphertext_page;
>  	struct bio *bio;
>  	int ret, err = 0;
>  
> -	ciphertext_page = fscrypt_alloc_bounce_page(GFP_NOWAIT);
> -	if (!ciphertext_page)
> -		return -ENOMEM;
> +	if (inlinecrypt) {
> +		ciphertext_page = ZERO_PAGE(0);
> +	} else {
> +		ciphertext_page = fscrypt_alloc_bounce_page(GFP_NOWAIT);
> +		if (!ciphertext_page)
> +			return -ENOMEM;

I think you just want to split this into two functions for the
inline crypto vs not cases.

> @@ -391,6 +450,16 @@ struct fscrypt_master_key {
>  	 */
>  	struct crypto_skcipher	*mk_iv_ino_lblk_64_tfms[__FSCRYPT_MODE_MAX + 1];
>  
> +#ifdef CONFIG_FS_ENCRYPTION_INLINE_CRYPT
> +	/* Raw keys for IV_INO_LBLK_64 policies, allocated on-demand */
> +	u8			*mk_iv_ino_lblk_64_raw_keys[__FSCRYPT_MODE_MAX + 1];
> +
> +	/* The data unit size being used for inline encryption */
> +	unsigned int		mk_data_unit_size;
> +
> +	/* The filesystem's block device */
> +	struct block_device	*mk_bdev;

File systems (including f2fs) can have multiple underlying block
devices.  

> +{
> +	const struct inode *inode = ci->ci_inode;
> +	struct super_block *sb = inode->i_sb;
> +
> +	/* The file must need contents encryption, not filenames encryption */
> +	if (!S_ISREG(inode->i_mode))
> +		return false;

But that isn't really what the check checks for..

> +	/* The filesystem must be mounted with -o inlinecrypt */
> +	if (!sb->s_cop->inline_crypt_enabled ||
> +	    !sb->s_cop->inline_crypt_enabled(sb))
> +		return false;

So please add a SB_* flag for that option instead of the weird
indirection.

> +/**
> + * fscrypt_inode_uses_inline_crypto - test whether an inode uses inline encryption

This adds an overly long line.  This happens many more times in the
patch.

Btw, I'm not happy about the 8-byte IV assumptions everywhere here.
That really should be a parameter, not hardcoded.