[PATCH 0/4] Inline Encryption Support for fscrypt

[PATCH 0/4] Inline Encryption Support for fscrypt

[Satya Tangirala]

This patch series adds support for Inline Encryption to fscrypt, f2fs
and ext4. It builds on the inline encryption support now present in
the block layer, and has been rebased on v5.8-rc1.

Patch 1 introduces the SB_INLINECRYPT sb options, which filesystems
should set if they want to use blk-crypto for file content en/decryption.

Patch 2 adds inline encryption support to fscrypt. To use inline
encryption with fscrypt, the filesystem must set the above mentioned
SB_INLINECRYPT sb option. When this option is set, the contents of
encrypted files will be en/decrypted using blk-crypto.

Patches 3 and 4 wire up f2fs and ext4 respectively to fscrypt support for
inline encryption, and e.g ensure that bios are submitted with blocks
that not only are contiguous, but also have contiguous DUNs.

Eric Biggers (1):
  ext4: add inline encryption support

Satya Tangirala (3):
  fs: introduce SB_INLINECRYPT
  fscrypt: add inline encryption support
  f2fs: add inline encryption support

 Documentation/admin-guide/ext4.rst |   6 +
 Documentation/filesystems/f2fs.rst |   7 +-
 fs/buffer.c                        |   7 +-
 fs/crypto/Kconfig                  |   6 +
 fs/crypto/Makefile                 |   1 +
 fs/crypto/bio.c                    |  50 +++++
 fs/crypto/crypto.c                 |   2 +-
 fs/crypto/fname.c                  |   4 +-
 fs/crypto/fscrypt_private.h        | 118 +++++++++-
 fs/crypto/inline_crypt.c           | 349 +++++++++++++++++++++++++++++
 fs/crypto/keyring.c                |   6 +-
 fs/crypto/keysetup.c               |  68 ++++--
 fs/crypto/keysetup_v1.c            |  16 +-
 fs/ext4/inode.c                    |   4 +-
 fs/ext4/page-io.c                  |   6 +-
 fs/ext4/readpage.c                 |  11 +-
 fs/ext4/super.c                    |   9 +
 fs/f2fs/compress.c                 |   2 +-
 fs/f2fs/data.c                     |  81 +++++--
 fs/f2fs/super.c                    |  32 +++
 fs/proc_namespace.c                |   1 +
 include/linux/fs.h                 |   1 +
 include/linux/fscrypt.h            |  82 +++++++
 23 files changed, 794 insertions(+), 75 deletions(-)
 create mode 100644 fs/crypto/inline_crypt.c

-- 
2.27.0.290.gba653c62da-goog

[PATCH 1/4] fs: introduce SB_INLINECRYPT

[Satya Tangirala]

Introduce SB_INLINECRYPT, which is set by filesystems that wish to use
blk-crypto for file content en/decryption. This flag maps to the
'-o inlinecrypt' mount option which multiple filesystems will implement,
and code in fs/crypto/ needs to be able to check for this mount option
in a filesystem-independent way.

Signed-off-by: Satya Tangirala <satyat@google.com>
---
 fs/proc_namespace.c | 1 +
 include/linux/fs.h  | 1 +
 2 files changed, 2 insertions(+)

diff --git a/fs/proc_namespace.c b/fs/proc_namespace.c
index 3059a9394c2d..e0ff1f6ac8f1 100644
--- a/fs/proc_namespace.c
+++ b/fs/proc_namespace.c
@@ -49,6 +49,7 @@ static int show_sb_opts(struct seq_file *m, struct super_block *sb)
 		{ SB_DIRSYNC, ",dirsync" },
 		{ SB_MANDLOCK, ",mand" },
 		{ SB_LAZYTIME, ",lazytime" },
+		{ SB_INLINECRYPT, ",inlinecrypt" },
 		{ 0, NULL }
 	};
 	const struct proc_fs_opts *fs_infop;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 6c4ab4dc1cd7..abef6aa95524 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1380,6 +1380,7 @@ extern int send_sigurg(struct fown_struct *fown);
 #define SB_NODIRATIME	2048	/* Do not update directory access times */
 #define SB_SILENT	32768
 #define SB_POSIXACL	(1<<16)	/* VFS does not apply the umask */
+#define SB_INLINECRYPT	(1<<17)	/* Use blk-crypto for encrypted files */
 #define SB_KERNMOUNT	(1<<22) /* this is a kern_mount call */
 #define SB_I_VERSION	(1<<23) /* Update inode I_version field */
 #define SB_LAZYTIME	(1<<25) /* Update the on-disk [acm]times lazily */
-- 
2.27.0.290.gba653c62da-goog

Re: [PATCH 1/4] fs: introduce SB_INLINECRYPT

[Jaegeuk Kim]

On 06/17, Satya Tangirala wrote:
> Introduce SB_INLINECRYPT, which is set by filesystems that wish to use
> blk-crypto for file content en/decryption. This flag maps to the
> '-o inlinecrypt' mount option which multiple filesystems will implement,
> and code in fs/crypto/ needs to be able to check for this mount option
> in a filesystem-independent way.
> 
> Signed-off-by: Satya Tangirala <satyat@google.com>

Reviewed-by: Jaegeuk Kim <jaegeuk@kernel.org>

> ---
>  fs/proc_namespace.c | 1 +
>  include/linux/fs.h  | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/fs/proc_namespace.c b/fs/proc_namespace.c
> index 3059a9394c2d..e0ff1f6ac8f1 100644
> --- a/fs/proc_namespace.c
> +++ b/fs/proc_namespace.c
> @@ -49,6 +49,7 @@ static int show_sb_opts(struct seq_file *m, struct super_block *sb)
>  		{ SB_DIRSYNC, ",dirsync" },
>  		{ SB_MANDLOCK, ",mand" },
>  		{ SB_LAZYTIME, ",lazytime" },
> +		{ SB_INLINECRYPT, ",inlinecrypt" },
>  		{ 0, NULL }
>  	};
>  	const struct proc_fs_opts *fs_infop;
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 6c4ab4dc1cd7..abef6aa95524 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1380,6 +1380,7 @@ extern int send_sigurg(struct fown_struct *fown);
>  #define SB_NODIRATIME	2048	/* Do not update directory access times */
>  #define SB_SILENT	32768
>  #define SB_POSIXACL	(1<<16)	/* VFS does not apply the umask */
> +#define SB_INLINECRYPT	(1<<17)	/* Use blk-crypto for encrypted files */
>  #define SB_KERNMOUNT	(1<<22) /* this is a kern_mount call */
>  #define SB_I_VERSION	(1<<23) /* Update inode I_version field */
>  #define SB_LAZYTIME	(1<<25) /* Update the on-disk [acm]times lazily */
> -- 
> 2.27.0.290.gba653c62da-goog

Re: [PATCH 1/4] fs: introduce SB_INLINECRYPT

[Dave Chinner]

On Wed, Jun 17, 2020 at 07:57:29AM +0000, Satya Tangirala wrote:
> Introduce SB_INLINECRYPT, which is set by filesystems that wish to use
> blk-crypto for file content en/decryption. This flag maps to the
> '-o inlinecrypt' mount option which multiple filesystems will implement,
> and code in fs/crypto/ needs to be able to check for this mount option
> in a filesystem-independent way.
> 
> Signed-off-by: Satya Tangirala <satyat@google.com>
> ---
>  fs/proc_namespace.c | 1 +
>  include/linux/fs.h  | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/fs/proc_namespace.c b/fs/proc_namespace.c
> index 3059a9394c2d..e0ff1f6ac8f1 100644
> --- a/fs/proc_namespace.c
> +++ b/fs/proc_namespace.c
> @@ -49,6 +49,7 @@ static int show_sb_opts(struct seq_file *m, struct super_block *sb)
>  		{ SB_DIRSYNC, ",dirsync" },
>  		{ SB_MANDLOCK, ",mand" },
>  		{ SB_LAZYTIME, ",lazytime" },
> +		{ SB_INLINECRYPT, ",inlinecrypt" },
>  		{ 0, NULL }
>  	};
>  	const struct proc_fs_opts *fs_infop;

NACK.

SB_* flgs are for functionality enabled on the superblock, not for
indicating mount options that have been set by the user.

If the mount options are directly parsed by the filesystem option
parser (as is done later in this patchset), then the mount option
setting should be emitted by the filesystem's ->show_options
function, not a generic function.

The option string must match what the filesystem defines, not
require separate per-filesystem and VFS definitions of the same
option that people could potentially get wrong (*cough* i_version vs
iversion *cough*)....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 1/4] fs: introduce SB_INLINECRYPT

[Eric Biggers]

On Thu, Jun 18, 2020 at 11:19:12AM +1000, Dave Chinner wrote:
> On Wed, Jun 17, 2020 at 07:57:29AM +0000, Satya Tangirala wrote:
> > Introduce SB_INLINECRYPT, which is set by filesystems that wish to use
> > blk-crypto for file content en/decryption. This flag maps to the
> > '-o inlinecrypt' mount option which multiple filesystems will implement,
> > and code in fs/crypto/ needs to be able to check for this mount option
> > in a filesystem-independent way.
> > 
> > Signed-off-by: Satya Tangirala <satyat@google.com>
> > ---
> >  fs/proc_namespace.c | 1 +
> >  include/linux/fs.h  | 1 +
> >  2 files changed, 2 insertions(+)
> > 
> > diff --git a/fs/proc_namespace.c b/fs/proc_namespace.c
> > index 3059a9394c2d..e0ff1f6ac8f1 100644
> > --- a/fs/proc_namespace.c
> > +++ b/fs/proc_namespace.c
> > @@ -49,6 +49,7 @@ static int show_sb_opts(struct seq_file *m, struct super_block *sb)
> >  		{ SB_DIRSYNC, ",dirsync" },
> >  		{ SB_MANDLOCK, ",mand" },
> >  		{ SB_LAZYTIME, ",lazytime" },
> > +		{ SB_INLINECRYPT, ",inlinecrypt" },
> >  		{ 0, NULL }
> >  	};
> >  	const struct proc_fs_opts *fs_infop;
> 
> NACK.
> 
> SB_* flgs are for functionality enabled on the superblock, not for
> indicating mount options that have been set by the user.

That's an interesting claim, given that most SB_* flags are for mount options.
E.g.:

	ro => SB_RDONLY
	nosuid => SB_NOSUID
	nodev => SB_NODEV
	noexec => SB_NOEXEC
	sync => SB_SYNCHRONOUS
	mand => SB_MANDLOCK
	noatime => SB_NOATIME
	nodiratime => SB_NODIRATIME
	lazytime => SB_LAZYTIME

> 
> If the mount options are directly parsed by the filesystem option
> parser (as is done later in this patchset), then the mount option
> setting should be emitted by the filesystem's ->show_options
> function, not a generic function.
> 
> The option string must match what the filesystem defines, not
> require separate per-filesystem and VFS definitions of the same
> option that people could potentially get wrong (*cough* i_version vs
> iversion *cough*)....

Are you objecting to the use of a SB_* flag, or just to showing the flag in
show_sb_opts() instead of in the individual filesystems?  Note that the SB_*
flag was requested by Christoph
(https://lkml.kernel.org/r/20191031183217.GF23601@infradead.org/,
https://lkml.kernel.org/r/20191031212103.GA6244@infradead.org/).  We originally
used a function fscrypt_operations::inline_crypt_enabled() instead.

- Eric

Re: [PATCH 1/4] fs: introduce SB_INLINECRYPT

[Dave Chinner]

On Wed, Jun 17, 2020 at 08:19:35PM -0700, Eric Biggers wrote:
> On Thu, Jun 18, 2020 at 11:19:12AM +1000, Dave Chinner wrote:
> > On Wed, Jun 17, 2020 at 07:57:29AM +0000, Satya Tangirala wrote:
> > > Introduce SB_INLINECRYPT, which is set by filesystems that wish to use
> > > blk-crypto for file content en/decryption. This flag maps to the
> > > '-o inlinecrypt' mount option which multiple filesystems will implement,
> > > and code in fs/crypto/ needs to be able to check for this mount option
> > > in a filesystem-independent way.
> > > 
> > > Signed-off-by: Satya Tangirala <satyat@google.com>
> > > ---
> > >  fs/proc_namespace.c | 1 +
> > >  include/linux/fs.h  | 1 +
> > >  2 files changed, 2 insertions(+)
> > > 
> > > diff --git a/fs/proc_namespace.c b/fs/proc_namespace.c
> > > index 3059a9394c2d..e0ff1f6ac8f1 100644
> > > --- a/fs/proc_namespace.c
> > > +++ b/fs/proc_namespace.c
> > > @@ -49,6 +49,7 @@ static int show_sb_opts(struct seq_file *m, struct super_block *sb)
> > >  		{ SB_DIRSYNC, ",dirsync" },
> > >  		{ SB_MANDLOCK, ",mand" },
> > >  		{ SB_LAZYTIME, ",lazytime" },
> > > +		{ SB_INLINECRYPT, ",inlinecrypt" },
> > >  		{ 0, NULL }
> > >  	};
> > >  	const struct proc_fs_opts *fs_infop;
> > 
> > NACK.
> > 
> > SB_* flgs are for functionality enabled on the superblock, not for
> > indicating mount options that have been set by the user.
> 
> That's an interesting claim, given that most SB_* flags are for mount options.
> E.g.:
> 
> 	ro => SB_RDONLY
> 	nosuid => SB_NOSUID
> 	nodev => SB_NODEV
> 	noexec => SB_NOEXEC
> 	sync => SB_SYNCHRONOUS
> 	mand => SB_MANDLOCK
> 	noatime => SB_NOATIME
> 	nodiratime => SB_NODIRATIME
> 	lazytime => SB_LAZYTIME

Yes, they *reflect* options set by mount options, but this is all so
screwed up because the split of superblock functionality from the
mount option API (i.e. the MS_* flag introduction to avoid having
the superblock feature flags being directly defined by the userspace
mount API) was never followed through to properly separate the
implementation of *active superblock feature flags* from the *user
specified mount API flags*.

Yes, the UAPI definitions were separated, but the rest of the
interface wasn't and only works because of the "MS* flag exactly
equal to the SB* flag" hack that was used. So now people have no
idea when to use one or the other and we're repeatedly ending up
with broken mount option parsing because SB flags are used where MS
flags should be used and vice versa.

We've made a damn mess of mount options, and the fscontext stuff
hasn't fixed any of this ... mess. It's just stirred it around and so
nobody really knows what they are supposed to with mount options
right now.

> > If the mount options are directly parsed by the filesystem option
> > parser (as is done later in this patchset), then the mount option
> > setting should be emitted by the filesystem's ->show_options
> > function, not a generic function.
> > 
> > The option string must match what the filesystem defines, not
> > require separate per-filesystem and VFS definitions of the same
> > option that people could potentially get wrong (*cough* i_version vs
> > iversion *cough*)....
> 
> Are you objecting to the use of a SB_* flag, or just to showing the flag in
> show_sb_opts() instead of in the individual filesystems?  Note that the SB_*
> flag was requested by Christoph
> (https://lkml.kernel.org/r/20191031183217.GF23601@infradead.org/,
> https://lkml.kernel.org/r/20191031212103.GA6244@infradead.org/).  We originally
> used a function fscrypt_operations::inline_crypt_enabled() instead.

I'm objecting to the layering violations of having the filesystem
control the mount option parsing and superblock feature flags, but
then having no control over whether features that the filesystem has
indicated to the VFS it is using get emitted as a mount option or
not, and then having the VFS code unconditionally override the
functionality that the filesystem uses because it thinks it's a
mount option the filesystem supports....

For example, the current mess that has just come to light:
filesystems like btrfs and XFS v5 which set SB_IVERSION
unconditionally (i.e. it's not a mount option!) end up having that
functionality turned off on remount because the VFS conflates
MS_IVERSION with SB_IVERSION and so unconditionally clears
SB_IVERSION because MS_IVERSION is not set on remount by userspace.
Which userspace will never set be because the filesystems don't put
"iversion" in their mount option strings because -its not a mount
option- for those filesystems.

See the problem?  MS_IVERSION should be passed to the filesystem to
deal with as a mount option, not treated as a flag to directly
change SB_IVERSION in the superblock.

We really need to stop with the "global mount options for everyone
at the VFS" and instead pass everything down to the filesystems to
parse appropriately. Yes, provide generic helper functions to deal
with the common flags that everything supports, but the filesystems
should be masking off mount options they doesn't support changing
before changing their superblock feature support mask....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [f2fs-dev] [PATCH 1/4] fs: introduce SB_INLINECRYPT

[Eric Biggers]

Hi Dave,

On Tue, Jun 23, 2020 at 10:46:36AM +1000, Dave Chinner wrote:
> On Wed, Jun 17, 2020 at 08:19:35PM -0700, Eric Biggers wrote:
> > On Thu, Jun 18, 2020 at 11:19:12AM +1000, Dave Chinner wrote:
> > > On Wed, Jun 17, 2020 at 07:57:29AM +0000, Satya Tangirala wrote:
> > > > Introduce SB_INLINECRYPT, which is set by filesystems that wish to use
> > > > blk-crypto for file content en/decryption. This flag maps to the
> > > > '-o inlinecrypt' mount option which multiple filesystems will implement,
> > > > and code in fs/crypto/ needs to be able to check for this mount option
> > > > in a filesystem-independent way.
> > > > 
> > > > Signed-off-by: Satya Tangirala <satyat@google.com>
> > > > ---
> > > >  fs/proc_namespace.c | 1 +
> > > >  include/linux/fs.h  | 1 +
> > > >  2 files changed, 2 insertions(+)
> > > > 
> > > > diff --git a/fs/proc_namespace.c b/fs/proc_namespace.c
> > > > index 3059a9394c2d..e0ff1f6ac8f1 100644
> > > > --- a/fs/proc_namespace.c
> > > > +++ b/fs/proc_namespace.c
> > > > @@ -49,6 +49,7 @@ static int show_sb_opts(struct seq_file *m, struct super_block *sb)
> > > >  		{ SB_DIRSYNC, ",dirsync" },
> > > >  		{ SB_MANDLOCK, ",mand" },
> > > >  		{ SB_LAZYTIME, ",lazytime" },
> > > > +		{ SB_INLINECRYPT, ",inlinecrypt" },
> > > >  		{ 0, NULL }
> > > >  	};
> > > >  	const struct proc_fs_opts *fs_infop;
> > > 
> > > NACK.
> > > 
> > > SB_* flgs are for functionality enabled on the superblock, not for
> > > indicating mount options that have been set by the user.
> > 
> > That's an interesting claim, given that most SB_* flags are for mount options.
> > E.g.:
> > 
> > 	ro => SB_RDONLY
> > 	nosuid => SB_NOSUID
> > 	nodev => SB_NODEV
> > 	noexec => SB_NOEXEC
> > 	sync => SB_SYNCHRONOUS
> > 	mand => SB_MANDLOCK
> > 	noatime => SB_NOATIME
> > 	nodiratime => SB_NODIRATIME
> > 	lazytime => SB_LAZYTIME
> 
> Yes, they *reflect* options set by mount options, but this is all so
> screwed up because the split of superblock functionality from the
> mount option API (i.e. the MS_* flag introduction to avoid having
> the superblock feature flags being directly defined by the userspace
> mount API) was never followed through to properly separate the
> implementation of *active superblock feature flags* from the *user
> specified mount API flags*.
> 
> Yes, the UAPI definitions were separated, but the rest of the
> interface wasn't and only works because of the "MS* flag exactly
> equal to the SB* flag" hack that was used. So now people have no
> idea when to use one or the other and we're repeatedly ending up
> with broken mount option parsing because SB flags are used where MS
> flags should be used and vice versa.
> 
> We've made a damn mess of mount options, and the fscontext stuff
> hasn't fixed any of this ... mess. It's just stirred it around and so
> nobody really knows what they are supposed to with mount options
> right now.
> 
> > > If the mount options are directly parsed by the filesystem option
> > > parser (as is done later in this patchset), then the mount option
> > > setting should be emitted by the filesystem's ->show_options
> > > function, not a generic function.
> > > 
> > > The option string must match what the filesystem defines, not
> > > require separate per-filesystem and VFS definitions of the same
> > > option that people could potentially get wrong (*cough* i_version vs
> > > iversion *cough*)....
> > 
> > Are you objecting to the use of a SB_* flag, or just to showing the flag in
> > show_sb_opts() instead of in the individual filesystems?  Note that the SB_*
> > flag was requested by Christoph
> > (https://lkml.kernel.org/r/20191031183217.GF23601@infradead.org/,
> > https://lkml.kernel.org/r/20191031212103.GA6244@infradead.org/).  We originally
> > used a function fscrypt_operations::inline_crypt_enabled() instead.
> 
> I'm objecting to the layering violations of having the filesystem
> control the mount option parsing and superblock feature flags, but
> then having no control over whether features that the filesystem has
> indicated to the VFS it is using get emitted as a mount option or
> not, and then having the VFS code unconditionally override the
> functionality that the filesystem uses because it thinks it's a
> mount option the filesystem supports....
> 
> For example, the current mess that has just come to light:
> filesystems like btrfs and XFS v5 which set SB_IVERSION
> unconditionally (i.e. it's not a mount option!) end up having that
> functionality turned off on remount because the VFS conflates
> MS_IVERSION with SB_IVERSION and so unconditionally clears
> SB_IVERSION because MS_IVERSION is not set on remount by userspace.
> Which userspace will never set be because the filesystems don't put
> "iversion" in their mount option strings because -its not a mount
> option- for those filesystems.
> 
> See the problem?  MS_IVERSION should be passed to the filesystem to
> deal with as a mount option, not treated as a flag to directly
> change SB_IVERSION in the superblock.
> 
> We really need to stop with the "global mount options for everyone
> at the VFS" and instead pass everything down to the filesystems to
> parse appropriately. Yes, provide generic helper functions to deal
> with the common flags that everything supports, but the filesystems
> should be masking off mount options they doesn't support changing
> before changing their superblock feature support mask....

I think the MS_* flags are best saved for mount options that are applicable to
many/most filesystems and are mostly/entirely implementable at the VFS level.
I don't think "inlinecrypt" qualifies, since while it will be shared by the
block device-based filesystems that support fscrypt, that is only 2 filesystems
currently; and while some of the code needed to implement it is shared in
fs/crypto/, there are still substantial filesystem-specific hooks needed.

Hence this patchset intentionally does *not* allocate an MS_INLINECRYPT flag.

I believe that already addresses half of your concern, as it means
SB_INLINECRYPT can only be set/cleared by the filesystem itself, not by the VFS.
(But the commit message could use an explanation of this.)

The other half would be addressed by the following change, right?

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index faf9e5eaa029..954b132ae36b 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2362,6 +2362,9 @@ static int _ext4_show_options(struct seq_file *seq, struct super_block *sb,
 
 	fscrypt_show_test_dummy_encryption(seq, sep, sb);
 
+	if (sb->s_flags & SB_INLINECRYPT)
+		SEQ_OPTS_PUTS("inlinecrypt");
+
 	ext4_show_quota_options(seq, sb);
 	return 0;
 }
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 3621969b2665..23c49c313fb6 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1599,6 +1599,9 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
 
 	fscrypt_show_test_dummy_encryption(seq, ',', sbi->sb);
 
+	if (sbi->sb->s_flags & SB_INLINECRYPT)
+		seq_puts(seq, ",inlinecrypt");
+
 	if (F2FS_OPTION(sbi).alloc_mode == ALLOC_MODE_DEFAULT)
 		seq_printf(seq, ",alloc_mode=%s", "default");
 	else if (F2FS_OPTION(sbi).alloc_mode == ALLOC_MODE_REUSE)
diff --git a/fs/proc_namespace.c b/fs/proc_namespace.c
index e0ff1f6ac8f1..3059a9394c2d 100644
--- a/fs/proc_namespace.c
+++ b/fs/proc_namespace.c
@@ -49,7 +49,6 @@ static int show_sb_opts(struct seq_file *m, struct super_block *sb)
 		{ SB_DIRSYNC, ",dirsync" },
 		{ SB_MANDLOCK, ",mand" },
 		{ SB_LAZYTIME, ",lazytime" },
-		{ SB_INLINECRYPT, ",inlinecrypt" },
 		{ 0, NULL }
 	};
 	const struct proc_fs_opts *fs_infop;

Re: [f2fs-dev] [PATCH 1/4] fs: introduce SB_INLINECRYPT

[Dave Chinner]

On Mon, Jun 22, 2020 at 06:50:17PM -0700, Eric Biggers wrote:
> On Tue, Jun 23, 2020 at 10:46:36AM +1000, Dave Chinner wrote:
> > On Wed, Jun 17, 2020 at 08:19:35PM -0700, Eric Biggers wrote:
> > > Are you objecting to the use of a SB_* flag, or just to showing the flag in
> > > show_sb_opts() instead of in the individual filesystems?  Note that the SB_*
> > > flag was requested by Christoph
> > > (https://lkml.kernel.org/r/20191031183217.GF23601@infradead.org/,
> > > https://lkml.kernel.org/r/20191031212103.GA6244@infradead.org/).  We originally
> > > used a function fscrypt_operations::inline_crypt_enabled() instead.
> > 
> > I'm objecting to the layering violations of having the filesystem
> > control the mount option parsing and superblock feature flags, but
> > then having no control over whether features that the filesystem has
> > indicated to the VFS it is using get emitted as a mount option or
> > not, and then having the VFS code unconditionally override the
> > functionality that the filesystem uses because it thinks it's a
> > mount option the filesystem supports....
> > 
> > For example, the current mess that has just come to light:
> > filesystems like btrfs and XFS v5 which set SB_IVERSION
> > unconditionally (i.e. it's not a mount option!) end up having that
> > functionality turned off on remount because the VFS conflates
> > MS_IVERSION with SB_IVERSION and so unconditionally clears
> > SB_IVERSION because MS_IVERSION is not set on remount by userspace.
> > Which userspace will never set be because the filesystems don't put
> > "iversion" in their mount option strings because -its not a mount
> > option- for those filesystems.
> > 
> > See the problem?  MS_IVERSION should be passed to the filesystem to
> > deal with as a mount option, not treated as a flag to directly
> > change SB_IVERSION in the superblock.
> > 
> > We really need to stop with the "global mount options for everyone
> > at the VFS" and instead pass everything down to the filesystems to
> > parse appropriately. Yes, provide generic helper functions to deal
> > with the common flags that everything supports, but the filesystems
> > should be masking off mount options they doesn't support changing
> > before changing their superblock feature support mask....
> 
> I think the MS_* flags are best saved for mount options that are applicable to
> many/most filesystems and are mostly/entirely implementable at the VFS level.

That's the theory, but so far it's caused nothing but pain.

In reality, I think ithe only sane way forward if to stop mount
option parsing in userspace (i.e. no new MS_* flags) for any new
functionality as it only leads to future pain. i.e. all new mount
options should be parsed entirely in the kernel by the filesystem
parsing code....

> I don't think "inlinecrypt" qualifies, since while it will be shared by the
> block device-based filesystems that support fscrypt, that is only 2 filesystems
> currently; and while some of the code needed to implement it is shared in
> fs/crypto/, there are still substantial filesystem-specific hooks needed.

Right. I wasn't suggesting this patchset should use an MS_ flag -
it was pointing out the problem with the VFS code using SB_ flags to
indicate enabled filesystem functionality unconditionally as a mount
option that can be changed by userspace.

> Hence this patchset intentionally does *not* allocate an MS_INLINECRYPT flag.
> 
> I believe that already addresses half of your concern, as it means
> SB_INLINECRYPT can only be set/cleared by the filesystem itself, not by the VFS.
> (But the commit message could use an explanation of this.)
> 
> The other half would be addressed by the following change, right?

Yes, it does. Thanks, Eric!

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[PATCH 2/4] fscrypt: add inline encryption support

[Satya Tangirala]

Add support for inline encryption to fs/crypto/.  With "inline
encryption", the block layer handles the decryption/encryption as part
of the bio, instead of the filesystem doing the crypto itself via
Linux's crypto API.  This model is needed in order to take advantage of
the inline encryption hardware present on most modern mobile SoCs.

To use inline encryption, the filesystem needs to be mounted with
'-o inlinecrypt'.  The contents of any encrypted files will then be
encrypted using blk-crypto, instead of using the traditional
filesystem-layer crypto. Fscrypt still provides the key and IV to use,
and the actual ciphertext on-disk is still the same; therefore it's
testable using the existing fscrypt ciphertext verification tests.

Note that since blk-crypto has a fallback to Linux's crypto API, and
also supports all the encryption modes currently supported by fscrypt,
this feature is usable and testable even without actual inline
encryption hardware.

Per-filesystem changes will be needed to set encryption contexts when
submitting bios and to implement the 'inlinecrypt' mount option.  This
patch just adds the common code.

Co-developed-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Satya Tangirala <satyat@google.com>
---
 fs/crypto/Kconfig           |   6 +
 fs/crypto/Makefile          |   1 +
 fs/crypto/bio.c             |  50 ++++++
 fs/crypto/crypto.c          |   2 +-
 fs/crypto/fname.c           |   4 +-
 fs/crypto/fscrypt_private.h | 118 ++++++++++--
 fs/crypto/inline_crypt.c    | 349 ++++++++++++++++++++++++++++++++++++
 fs/crypto/keyring.c         |   6 +-
 fs/crypto/keysetup.c        |  68 ++++---
 fs/crypto/keysetup_v1.c     |  16 +-
 include/linux/fscrypt.h     |  82 +++++++++
 11 files changed, 655 insertions(+), 47 deletions(-)
 create mode 100644 fs/crypto/inline_crypt.c

diff --git a/fs/crypto/Kconfig b/fs/crypto/Kconfig
index 8046d7c7a3e9..f1f11a6228eb 100644
--- a/fs/crypto/Kconfig
+++ b/fs/crypto/Kconfig
@@ -24,3 +24,9 @@ config FS_ENCRYPTION_ALGS
 	select CRYPTO_SHA256
 	select CRYPTO_SHA512
 	select CRYPTO_XTS
+
+config FS_ENCRYPTION_INLINE_CRYPT
+	bool "Enable fscrypt to use inline crypto"
+	depends on FS_ENCRYPTION && BLK_INLINE_ENCRYPTION
+	help
+	  Enable fscrypt to use inline encryption hardware if available.
diff --git a/fs/crypto/Makefile b/fs/crypto/Makefile
index 232e2bb5a337..652c7180ec6d 100644
--- a/fs/crypto/Makefile
+++ b/fs/crypto/Makefile
@@ -11,3 +11,4 @@ fscrypto-y := crypto.o \
 	      policy.o
 
 fscrypto-$(CONFIG_BLOCK) += bio.o
+fscrypto-$(CONFIG_FS_ENCRYPTION_INLINE_CRYPT) += inline_crypt.o
diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c
index 4fa18fff9c4e..1ea9369a7688 100644
--- a/fs/crypto/bio.c
+++ b/fs/crypto/bio.c
@@ -41,6 +41,52 @@ void fscrypt_decrypt_bio(struct bio *bio)
 }
 EXPORT_SYMBOL(fscrypt_decrypt_bio);
 
+static int fscrypt_zeroout_range_inline_crypt(const struct inode *inode,
+					      pgoff_t lblk, sector_t pblk,
+					      unsigned int len)
+{
+	const unsigned int blockbits = inode->i_blkbits;
+	const unsigned int blocks_per_page = 1 << (PAGE_SHIFT - blockbits);
+	struct bio *bio;
+	int ret, err = 0;
+	int num_pages = 0;
+
+	/* This always succeeds since __GFP_DIRECT_RECLAIM is set. */
+	bio = bio_alloc(GFP_NOFS, BIO_MAX_PAGES);
+
+	while (len) {
+		unsigned int blocks_this_page = min(len, blocks_per_page);
+		unsigned int bytes_this_page = blocks_this_page << blockbits;
+
+		if (num_pages == 0) {
+			fscrypt_set_bio_crypt_ctx(bio, inode, lblk, GFP_NOFS);
+			bio_set_dev(bio, inode->i_sb->s_bdev);
+			bio->bi_iter.bi_sector =
+					pblk << (blockbits - SECTOR_SHIFT);
+			bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
+		}
+		ret = bio_add_page(bio, ZERO_PAGE(0), bytes_this_page, 0);
+		if (WARN_ON(ret != bytes_this_page)) {
+			err = -EIO;
+			goto out;
+		}
+		num_pages++;
+		len -= blocks_this_page;
+		lblk += blocks_this_page;
+		pblk += blocks_this_page;
+		if (num_pages == BIO_MAX_PAGES || !len) {
+			err = submit_bio_wait(bio);
+			if (err)
+				goto out;
+			bio_reset(bio);
+			num_pages = 0;
+		}
+	}
+out:
+	bio_put(bio);
+	return err;
+}
+
 /**
  * fscrypt_zeroout_range() - zero out a range of blocks in an encrypted file
  * @inode: the file's inode
@@ -75,6 +121,10 @@ int fscrypt_zeroout_range(const struct inode *inode, pgoff_t lblk,
 	if (len == 0)
 		return 0;
 
+	if (fscrypt_inode_uses_inline_crypto(inode))
+		return fscrypt_zeroout_range_inline_crypt(inode, lblk, pblk,
+							  len);
+
 	BUILD_BUG_ON(ARRAY_SIZE(pages) > BIO_MAX_PAGES);
 	nr_pages = min_t(unsigned int, ARRAY_SIZE(pages),
 			 (len + blocks_per_page - 1) >> blocks_per_page_bits);
diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index ed015cb66c7c..a52cf32733ab 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -100,7 +100,7 @@ int fscrypt_crypt_block(const struct inode *inode, fscrypt_direction_t rw,
 	DECLARE_CRYPTO_WAIT(wait);
 	struct scatterlist dst, src;
 	struct fscrypt_info *ci = inode->i_crypt_info;
-	struct crypto_skcipher *tfm = ci->ci_ctfm;
+	struct crypto_skcipher *tfm = ci->ci_enc_key.tfm;
 	int res = 0;
 
 	if (WARN_ON_ONCE(len <= 0))
diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index 83ca5f1e7934..d828e3df898b 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -115,7 +115,7 @@ int fscrypt_fname_encrypt(const struct inode *inode, const struct qstr *iname,
 	struct skcipher_request *req = NULL;
 	DECLARE_CRYPTO_WAIT(wait);
 	const struct fscrypt_info *ci = inode->i_crypt_info;
-	struct crypto_skcipher *tfm = ci->ci_ctfm;
+	struct crypto_skcipher *tfm = ci->ci_enc_key.tfm;
 	union fscrypt_iv iv;
 	struct scatterlist sg;
 	int res;
@@ -171,7 +171,7 @@ static int fname_decrypt(const struct inode *inode,
 	DECLARE_CRYPTO_WAIT(wait);
 	struct scatterlist src_sg, dst_sg;
 	const struct fscrypt_info *ci = inode->i_crypt_info;
-	struct crypto_skcipher *tfm = ci->ci_ctfm;
+	struct crypto_skcipher *tfm = ci->ci_enc_key.tfm;
 	union fscrypt_iv iv;
 	int res;
 
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index eb7fcd2b7fb8..1572186b0db4 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -14,6 +14,7 @@
 #include <linux/fscrypt.h>
 #include <linux/siphash.h>
 #include <crypto/hash.h>
+#include <linux/blk-crypto.h>
 
 #define CONST_STRLEN(str)	(sizeof(str) - 1)
 
@@ -166,6 +167,20 @@ struct fscrypt_symlink_data {
 	char encrypted_path[1];
 } __packed;
 
+/**
+ * struct fscrypt_prepared_key - a key prepared for actual encryption/decryption
+ * @tfm: crypto API transform object
+ * @blk_key: key for blk-crypto
+ *
+ * Normally only one of the fields will be non-NULL.
+ */
+struct fscrypt_prepared_key {
+	struct crypto_skcipher *tfm;
+#ifdef CONFIG_FS_ENCRYPTION_INLINE_CRYPT
+	struct fscrypt_blk_crypto_key *blk_key;
+#endif
+};
+
 /*
  * fscrypt_info - the "encryption key" for an inode
  *
@@ -175,12 +190,23 @@ struct fscrypt_symlink_data {
  */
 struct fscrypt_info {
 
-	/* The actual crypto transform used for encryption and decryption */
-	struct crypto_skcipher *ci_ctfm;
+	/* The key in a form prepared for actual encryption/decryption */
+	struct fscrypt_prepared_key	ci_enc_key;
 
-	/* True if the key should be freed when this fscrypt_info is freed */
+	/*
+	 * True if the ci_enc_key should be freed when this fscrypt_info is
+	 * freed
+	 */
 	bool ci_owns_key;
 
+#ifdef CONFIG_FS_ENCRYPTION_INLINE_CRYPT
+	/*
+	 * True if this inode will use inline encryption (blk-crypto) instead of
+	 * the traditional filesystem-layer encryption.
+	 */
+	bool ci_inlinecrypt;
+#endif
+
 	/*
 	 * Encryption mode used for this inode.  It corresponds to either the
 	 * contents or filenames encryption mode, depending on the inode type.
@@ -205,7 +231,7 @@ struct fscrypt_info {
 
 	/*
 	 * If non-NULL, then encryption is done using the master key directly
-	 * and ci_ctfm will equal ci_direct_key->dk_ctfm.
+	 * and ci_enc_key will equal ci_direct_key->dk_key.
 	 */
 	struct fscrypt_direct_key *ci_direct_key;
 
@@ -260,6 +286,7 @@ union fscrypt_iv {
 		u8 nonce[FS_KEY_DERIVATION_NONCE_SIZE];
 	};
 	u8 raw[FSCRYPT_MAX_IV_SIZE];
+	__le64 dun[FSCRYPT_MAX_IV_SIZE / sizeof(__le64)];
 };
 
 void fscrypt_generate_iv(union fscrypt_iv *iv, u64 lblk_num,
@@ -302,6 +329,74 @@ int fscrypt_hkdf_expand(const struct fscrypt_hkdf *hkdf, u8 context,
 
 void fscrypt_destroy_hkdf(struct fscrypt_hkdf *hkdf);
 
+/* inline_crypt.c */
+#ifdef CONFIG_FS_ENCRYPTION_INLINE_CRYPT
+void fscrypt_select_encryption_impl(struct fscrypt_info *ci);
+
+static inline bool
+fscrypt_using_inline_encryption(const struct fscrypt_info *ci)
+{
+	return ci->ci_inlinecrypt;
+}
+
+int fscrypt_prepare_inline_crypt_key(struct fscrypt_prepared_key *prep_key,
+				     const u8 *raw_key,
+				     const struct fscrypt_info *ci);
+
+void fscrypt_destroy_inline_crypt_key(struct fscrypt_prepared_key *prep_key);
+
+/*
+ * Check whether the crypto transform or blk-crypto key has been allocated in
+ * @prep_key, depending on which encryption implementation the file will use.
+ */
+static inline bool
+fscrypt_is_key_prepared(struct fscrypt_prepared_key *prep_key,
+			const struct fscrypt_info *ci)
+{
+	/*
+	 * The READ_ONCE() here pairs with the smp_store_release() in
+	 * fscrypt_prepare_key().  (This only matters for the per-mode keys,
+	 * which are shared by multiple inodes.)
+	 */
+	if (fscrypt_using_inline_encryption(ci))
+		return READ_ONCE(prep_key->blk_key) != NULL;
+	return READ_ONCE(prep_key->tfm) != NULL;
+}
+
+#else /* CONFIG_FS_ENCRYPTION_INLINE_CRYPT */
+
+static inline void fscrypt_select_encryption_impl(struct fscrypt_info *ci)
+{
+}
+
+static inline bool fscrypt_using_inline_encryption(
+					const struct fscrypt_info *ci)
+{
+	return false;
+}
+
+static inline int
+fscrypt_prepare_inline_crypt_key(struct fscrypt_prepared_key *prep_key,
+				 const u8 *raw_key,
+				 const struct fscrypt_info *ci)
+{
+	WARN_ON(1);
+	return -EOPNOTSUPP;
+}
+
+static inline void
+fscrypt_destroy_inline_crypt_key(struct fscrypt_prepared_key *prep_key)
+{
+}
+
+static inline bool
+fscrypt_is_key_prepared(struct fscrypt_prepared_key *prep_key,
+			const struct fscrypt_info *ci)
+{
+	return READ_ONCE(prep_key->tfm) != NULL;
+}
+#endif /* !CONFIG_FS_ENCRYPTION_INLINE_CRYPT */
+
 /* keyring.c */
 
 /*
@@ -395,9 +490,9 @@ struct fscrypt_master_key {
 	 * Per-mode encryption keys for the various types of encryption policies
 	 * that use them.  Allocated and derived on-demand.
 	 */
-	struct crypto_skcipher *mk_direct_keys[__FSCRYPT_MODE_MAX + 1];
-	struct crypto_skcipher *mk_iv_ino_lblk_64_keys[__FSCRYPT_MODE_MAX + 1];
-	struct crypto_skcipher *mk_iv_ino_lblk_32_keys[__FSCRYPT_MODE_MAX + 1];
+	struct fscrypt_prepared_key mk_direct_keys[__FSCRYPT_MODE_MAX + 1];
+	struct fscrypt_prepared_key mk_iv_ino_lblk_64_keys[__FSCRYPT_MODE_MAX + 1];
+	struct fscrypt_prepared_key mk_iv_ino_lblk_32_keys[__FSCRYPT_MODE_MAX + 1];
 
 	/* Hash key for inode numbers.  Initialized only when needed. */
 	siphash_key_t		mk_ino_hash_key;
@@ -461,13 +556,16 @@ struct fscrypt_mode {
 	int keysize;
 	int ivsize;
 	int logged_impl_name;
+	enum blk_crypto_mode_num blk_crypto_mode;
 };
 
 extern struct fscrypt_mode fscrypt_modes[];
 
-struct crypto_skcipher *fscrypt_allocate_skcipher(struct fscrypt_mode *mode,
-						  const u8 *raw_key,
-						  const struct inode *inode);
+int fscrypt_prepare_key(struct fscrypt_prepared_key *prep_key,
+			const u8 *raw_key,
+			const struct fscrypt_info *ci);
+
+void fscrypt_destroy_prepared_key(struct fscrypt_prepared_key *prep_key);
 
 int fscrypt_set_per_file_enc_key(struct fscrypt_info *ci, const u8 *raw_key);
 
diff --git a/fs/crypto/inline_crypt.c b/fs/crypto/inline_crypt.c
new file mode 100644
index 000000000000..7dbeb49260a6
--- /dev/null
+++ b/fs/crypto/inline_crypt.c
@@ -0,0 +1,349 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Inline encryption support for fscrypt
+ *
+ * Copyright 2019 Google LLC
+ */
+
+/*
+ * With "inline encryption", the block layer handles the decryption/encryption
+ * as part of the bio, instead of the filesystem doing the crypto itself via
+ * crypto API.  See Documentation/block/inline-encryption.rst.  fscrypt still
+ * provides the key and IV to use.
+ */
+
+#include <linux/blk-crypto.h>
+#include <linux/blkdev.h>
+#include <linux/buffer_head.h>
+#include <linux/sched/mm.h>
+
+#include "fscrypt_private.h"
+
+struct fscrypt_blk_crypto_key {
+	struct blk_crypto_key base;
+	int num_devs;
+	struct request_queue *devs[];
+};
+
+static int fscrypt_get_num_devices(struct super_block *sb)
+{
+	if (sb->s_cop->get_num_devices)
+		return sb->s_cop->get_num_devices(sb);
+	return 1;
+}
+
+static void fscrypt_get_devices(struct super_block *sb, int num_devs,
+				struct request_queue **devs)
+{
+	if (num_devs == 1)
+		devs[0] = bdev_get_queue(sb->s_bdev);
+	else
+		sb->s_cop->get_devices(sb, devs);
+}
+
+static unsigned int fscrypt_get_dun_bytes(const struct fscrypt_info *ci)
+{
+	struct super_block *sb = ci->ci_inode->i_sb;
+	unsigned int flags = fscrypt_policy_flags(&ci->ci_policy);
+	int ino_bits = 64, lblk_bits = 64;
+
+	if (flags & FSCRYPT_POLICY_FLAG_DIRECT_KEY)
+		return offsetofend(union fscrypt_iv, nonce);
+
+	if (flags & FSCRYPT_POLICY_FLAG_IV_INO_LBLK_64)
+		return sizeof(__le64);
+
+	if (flags & FSCRYPT_POLICY_FLAG_IV_INO_LBLK_32)
+		return sizeof(__le32);
+
+	/* Default case: IVs are just the file logical block number */
+	if (sb->s_cop->get_ino_and_lblk_bits)
+		sb->s_cop->get_ino_and_lblk_bits(sb, &ino_bits, &lblk_bits);
+	return DIV_ROUND_UP(lblk_bits, 8);
+}
+
+/* Enable inline encryption for this file if supported. */
+void fscrypt_select_encryption_impl(struct fscrypt_info *ci)
+{
+	const struct inode *inode = ci->ci_inode;
+	struct super_block *sb = inode->i_sb;
+	struct blk_crypto_config crypto_cfg;
+	int num_devs;
+	struct request_queue **devs;
+	int i;
+
+	/* The file must need contents encryption, not filenames encryption */
+	if (!fscrypt_needs_contents_encryption(inode))
+		return;
+
+	/* The crypto mode must be valid */
+	if (ci->ci_mode->blk_crypto_mode == BLK_ENCRYPTION_MODE_INVALID)
+		return;
+
+	/* The filesystem must be mounted with -o inlinecrypt */
+	if (!(sb->s_flags & SB_INLINECRYPT))
+		return;
+
+	/*
+	 * blk-crypto must support the crypto configuration we'll use for the
+	 * inode on all devices in the sb
+	 */
+	crypto_cfg.crypto_mode = ci->ci_mode->blk_crypto_mode;
+	crypto_cfg.data_unit_size = sb->s_blocksize;
+	crypto_cfg.dun_bytes = fscrypt_get_dun_bytes(ci);
+	num_devs = fscrypt_get_num_devices(sb);
+	devs = kmalloc_array(num_devs, sizeof(*devs), GFP_NOFS);
+	if (!devs)
+		return;
+	fscrypt_get_devices(sb, num_devs, devs);
+
+	for (i = 0; i < num_devs; i++) {
+		if (!blk_crypto_config_supported(devs[i], &crypto_cfg))
+			goto out_free_devs;
+	}
+
+	ci->ci_inlinecrypt = true;
+out_free_devs:
+	kfree(devs);
+}
+
+int fscrypt_prepare_inline_crypt_key(struct fscrypt_prepared_key *prep_key,
+				     const u8 *raw_key,
+				     const struct fscrypt_info *ci)
+{
+	const struct inode *inode = ci->ci_inode;
+	struct super_block *sb = inode->i_sb;
+	enum blk_crypto_mode_num crypto_mode = ci->ci_mode->blk_crypto_mode;
+	int num_devs = fscrypt_get_num_devices(sb);
+	int queue_refs = 0;
+	struct fscrypt_blk_crypto_key *blk_key;
+	int err;
+	int i;
+	unsigned int flags;
+
+	blk_key = kzalloc(struct_size(blk_key, devs, num_devs), GFP_NOFS);
+	if (!blk_key)
+		return -ENOMEM;
+
+	blk_key->num_devs = num_devs;
+	fscrypt_get_devices(sb, num_devs, blk_key->devs);
+
+	err = blk_crypto_init_key(&blk_key->base, raw_key, crypto_mode,
+				  fscrypt_get_dun_bytes(ci), sb->s_blocksize);
+	if (err) {
+		fscrypt_err(inode, "error %d initializing blk-crypto key", err);
+		goto fail;
+	}
+
+	/*
+	 * We have to start using blk-crypto on all the filesystem's devices.
+	 * We also have to save all the request_queue's for later so that the
+	 * key can be evicted from them.  This is needed because some keys
+	 * aren't destroyed until after the filesystem was already unmounted
+	 * (namely, the per-mode keys in struct fscrypt_master_key).
+	 */
+	for (i = 0; i < num_devs; i++) {
+		if (!blk_get_queue(blk_key->devs[i])) {
+			fscrypt_err(inode, "couldn't get request_queue");
+			err = -EAGAIN;
+			goto fail;
+		}
+		queue_refs++;
+
+		flags = memalloc_nofs_save();
+		err = blk_crypto_start_using_key(&blk_key->base,
+						 blk_key->devs[i]);
+		memalloc_nofs_restore(flags);
+		if (err) {
+			fscrypt_err(inode,
+				    "error %d starting to use blk-crypto", err);
+			goto fail;
+		}
+	}
+	/*
+	 * Pairs with READ_ONCE() in fscrypt_is_key_prepared().  (Only matters
+	 * for the per-mode keys, which are shared by multiple inodes.)
+	 */
+	smp_store_release(&prep_key->blk_key, blk_key);
+	return 0;
+
+fail:
+	for (i = 0; i < queue_refs; i++)
+		blk_put_queue(blk_key->devs[i]);
+	kzfree(blk_key);
+	return err;
+}
+
+void fscrypt_destroy_inline_crypt_key(struct fscrypt_prepared_key *prep_key)
+{
+	struct fscrypt_blk_crypto_key *blk_key = prep_key->blk_key;
+	int i;
+
+	if (blk_key) {
+		for (i = 0; i < blk_key->num_devs; i++) {
+			blk_crypto_evict_key(blk_key->devs[i], &blk_key->base);
+			blk_put_queue(blk_key->devs[i]);
+		}
+		kzfree(blk_key);
+	}
+}
+
+bool __fscrypt_inode_uses_inline_crypto(const struct inode *inode)
+{
+	return inode->i_crypt_info->ci_inlinecrypt;
+}
+EXPORT_SYMBOL_GPL(__fscrypt_inode_uses_inline_crypto);
+
+static void fscrypt_generate_dun(const struct fscrypt_info *ci, u64 lblk_num,
+				 u64 dun[BLK_CRYPTO_DUN_ARRAY_SIZE])
+{
+	union fscrypt_iv iv;
+	int i;
+
+	fscrypt_generate_iv(&iv, lblk_num, ci);
+
+	BUILD_BUG_ON(FSCRYPT_MAX_IV_SIZE > BLK_CRYPTO_MAX_IV_SIZE);
+	memset(dun, 0, BLK_CRYPTO_MAX_IV_SIZE);
+	for (i = 0; i < ci->ci_mode->ivsize/sizeof(dun[0]); i++)
+		dun[i] = le64_to_cpu(iv.dun[i]);
+}
+
+/**
+ * fscrypt_set_bio_crypt_ctx() - prepare a file contents bio for inline crypto
+ * @bio: a bio which will eventually be submitted to the file
+ * @inode: the file's inode
+ * @first_lblk: the first file logical block number in the I/O
+ * @gfp_mask: memory allocation flags - these must be a waiting mask so that
+ *					bio_crypt_set_ctx can't fail.
+ *
+ * If the contents of the file should be encrypted (or decrypted) with inline
+ * encryption, then assign the appropriate encryption context to the bio.
+ *
+ * Normally the bio should be newly allocated (i.e. no pages added yet), as
+ * otherwise fscrypt_mergeable_bio() won't work as intended.
+ *
+ * The encryption context will be freed automatically when the bio is freed.
+ */
+void fscrypt_set_bio_crypt_ctx(struct bio *bio, const struct inode *inode,
+			       u64 first_lblk, gfp_t gfp_mask)
+{
+	const struct fscrypt_info *ci = inode->i_crypt_info;
+	u64 dun[BLK_CRYPTO_DUN_ARRAY_SIZE];
+
+	if (!fscrypt_inode_uses_inline_crypto(inode))
+		return;
+
+	fscrypt_generate_dun(ci, first_lblk, dun);
+	bio_crypt_set_ctx(bio, &ci->ci_enc_key.blk_key->base, dun, gfp_mask);
+}
+EXPORT_SYMBOL_GPL(fscrypt_set_bio_crypt_ctx);
+
+/* Extract the inode and logical block number from a buffer_head. */
+static bool bh_get_inode_and_lblk_num(const struct buffer_head *bh,
+				      const struct inode **inode_ret,
+				      u64 *lblk_num_ret)
+{
+	struct page *page = bh->b_page;
+	const struct address_space *mapping;
+	const struct inode *inode;
+
+	/*
+	 * The ext4 journal (jbd2) can submit a buffer_head it directly created
+	 * for a non-pagecache page.  fscrypt doesn't care about these.
+	 */
+	mapping = page_mapping(page);
+	if (!mapping)
+		return false;
+	inode = mapping->host;
+
+	*inode_ret = inode;
+	*lblk_num_ret = ((u64)page->index << (PAGE_SHIFT - inode->i_blkbits)) +
+			(bh_offset(bh) >> inode->i_blkbits);
+	return true;
+}
+
+/**
+ * fscrypt_set_bio_crypt_ctx_bh() - prepare a file contents bio for inline
+ *				    crypto
+ * @bio: a bio which will eventually be submitted to the file
+ * @first_bh: the first buffer_head for which I/O will be submitted
+ * @gfp_mask: memory allocation flags
+ *
+ * Same as fscrypt_set_bio_crypt_ctx(), except this takes a buffer_head instead
+ * of an inode and block number directly.
+ */
+void fscrypt_set_bio_crypt_ctx_bh(struct bio *bio,
+				  const struct buffer_head *first_bh,
+				  gfp_t gfp_mask)
+{
+	const struct inode *inode;
+	u64 first_lblk;
+
+	if (bh_get_inode_and_lblk_num(first_bh, &inode, &first_lblk))
+		fscrypt_set_bio_crypt_ctx(bio, inode, first_lblk, gfp_mask);
+}
+EXPORT_SYMBOL_GPL(fscrypt_set_bio_crypt_ctx_bh);
+
+/**
+ * fscrypt_mergeable_bio() - test whether data can be added to a bio
+ * @bio: the bio being built up
+ * @inode: the inode for the next part of the I/O
+ * @next_lblk: the next file logical block number in the I/O
+ *
+ * When building a bio which may contain data which should undergo inline
+ * encryption (or decryption) via fscrypt, filesystems should call this function
+ * to ensure that the resulting bio contains only logically contiguous data.
+ * This will return false if the next part of the I/O cannot be merged with the
+ * bio because either the encryption key would be different or the encryption
+ * data unit numbers would be discontiguous.
+ *
+ * fscrypt_set_bio_crypt_ctx() must have already been called on the bio.
+ *
+ * Return: true iff the I/O is mergeable
+ */
+bool fscrypt_mergeable_bio(struct bio *bio, const struct inode *inode,
+			   u64 next_lblk)
+{
+	const struct bio_crypt_ctx *bc = bio->bi_crypt_context;
+	u64 next_dun[BLK_CRYPTO_DUN_ARRAY_SIZE];
+
+	if (!!bc != fscrypt_inode_uses_inline_crypto(inode))
+		return false;
+	if (!bc)
+		return true;
+
+	/*
+	 * Comparing the key pointers is good enough, as all I/O for each key
+	 * uses the same pointer.  I.e., there's currently no need to support
+	 * merging requests where the keys are the same but the pointers differ.
+	 */
+	if (bc->bc_key != &inode->i_crypt_info->ci_enc_key.blk_key->base)
+		return false;
+
+	fscrypt_generate_dun(inode->i_crypt_info, next_lblk, next_dun);
+	return bio_crypt_dun_is_contiguous(bc, bio->bi_iter.bi_size, next_dun);
+}
+EXPORT_SYMBOL_GPL(fscrypt_mergeable_bio);
+
+/**
+ * fscrypt_mergeable_bio_bh() - test whether data can be added to a bio
+ * @bio: the bio being built up
+ * @next_bh: the next buffer_head for which I/O will be submitted
+ *
+ * Same as fscrypt_mergeable_bio(), except this takes a buffer_head instead of
+ * an inode and block number directly.
+ *
+ * Return: true iff the I/O is mergeable
+ */
+bool fscrypt_mergeable_bio_bh(struct bio *bio,
+			      const struct buffer_head *next_bh)
+{
+	const struct inode *inode;
+	u64 next_lblk;
+
+	if (!bh_get_inode_and_lblk_num(next_bh, &inode, &next_lblk))
+		return !bio->bi_crypt_context;
+
+	return fscrypt_mergeable_bio(bio, inode, next_lblk);
+}
+EXPORT_SYMBOL_GPL(fscrypt_mergeable_bio_bh);
diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
index e24eb48bfbe1..7f8ac61a20d6 100644
--- a/fs/crypto/keyring.c
+++ b/fs/crypto/keyring.c
@@ -45,9 +45,9 @@ static void free_master_key(struct fscrypt_master_key *mk)
 	wipe_master_key_secret(&mk->mk_secret);
 
 	for (i = 0; i <= __FSCRYPT_MODE_MAX; i++) {
-		crypto_free_skcipher(mk->mk_direct_keys[i]);
-		crypto_free_skcipher(mk->mk_iv_ino_lblk_64_keys[i]);
-		crypto_free_skcipher(mk->mk_iv_ino_lblk_32_keys[i]);
+		fscrypt_destroy_prepared_key(&mk->mk_direct_keys[i]);
+		fscrypt_destroy_prepared_key(&mk->mk_iv_ino_lblk_64_keys[i]);
+		fscrypt_destroy_prepared_key(&mk->mk_iv_ino_lblk_32_keys[i]);
 	}
 
 	key_put(mk->mk_users);
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 1129adfa097d..c789ac30af7c 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -19,6 +19,7 @@ struct fscrypt_mode fscrypt_modes[] = {
 		.cipher_str = "xts(aes)",
 		.keysize = 64,
 		.ivsize = 16,
+		.blk_crypto_mode = BLK_ENCRYPTION_MODE_AES_256_XTS,
 	},
 	[FSCRYPT_MODE_AES_256_CTS] = {
 		.friendly_name = "AES-256-CTS-CBC",
@@ -31,6 +32,7 @@ struct fscrypt_mode fscrypt_modes[] = {
 		.cipher_str = "essiv(cbc(aes),sha256)",
 		.keysize = 16,
 		.ivsize = 16,
+		.blk_crypto_mode = BLK_ENCRYPTION_MODE_AES_128_CBC_ESSIV,
 	},
 	[FSCRYPT_MODE_AES_128_CTS] = {
 		.friendly_name = "AES-128-CTS-CBC",
@@ -43,6 +45,7 @@ struct fscrypt_mode fscrypt_modes[] = {
 		.cipher_str = "adiantum(xchacha12,aes)",
 		.keysize = 32,
 		.ivsize = 32,
+		.blk_crypto_mode = BLK_ENCRYPTION_MODE_ADIANTUM,
 	},
 };
 
@@ -64,9 +67,9 @@ select_encryption_mode(const union fscrypt_policy *policy,
 }
 
 /* Create a symmetric cipher object for the given encryption mode and key */
-struct crypto_skcipher *fscrypt_allocate_skcipher(struct fscrypt_mode *mode,
-						  const u8 *raw_key,
-						  const struct inode *inode)
+static struct crypto_skcipher *
+fscrypt_allocate_skcipher(struct fscrypt_mode *mode, const u8 *raw_key,
+			  const struct inode *inode)
 {
 	struct crypto_skcipher *tfm;
 	int err;
@@ -109,30 +112,54 @@ struct crypto_skcipher *fscrypt_allocate_skcipher(struct fscrypt_mode *mode,
 	return ERR_PTR(err);
 }
 
-/* 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)
+/*
+ * Prepare the crypto transform object or blk-crypto key in @prep_key, given the
+ * raw key, encryption mode, and flag indicating which encryption implementation
+ * (fs-layer or blk-crypto) will be used.
+ */
+int fscrypt_prepare_key(struct fscrypt_prepared_key *prep_key,
+			const u8 *raw_key, const struct fscrypt_info *ci)
 {
 	struct crypto_skcipher *tfm;
 
+	if (fscrypt_using_inline_encryption(ci))
+		return fscrypt_prepare_inline_crypt_key(prep_key, raw_key, ci);
+
 	tfm = fscrypt_allocate_skcipher(ci->ci_mode, raw_key, ci->ci_inode);
 	if (IS_ERR(tfm))
 		return PTR_ERR(tfm);
+	/*
+	 * Pairs with READ_ONCE() in fscrypt_is_key_prepared().  (Only matters
+	 * for the per-mode keys, which are shared by multiple inodes.)
+	 */
+	smp_store_release(&prep_key->tfm, tfm);
+	return 0;
+}
+
+/* Destroy a crypto transform object and/or blk-crypto key. */
+void fscrypt_destroy_prepared_key(struct fscrypt_prepared_key *prep_key)
+{
+	crypto_free_skcipher(prep_key->tfm);
+	fscrypt_destroy_inline_crypt_key(prep_key);
+}
 
-	ci->ci_ctfm = tfm;
+/* 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;
-	return 0;
+	return fscrypt_prepare_key(&ci->ci_enc_key, raw_key, ci);
 }
 
 static int setup_per_mode_enc_key(struct fscrypt_info *ci,
 				  struct fscrypt_master_key *mk,
-				  struct crypto_skcipher **tfms,
+				  struct fscrypt_prepared_key *keys,
 				  u8 hkdf_context, bool include_fs_uuid)
 {
 	const struct inode *inode = ci->ci_inode;
 	const struct super_block *sb = inode->i_sb;
 	struct fscrypt_mode *mode = ci->ci_mode;
 	const u8 mode_num = mode - fscrypt_modes;
-	struct crypto_skcipher *tfm;
+	struct fscrypt_prepared_key *prep_key;
 	u8 mode_key[FSCRYPT_MAX_KEY_SIZE];
 	u8 hkdf_info[sizeof(mode_num) + sizeof(sb->s_uuid)];
 	unsigned int hkdf_infolen = 0;
@@ -141,16 +168,15 @@ static int setup_per_mode_enc_key(struct fscrypt_info *ci,
 	if (WARN_ON(mode_num > __FSCRYPT_MODE_MAX))
 		return -EINVAL;
 
-	/* pairs with smp_store_release() below */
-	tfm = READ_ONCE(tfms[mode_num]);
-	if (likely(tfm != NULL)) {
-		ci->ci_ctfm = tfm;
+	prep_key = &keys[mode_num];
+	if (fscrypt_is_key_prepared(prep_key, ci)) {
+		ci->ci_enc_key = *prep_key;
 		return 0;
 	}
 
 	mutex_lock(&fscrypt_mode_key_setup_mutex);
 
-	if (tfms[mode_num])
+	if (fscrypt_is_key_prepared(prep_key, ci))
 		goto done_unlock;
 
 	BUILD_BUG_ON(sizeof(mode_num) != 1);
@@ -167,16 +193,12 @@ static int setup_per_mode_enc_key(struct fscrypt_info *ci,
 				  mode_key, mode->keysize);
 	if (err)
 		goto out_unlock;
-	tfm = fscrypt_allocate_skcipher(mode, mode_key, inode);
+	err = fscrypt_prepare_key(prep_key, mode_key, ci);
 	memzero_explicit(mode_key, mode->keysize);
-	if (IS_ERR(tfm)) {
-		err = PTR_ERR(tfm);
+	if (err)
 		goto out_unlock;
-	}
-	/* pairs with READ_ONCE() above */
-	smp_store_release(&tfms[mode_num], tfm);
 done_unlock:
-	ci->ci_ctfm = tfm;
+	ci->ci_enc_key = *prep_key;
 	err = 0;
 out_unlock:
 	mutex_unlock(&fscrypt_mode_key_setup_mutex);
@@ -310,6 +332,8 @@ static int setup_file_encryption_key(struct fscrypt_info *ci,
 	struct fscrypt_key_specifier mk_spec;
 	int err;
 
+	fscrypt_select_encryption_impl(ci);
+
 	switch (ci->ci_policy.version) {
 	case FSCRYPT_POLICY_V1:
 		mk_spec.type = FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR;
@@ -402,7 +426,7 @@ static void put_crypt_info(struct fscrypt_info *ci)
 	if (ci->ci_direct_key)
 		fscrypt_put_direct_key(ci->ci_direct_key);
 	else if (ci->ci_owns_key)
-		crypto_free_skcipher(ci->ci_ctfm);
+		fscrypt_destroy_prepared_key(&ci->ci_enc_key);
 
 	key = ci->ci_master_key;
 	if (key) {
diff --git a/fs/crypto/keysetup_v1.c b/fs/crypto/keysetup_v1.c
index 801b48c0cd7f..a52686729a67 100644
--- a/fs/crypto/keysetup_v1.c
+++ b/fs/crypto/keysetup_v1.c
@@ -146,7 +146,7 @@ struct fscrypt_direct_key {
 	struct hlist_node		dk_node;
 	refcount_t			dk_refcount;
 	const struct fscrypt_mode	*dk_mode;
-	struct crypto_skcipher		*dk_ctfm;
+	struct fscrypt_prepared_key	dk_key;
 	u8				dk_descriptor[FSCRYPT_KEY_DESCRIPTOR_SIZE];
 	u8				dk_raw[FSCRYPT_MAX_KEY_SIZE];
 };
@@ -154,7 +154,7 @@ struct fscrypt_direct_key {
 static void free_direct_key(struct fscrypt_direct_key *dk)
 {
 	if (dk) {
-		crypto_free_skcipher(dk->dk_ctfm);
+		fscrypt_destroy_prepared_key(&dk->dk_key);
 		kzfree(dk);
 	}
 }
@@ -199,6 +199,8 @@ find_or_insert_direct_key(struct fscrypt_direct_key *to_insert,
 			continue;
 		if (ci->ci_mode != dk->dk_mode)
 			continue;
+		if (!fscrypt_is_key_prepared(&dk->dk_key, ci))
+			continue;
 		if (crypto_memneq(raw_key, dk->dk_raw, ci->ci_mode->keysize))
 			continue;
 		/* using existing tfm with same (descriptor, mode, raw_key) */
@@ -231,13 +233,9 @@ fscrypt_get_direct_key(const struct fscrypt_info *ci, const u8 *raw_key)
 		return ERR_PTR(-ENOMEM);
 	refcount_set(&dk->dk_refcount, 1);
 	dk->dk_mode = ci->ci_mode;
-	dk->dk_ctfm = fscrypt_allocate_skcipher(ci->ci_mode, raw_key,
-						ci->ci_inode);
-	if (IS_ERR(dk->dk_ctfm)) {
-		err = PTR_ERR(dk->dk_ctfm);
-		dk->dk_ctfm = NULL;
+	err = fscrypt_prepare_key(&dk->dk_key, raw_key, ci);
+	if (err)
 		goto err_free_dk;
-	}
 	memcpy(dk->dk_descriptor, ci->ci_policy.v1.master_key_descriptor,
 	       FSCRYPT_KEY_DESCRIPTOR_SIZE);
 	memcpy(dk->dk_raw, raw_key, ci->ci_mode->keysize);
@@ -259,7 +257,7 @@ static int setup_v1_file_key_direct(struct fscrypt_info *ci,
 	if (IS_ERR(dk))
 		return PTR_ERR(dk);
 	ci->ci_direct_key = dk;
-	ci->ci_ctfm = dk->dk_ctfm;
+	ci->ci_enc_key = dk->dk_key;
 	return 0;
 }
 
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 2862ca5fea33..bb257411365f 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -69,6 +69,9 @@ struct fscrypt_operations {
 	bool (*has_stable_inodes)(struct super_block *sb);
 	void (*get_ino_and_lblk_bits)(struct super_block *sb,
 				      int *ino_bits_ret, int *lblk_bits_ret);
+	int (*get_num_devices)(struct super_block *sb);
+	void (*get_devices)(struct super_block *sb,
+			    struct request_queue **devs);
 };
 
 static inline bool fscrypt_has_encryption_key(const struct inode *inode)
@@ -537,6 +540,85 @@ static inline void fscrypt_set_ops(struct super_block *sb,
 
 #endif	/* !CONFIG_FS_ENCRYPTION */
 
+/* inline_crypt.c */
+#ifdef CONFIG_FS_ENCRYPTION_INLINE_CRYPT
+
+bool __fscrypt_inode_uses_inline_crypto(const struct inode *inode);
+
+void fscrypt_set_bio_crypt_ctx(struct bio *bio,
+			       const struct inode *inode, u64 first_lblk,
+			       gfp_t gfp_mask);
+
+void fscrypt_set_bio_crypt_ctx_bh(struct bio *bio,
+				  const struct buffer_head *first_bh,
+				  gfp_t gfp_mask);
+
+bool fscrypt_mergeable_bio(struct bio *bio, const struct inode *inode,
+			   u64 next_lblk);
+
+bool fscrypt_mergeable_bio_bh(struct bio *bio,
+			      const struct buffer_head *next_bh);
+
+#else /* CONFIG_FS_ENCRYPTION_INLINE_CRYPT */
+
+static inline bool __fscrypt_inode_uses_inline_crypto(const struct inode *inode)
+{
+	return false;
+}
+
+static inline void fscrypt_set_bio_crypt_ctx(struct bio *bio,
+					     const struct inode *inode,
+					     u64 first_lblk, gfp_t gfp_mask) { }
+
+static inline void fscrypt_set_bio_crypt_ctx_bh(
+					 struct bio *bio,
+					 const struct buffer_head *first_bh,
+					 gfp_t gfp_mask) { }
+
+static inline bool fscrypt_mergeable_bio(struct bio *bio,
+					 const struct inode *inode,
+					 u64 next_lblk)
+{
+	return true;
+}
+
+static inline bool fscrypt_mergeable_bio_bh(struct bio *bio,
+					    const struct buffer_head *next_bh)
+{
+	return true;
+}
+#endif /* !CONFIG_FS_ENCRYPTION_INLINE_CRYPT */
+
+/**
+ * fscrypt_inode_uses_inline_crypto() - test whether an inode uses inline
+ *					encryption
+ * @inode: an inode. If encrypted, its key must be set up.
+ *
+ * Return: true if the inode requires file contents encryption and if the
+ *	   encryption should be done in the block layer via blk-crypto rather
+ *	   than in the filesystem layer.
+ */
+static inline bool fscrypt_inode_uses_inline_crypto(const struct inode *inode)
+{
+	return fscrypt_needs_contents_encryption(inode) &&
+	       __fscrypt_inode_uses_inline_crypto(inode);
+}
+
+/**
+ * fscrypt_inode_uses_fs_layer_crypto() - test whether an inode uses fs-layer
+ *					  encryption
+ * @inode: an inode. If encrypted, its key must be set up.
+ *
+ * Return: true if the inode requires file contents encryption and if the
+ *	   encryption should be done in the filesystem layer rather than in the
+ *	   block layer via blk-crypto.
+ */
+static inline bool fscrypt_inode_uses_fs_layer_crypto(const struct inode *inode)
+{
+	return fscrypt_needs_contents_encryption(inode) &&
+	       !__fscrypt_inode_uses_inline_crypto(inode);
+}
+
 /**
  * fscrypt_require_key() - require an inode's encryption key
  * @inode: the inode we need the key for
-- 
2.27.0.290.gba653c62da-goog

Re: [PATCH 2/4] fscrypt: add inline encryption support

[Jaegeuk Kim]

On 06/17, Satya Tangirala wrote:
> Add support for inline encryption to fs/crypto/.  With "inline
> encryption", the block layer handles the decryption/encryption as part
> of the bio, instead of the filesystem doing the crypto itself via
> Linux's crypto API.  This model is needed in order to take advantage of
> the inline encryption hardware present on most modern mobile SoCs.
> 
> To use inline encryption, the filesystem needs to be mounted with
> '-o inlinecrypt'.  The contents of any encrypted files will then be
> encrypted using blk-crypto, instead of using the traditional
> filesystem-layer crypto. Fscrypt still provides the key and IV to use,
> and the actual ciphertext on-disk is still the same; therefore it's
> testable using the existing fscrypt ciphertext verification tests.
> 
> Note that since blk-crypto has a fallback to Linux's crypto API, and
> also supports all the encryption modes currently supported by fscrypt,
> this feature is usable and testable even without actual inline
> encryption hardware.
> 
> Per-filesystem changes will be needed to set encryption contexts when
> submitting bios and to implement the 'inlinecrypt' mount option.  This
> patch just adds the common code.
> 
> Co-developed-by: Eric Biggers <ebiggers@google.com>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> Signed-off-by: Satya Tangirala <satyat@google.com>

Reviewed-by: Jaegeuk Kim <jaegeuk@kernel.org>

> ---
>  fs/crypto/Kconfig           |   6 +
>  fs/crypto/Makefile          |   1 +
>  fs/crypto/bio.c             |  50 ++++++
>  fs/crypto/crypto.c          |   2 +-
>  fs/crypto/fname.c           |   4 +-
>  fs/crypto/fscrypt_private.h | 118 ++++++++++--
>  fs/crypto/inline_crypt.c    | 349 ++++++++++++++++++++++++++++++++++++
>  fs/crypto/keyring.c         |   6 +-
>  fs/crypto/keysetup.c        |  68 ++++---
>  fs/crypto/keysetup_v1.c     |  16 +-
>  include/linux/fscrypt.h     |  82 +++++++++
>  11 files changed, 655 insertions(+), 47 deletions(-)
>  create mode 100644 fs/crypto/inline_crypt.c
> 
> diff --git a/fs/crypto/Kconfig b/fs/crypto/Kconfig
> index 8046d7c7a3e9..f1f11a6228eb 100644
> --- a/fs/crypto/Kconfig
> +++ b/fs/crypto/Kconfig
> @@ -24,3 +24,9 @@ config FS_ENCRYPTION_ALGS
>  	select CRYPTO_SHA256
>  	select CRYPTO_SHA512
>  	select CRYPTO_XTS
> +
> +config FS_ENCRYPTION_INLINE_CRYPT
> +	bool "Enable fscrypt to use inline crypto"
> +	depends on FS_ENCRYPTION && BLK_INLINE_ENCRYPTION
> +	help
> +	  Enable fscrypt to use inline encryption hardware if available.
> diff --git a/fs/crypto/Makefile b/fs/crypto/Makefile
> index 232e2bb5a337..652c7180ec6d 100644
> --- a/fs/crypto/Makefile
> +++ b/fs/crypto/Makefile
> @@ -11,3 +11,4 @@ fscrypto-y := crypto.o \
>  	      policy.o
>  
>  fscrypto-$(CONFIG_BLOCK) += bio.o
> +fscrypto-$(CONFIG_FS_ENCRYPTION_INLINE_CRYPT) += inline_crypt.o
> diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c
> index 4fa18fff9c4e..1ea9369a7688 100644
> --- a/fs/crypto/bio.c
> +++ b/fs/crypto/bio.c
> @@ -41,6 +41,52 @@ void fscrypt_decrypt_bio(struct bio *bio)
>  }
>  EXPORT_SYMBOL(fscrypt_decrypt_bio);
>  
> +static int fscrypt_zeroout_range_inline_crypt(const struct inode *inode,
> +					      pgoff_t lblk, sector_t pblk,
> +					      unsigned int len)
> +{
> +	const unsigned int blockbits = inode->i_blkbits;
> +	const unsigned int blocks_per_page = 1 << (PAGE_SHIFT - blockbits);
> +	struct bio *bio;
> +	int ret, err = 0;
> +	int num_pages = 0;
> +
> +	/* This always succeeds since __GFP_DIRECT_RECLAIM is set. */
> +	bio = bio_alloc(GFP_NOFS, BIO_MAX_PAGES);
> +
> +	while (len) {
> +		unsigned int blocks_this_page = min(len, blocks_per_page);
> +		unsigned int bytes_this_page = blocks_this_page << blockbits;
> +
> +		if (num_pages == 0) {
> +			fscrypt_set_bio_crypt_ctx(bio, inode, lblk, GFP_NOFS);
> +			bio_set_dev(bio, inode->i_sb->s_bdev);
> +			bio->bi_iter.bi_sector =
> +					pblk << (blockbits - SECTOR_SHIFT);
> +			bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
> +		}
> +		ret = bio_add_page(bio, ZERO_PAGE(0), bytes_this_page, 0);
> +		if (WARN_ON(ret != bytes_this_page)) {
> +			err = -EIO;
> +			goto out;
> +		}
> +		num_pages++;
> +		len -= blocks_this_page;
> +		lblk += blocks_this_page;
> +		pblk += blocks_this_page;
> +		if (num_pages == BIO_MAX_PAGES || !len) {
> +			err = submit_bio_wait(bio);
> +			if (err)
> +				goto out;
> +			bio_reset(bio);
> +			num_pages = 0;
> +		}
> +	}
> +out:
> +	bio_put(bio);
> +	return err;
> +}
> +
>  /**
>   * fscrypt_zeroout_range() - zero out a range of blocks in an encrypted file
>   * @inode: the file's inode
> @@ -75,6 +121,10 @@ int fscrypt_zeroout_range(const struct inode *inode, pgoff_t lblk,
>  	if (len == 0)
>  		return 0;
>  
> +	if (fscrypt_inode_uses_inline_crypto(inode))
> +		return fscrypt_zeroout_range_inline_crypt(inode, lblk, pblk,
> +							  len);
> +
>  	BUILD_BUG_ON(ARRAY_SIZE(pages) > BIO_MAX_PAGES);
>  	nr_pages = min_t(unsigned int, ARRAY_SIZE(pages),
>  			 (len + blocks_per_page - 1) >> blocks_per_page_bits);
> diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
> index ed015cb66c7c..a52cf32733ab 100644
> --- a/fs/crypto/crypto.c
> +++ b/fs/crypto/crypto.c
> @@ -100,7 +100,7 @@ int fscrypt_crypt_block(const struct inode *inode, fscrypt_direction_t rw,
>  	DECLARE_CRYPTO_WAIT(wait);
>  	struct scatterlist dst, src;
>  	struct fscrypt_info *ci = inode->i_crypt_info;
> -	struct crypto_skcipher *tfm = ci->ci_ctfm;
> +	struct crypto_skcipher *tfm = ci->ci_enc_key.tfm;
>  	int res = 0;
>  
>  	if (WARN_ON_ONCE(len <= 0))
> diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
> index 83ca5f1e7934..d828e3df898b 100644
> --- a/fs/crypto/fname.c
> +++ b/fs/crypto/fname.c
> @@ -115,7 +115,7 @@ int fscrypt_fname_encrypt(const struct inode *inode, const struct qstr *iname,
>  	struct skcipher_request *req = NULL;
>  	DECLARE_CRYPTO_WAIT(wait);
>  	const struct fscrypt_info *ci = inode->i_crypt_info;
> -	struct crypto_skcipher *tfm = ci->ci_ctfm;
> +	struct crypto_skcipher *tfm = ci->ci_enc_key.tfm;
>  	union fscrypt_iv iv;
>  	struct scatterlist sg;
>  	int res;
> @@ -171,7 +171,7 @@ static int fname_decrypt(const struct inode *inode,
>  	DECLARE_CRYPTO_WAIT(wait);
>  	struct scatterlist src_sg, dst_sg;
>  	const struct fscrypt_info *ci = inode->i_crypt_info;
> -	struct crypto_skcipher *tfm = ci->ci_ctfm;
> +	struct crypto_skcipher *tfm = ci->ci_enc_key.tfm;
>  	union fscrypt_iv iv;
>  	int res;
>  
> diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
> index eb7fcd2b7fb8..1572186b0db4 100644
> --- a/fs/crypto/fscrypt_private.h
> +++ b/fs/crypto/fscrypt_private.h
> @@ -14,6 +14,7 @@
>  #include <linux/fscrypt.h>
>  #include <linux/siphash.h>
>  #include <crypto/hash.h>
> +#include <linux/blk-crypto.h>
>  
>  #define CONST_STRLEN(str)	(sizeof(str) - 1)
>  
> @@ -166,6 +167,20 @@ struct fscrypt_symlink_data {
>  	char encrypted_path[1];
>  } __packed;
>  
> +/**
> + * struct fscrypt_prepared_key - a key prepared for actual encryption/decryption
> + * @tfm: crypto API transform object
> + * @blk_key: key for blk-crypto
> + *
> + * Normally only one of the fields will be non-NULL.
> + */
> +struct fscrypt_prepared_key {
> +	struct crypto_skcipher *tfm;
> +#ifdef CONFIG_FS_ENCRYPTION_INLINE_CRYPT
> +	struct fscrypt_blk_crypto_key *blk_key;
> +#endif
> +};
> +
>  /*
>   * fscrypt_info - the "encryption key" for an inode
>   *
> @@ -175,12 +190,23 @@ struct fscrypt_symlink_data {
>   */
>  struct fscrypt_info {
>  
> -	/* The actual crypto transform used for encryption and decryption */
> -	struct crypto_skcipher *ci_ctfm;
> +	/* The key in a form prepared for actual encryption/decryption */
> +	struct fscrypt_prepared_key	ci_enc_key;
>  
> -	/* True if the key should be freed when this fscrypt_info is freed */
> +	/*
> +	 * True if the ci_enc_key should be freed when this fscrypt_info is
> +	 * freed
> +	 */
>  	bool ci_owns_key;
>  
> +#ifdef CONFIG_FS_ENCRYPTION_INLINE_CRYPT
> +	/*
> +	 * True if this inode will use inline encryption (blk-crypto) instead of
> +	 * the traditional filesystem-layer encryption.
> +	 */
> +	bool ci_inlinecrypt;
> +#endif
> +
>  	/*
>  	 * Encryption mode used for this inode.  It corresponds to either the
>  	 * contents or filenames encryption mode, depending on the inode type.
> @@ -205,7 +231,7 @@ struct fscrypt_info {
>  
>  	/*
>  	 * If non-NULL, then encryption is done using the master key directly
> -	 * and ci_ctfm will equal ci_direct_key->dk_ctfm.
> +	 * and ci_enc_key will equal ci_direct_key->dk_key.
>  	 */
>  	struct fscrypt_direct_key *ci_direct_key;
>  
> @@ -260,6 +286,7 @@ union fscrypt_iv {
>  		u8 nonce[FS_KEY_DERIVATION_NONCE_SIZE];
>  	};
>  	u8 raw[FSCRYPT_MAX_IV_SIZE];
> +	__le64 dun[FSCRYPT_MAX_IV_SIZE / sizeof(__le64)];
>  };
>  
>  void fscrypt_generate_iv(union fscrypt_iv *iv, u64 lblk_num,
> @@ -302,6 +329,74 @@ int fscrypt_hkdf_expand(const struct fscrypt_hkdf *hkdf, u8 context,
>  
>  void fscrypt_destroy_hkdf(struct fscrypt_hkdf *hkdf);
>  
> +/* inline_crypt.c */
> +#ifdef CONFIG_FS_ENCRYPTION_INLINE_CRYPT
> +void fscrypt_select_encryption_impl(struct fscrypt_info *ci);
> +
> +static inline bool
> +fscrypt_using_inline_encryption(const struct fscrypt_info *ci)
> +{
> +	return ci->ci_inlinecrypt;
> +}
> +
> +int fscrypt_prepare_inline_crypt_key(struct fscrypt_prepared_key *prep_key,
> +				     const u8 *raw_key,
> +				     const struct fscrypt_info *ci);
> +
> +void fscrypt_destroy_inline_crypt_key(struct fscrypt_prepared_key *prep_key);
> +
> +/*
> + * Check whether the crypto transform or blk-crypto key has been allocated in
> + * @prep_key, depending on which encryption implementation the file will use.
> + */
> +static inline bool
> +fscrypt_is_key_prepared(struct fscrypt_prepared_key *prep_key,
> +			const struct fscrypt_info *ci)
> +{
> +	/*
> +	 * The READ_ONCE() here pairs with the smp_store_release() in
> +	 * fscrypt_prepare_key().  (This only matters for the per-mode keys,
> +	 * which are shared by multiple inodes.)
> +	 */
> +	if (fscrypt_using_inline_encryption(ci))
> +		return READ_ONCE(prep_key->blk_key) != NULL;
> +	return READ_ONCE(prep_key->tfm) != NULL;
> +}
> +
> +#else /* CONFIG_FS_ENCRYPTION_INLINE_CRYPT */
> +
> +static inline void fscrypt_select_encryption_impl(struct fscrypt_info *ci)
> +{
> +}
> +
> +static inline bool fscrypt_using_inline_encryption(
> +					const struct fscrypt_info *ci)
> +{
> +	return false;
> +}
> +
> +static inline int
> +fscrypt_prepare_inline_crypt_key(struct fscrypt_prepared_key *prep_key,
> +				 const u8 *raw_key,
> +				 const struct fscrypt_info *ci)
> +{
> +	WARN_ON(1);
> +	return -EOPNOTSUPP;
> +}
> +
> +static inline void
> +fscrypt_destroy_inline_crypt_key(struct fscrypt_prepared_key *prep_key)
> +{
> +}
> +
> +static inline bool
> +fscrypt_is_key_prepared(struct fscrypt_prepared_key *prep_key,
> +			const struct fscrypt_info *ci)
> +{
> +	return READ_ONCE(prep_key->tfm) != NULL;
> +}
> +#endif /* !CONFIG_FS_ENCRYPTION_INLINE_CRYPT */
> +
>  /* keyring.c */
>  
>  /*
> @@ -395,9 +490,9 @@ struct fscrypt_master_key {
>  	 * Per-mode encryption keys for the various types of encryption policies
>  	 * that use them.  Allocated and derived on-demand.
>  	 */
> -	struct crypto_skcipher *mk_direct_keys[__FSCRYPT_MODE_MAX + 1];
> -	struct crypto_skcipher *mk_iv_ino_lblk_64_keys[__FSCRYPT_MODE_MAX + 1];
> -	struct crypto_skcipher *mk_iv_ino_lblk_32_keys[__FSCRYPT_MODE_MAX + 1];
> +	struct fscrypt_prepared_key mk_direct_keys[__FSCRYPT_MODE_MAX + 1];
> +	struct fscrypt_prepared_key mk_iv_ino_lblk_64_keys[__FSCRYPT_MODE_MAX + 1];
> +	struct fscrypt_prepared_key mk_iv_ino_lblk_32_keys[__FSCRYPT_MODE_MAX + 1];
>  
>  	/* Hash key for inode numbers.  Initialized only when needed. */
>  	siphash_key_t		mk_ino_hash_key;
> @@ -461,13 +556,16 @@ struct fscrypt_mode {
>  	int keysize;
>  	int ivsize;
>  	int logged_impl_name;
> +	enum blk_crypto_mode_num blk_crypto_mode;
>  };
>  
>  extern struct fscrypt_mode fscrypt_modes[];
>  
> -struct crypto_skcipher *fscrypt_allocate_skcipher(struct fscrypt_mode *mode,
> -						  const u8 *raw_key,
> -						  const struct inode *inode);
> +int fscrypt_prepare_key(struct fscrypt_prepared_key *prep_key,
> +			const u8 *raw_key,
> +			const struct fscrypt_info *ci);
> +
> +void fscrypt_destroy_prepared_key(struct fscrypt_prepared_key *prep_key);
>  
>  int fscrypt_set_per_file_enc_key(struct fscrypt_info *ci, const u8 *raw_key);
>  
> diff --git a/fs/crypto/inline_crypt.c b/fs/crypto/inline_crypt.c
> new file mode 100644
> index 000000000000..7dbeb49260a6
> --- /dev/null
> +++ b/fs/crypto/inline_crypt.c
> @@ -0,0 +1,349 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Inline encryption support for fscrypt
> + *
> + * Copyright 2019 Google LLC
> + */
> +
> +/*
> + * With "inline encryption", the block layer handles the decryption/encryption
> + * as part of the bio, instead of the filesystem doing the crypto itself via
> + * crypto API.  See Documentation/block/inline-encryption.rst.  fscrypt still
> + * provides the key and IV to use.
> + */
> +
> +#include <linux/blk-crypto.h>
> +#include <linux/blkdev.h>
> +#include <linux/buffer_head.h>
> +#include <linux/sched/mm.h>
> +
> +#include "fscrypt_private.h"
> +
> +struct fscrypt_blk_crypto_key {
> +	struct blk_crypto_key base;
> +	int num_devs;
> +	struct request_queue *devs[];
> +};
> +
> +static int fscrypt_get_num_devices(struct super_block *sb)
> +{
> +	if (sb->s_cop->get_num_devices)
> +		return sb->s_cop->get_num_devices(sb);
> +	return 1;
> +}
> +
> +static void fscrypt_get_devices(struct super_block *sb, int num_devs,
> +				struct request_queue **devs)
> +{
> +	if (num_devs == 1)
> +		devs[0] = bdev_get_queue(sb->s_bdev);
> +	else
> +		sb->s_cop->get_devices(sb, devs);
> +}
> +
> +static unsigned int fscrypt_get_dun_bytes(const struct fscrypt_info *ci)
> +{
> +	struct super_block *sb = ci->ci_inode->i_sb;
> +	unsigned int flags = fscrypt_policy_flags(&ci->ci_policy);
> +	int ino_bits = 64, lblk_bits = 64;
> +
> +	if (flags & FSCRYPT_POLICY_FLAG_DIRECT_KEY)
> +		return offsetofend(union fscrypt_iv, nonce);
> +
> +	if (flags & FSCRYPT_POLICY_FLAG_IV_INO_LBLK_64)
> +		return sizeof(__le64);
> +
> +	if (flags & FSCRYPT_POLICY_FLAG_IV_INO_LBLK_32)
> +		return sizeof(__le32);
> +
> +	/* Default case: IVs are just the file logical block number */
> +	if (sb->s_cop->get_ino_and_lblk_bits)
> +		sb->s_cop->get_ino_and_lblk_bits(sb, &ino_bits, &lblk_bits);
> +	return DIV_ROUND_UP(lblk_bits, 8);
> +}
> +
> +/* Enable inline encryption for this file if supported. */
> +void fscrypt_select_encryption_impl(struct fscrypt_info *ci)
> +{
> +	const struct inode *inode = ci->ci_inode;
> +	struct super_block *sb = inode->i_sb;
> +	struct blk_crypto_config crypto_cfg;
> +	int num_devs;
> +	struct request_queue **devs;
> +	int i;
> +
> +	/* The file must need contents encryption, not filenames encryption */
> +	if (!fscrypt_needs_contents_encryption(inode))
> +		return;
> +
> +	/* The crypto mode must be valid */
> +	if (ci->ci_mode->blk_crypto_mode == BLK_ENCRYPTION_MODE_INVALID)
> +		return;
> +
> +	/* The filesystem must be mounted with -o inlinecrypt */
> +	if (!(sb->s_flags & SB_INLINECRYPT))
> +		return;
> +
> +	/*
> +	 * blk-crypto must support the crypto configuration we'll use for the
> +	 * inode on all devices in the sb
> +	 */
> +	crypto_cfg.crypto_mode = ci->ci_mode->blk_crypto_mode;
> +	crypto_cfg.data_unit_size = sb->s_blocksize;
> +	crypto_cfg.dun_bytes = fscrypt_get_dun_bytes(ci);
> +	num_devs = fscrypt_get_num_devices(sb);
> +	devs = kmalloc_array(num_devs, sizeof(*devs), GFP_NOFS);
> +	if (!devs)
> +		return;
> +	fscrypt_get_devices(sb, num_devs, devs);
> +
> +	for (i = 0; i < num_devs; i++) {
> +		if (!blk_crypto_config_supported(devs[i], &crypto_cfg))
> +			goto out_free_devs;
> +	}
> +
> +	ci->ci_inlinecrypt = true;
> +out_free_devs:
> +	kfree(devs);
> +}
> +
> +int fscrypt_prepare_inline_crypt_key(struct fscrypt_prepared_key *prep_key,
> +				     const u8 *raw_key,
> +				     const struct fscrypt_info *ci)
> +{
> +	const struct inode *inode = ci->ci_inode;
> +	struct super_block *sb = inode->i_sb;
> +	enum blk_crypto_mode_num crypto_mode = ci->ci_mode->blk_crypto_mode;
> +	int num_devs = fscrypt_get_num_devices(sb);
> +	int queue_refs = 0;
> +	struct fscrypt_blk_crypto_key *blk_key;
> +	int err;
> +	int i;
> +	unsigned int flags;
> +
> +	blk_key = kzalloc(struct_size(blk_key, devs, num_devs), GFP_NOFS);
> +	if (!blk_key)
> +		return -ENOMEM;
> +
> +	blk_key->num_devs = num_devs;
> +	fscrypt_get_devices(sb, num_devs, blk_key->devs);
> +
> +	err = blk_crypto_init_key(&blk_key->base, raw_key, crypto_mode,
> +				  fscrypt_get_dun_bytes(ci), sb->s_blocksize);
> +	if (err) {
> +		fscrypt_err(inode, "error %d initializing blk-crypto key", err);
> +		goto fail;
> +	}
> +
> +	/*
> +	 * We have to start using blk-crypto on all the filesystem's devices.
> +	 * We also have to save all the request_queue's for later so that the
> +	 * key can be evicted from them.  This is needed because some keys
> +	 * aren't destroyed until after the filesystem was already unmounted
> +	 * (namely, the per-mode keys in struct fscrypt_master_key).
> +	 */
> +	for (i = 0; i < num_devs; i++) {
> +		if (!blk_get_queue(blk_key->devs[i])) {
> +			fscrypt_err(inode, "couldn't get request_queue");
> +			err = -EAGAIN;
> +			goto fail;
> +		}
> +		queue_refs++;
> +
> +		flags = memalloc_nofs_save();
> +		err = blk_crypto_start_using_key(&blk_key->base,
> +						 blk_key->devs[i]);
> +		memalloc_nofs_restore(flags);
> +		if (err) {
> +			fscrypt_err(inode,
> +				    "error %d starting to use blk-crypto", err);
> +			goto fail;
> +		}
> +	}
> +	/*
> +	 * Pairs with READ_ONCE() in fscrypt_is_key_prepared().  (Only matters
> +	 * for the per-mode keys, which are shared by multiple inodes.)
> +	 */
> +	smp_store_release(&prep_key->blk_key, blk_key);
> +	return 0;
> +
> +fail:
> +	for (i = 0; i < queue_refs; i++)
> +		blk_put_queue(blk_key->devs[i]);
> +	kzfree(blk_key);
> +	return err;
> +}
> +
> +void fscrypt_destroy_inline_crypt_key(struct fscrypt_prepared_key *prep_key)
> +{
> +	struct fscrypt_blk_crypto_key *blk_key = prep_key->blk_key;
> +	int i;
> +
> +	if (blk_key) {
> +		for (i = 0; i < blk_key->num_devs; i++) {
> +			blk_crypto_evict_key(blk_key->devs[i], &blk_key->base);
> +			blk_put_queue(blk_key->devs[i]);
> +		}
> +		kzfree(blk_key);
> +	}
> +}
> +
> +bool __fscrypt_inode_uses_inline_crypto(const struct inode *inode)
> +{
> +	return inode->i_crypt_info->ci_inlinecrypt;
> +}
> +EXPORT_SYMBOL_GPL(__fscrypt_inode_uses_inline_crypto);
> +
> +static void fscrypt_generate_dun(const struct fscrypt_info *ci, u64 lblk_num,
> +				 u64 dun[BLK_CRYPTO_DUN_ARRAY_SIZE])
> +{
> +	union fscrypt_iv iv;
> +	int i;
> +
> +	fscrypt_generate_iv(&iv, lblk_num, ci);
> +
> +	BUILD_BUG_ON(FSCRYPT_MAX_IV_SIZE > BLK_CRYPTO_MAX_IV_SIZE);
> +	memset(dun, 0, BLK_CRYPTO_MAX_IV_SIZE);
> +	for (i = 0; i < ci->ci_mode->ivsize/sizeof(dun[0]); i++)
> +		dun[i] = le64_to_cpu(iv.dun[i]);
> +}
> +
> +/**
> + * fscrypt_set_bio_crypt_ctx() - prepare a file contents bio for inline crypto
> + * @bio: a bio which will eventually be submitted to the file
> + * @inode: the file's inode
> + * @first_lblk: the first file logical block number in the I/O
> + * @gfp_mask: memory allocation flags - these must be a waiting mask so that
> + *					bio_crypt_set_ctx can't fail.
> + *
> + * If the contents of the file should be encrypted (or decrypted) with inline
> + * encryption, then assign the appropriate encryption context to the bio.
> + *
> + * Normally the bio should be newly allocated (i.e. no pages added yet), as
> + * otherwise fscrypt_mergeable_bio() won't work as intended.
> + *
> + * The encryption context will be freed automatically when the bio is freed.
> + */
> +void fscrypt_set_bio_crypt_ctx(struct bio *bio, const struct inode *inode,
> +			       u64 first_lblk, gfp_t gfp_mask)
> +{
> +	const struct fscrypt_info *ci = inode->i_crypt_info;
> +	u64 dun[BLK_CRYPTO_DUN_ARRAY_SIZE];
> +
> +	if (!fscrypt_inode_uses_inline_crypto(inode))
> +		return;
> +
> +	fscrypt_generate_dun(ci, first_lblk, dun);
> +	bio_crypt_set_ctx(bio, &ci->ci_enc_key.blk_key->base, dun, gfp_mask);
> +}
> +EXPORT_SYMBOL_GPL(fscrypt_set_bio_crypt_ctx);
> +
> +/* Extract the inode and logical block number from a buffer_head. */
> +static bool bh_get_inode_and_lblk_num(const struct buffer_head *bh,
> +				      const struct inode **inode_ret,
> +				      u64 *lblk_num_ret)
> +{
> +	struct page *page = bh->b_page;
> +	const struct address_space *mapping;
> +	const struct inode *inode;
> +
> +	/*
> +	 * The ext4 journal (jbd2) can submit a buffer_head it directly created
> +	 * for a non-pagecache page.  fscrypt doesn't care about these.
> +	 */
> +	mapping = page_mapping(page);
> +	if (!mapping)
> +		return false;
> +	inode = mapping->host;
> +
> +	*inode_ret = inode;
> +	*lblk_num_ret = ((u64)page->index << (PAGE_SHIFT - inode->i_blkbits)) +
> +			(bh_offset(bh) >> inode->i_blkbits);
> +	return true;
> +}
> +
> +/**
> + * fscrypt_set_bio_crypt_ctx_bh() - prepare a file contents bio for inline
> + *				    crypto
> + * @bio: a bio which will eventually be submitted to the file
> + * @first_bh: the first buffer_head for which I/O will be submitted
> + * @gfp_mask: memory allocation flags
> + *
> + * Same as fscrypt_set_bio_crypt_ctx(), except this takes a buffer_head instead
> + * of an inode and block number directly.
> + */
> +void fscrypt_set_bio_crypt_ctx_bh(struct bio *bio,
> +				  const struct buffer_head *first_bh,
> +				  gfp_t gfp_mask)
> +{
> +	const struct inode *inode;
> +	u64 first_lblk;
> +
> +	if (bh_get_inode_and_lblk_num(first_bh, &inode, &first_lblk))
> +		fscrypt_set_bio_crypt_ctx(bio, inode, first_lblk, gfp_mask);
> +}
> +EXPORT_SYMBOL_GPL(fscrypt_set_bio_crypt_ctx_bh);
> +
> +/**
> + * fscrypt_mergeable_bio() - test whether data can be added to a bio
> + * @bio: the bio being built up
> + * @inode: the inode for the next part of the I/O
> + * @next_lblk: the next file logical block number in the I/O
> + *
> + * When building a bio which may contain data which should undergo inline
> + * encryption (or decryption) via fscrypt, filesystems should call this function
> + * to ensure that the resulting bio contains only logically contiguous data.
> + * This will return false if the next part of the I/O cannot be merged with the
> + * bio because either the encryption key would be different or the encryption
> + * data unit numbers would be discontiguous.
> + *
> + * fscrypt_set_bio_crypt_ctx() must have already been called on the bio.
> + *
> + * Return: true iff the I/O is mergeable
> + */
> +bool fscrypt_mergeable_bio(struct bio *bio, const struct inode *inode,
> +			   u64 next_lblk)
> +{
> +	const struct bio_crypt_ctx *bc = bio->bi_crypt_context;
> +	u64 next_dun[BLK_CRYPTO_DUN_ARRAY_SIZE];
> +
> +	if (!!bc != fscrypt_inode_uses_inline_crypto(inode))
> +		return false;
> +	if (!bc)
> +		return true;
> +
> +	/*
> +	 * Comparing the key pointers is good enough, as all I/O for each key
> +	 * uses the same pointer.  I.e., there's currently no need to support
> +	 * merging requests where the keys are the same but the pointers differ.
> +	 */
> +	if (bc->bc_key != &inode->i_crypt_info->ci_enc_key.blk_key->base)
> +		return false;
> +
> +	fscrypt_generate_dun(inode->i_crypt_info, next_lblk, next_dun);
> +	return bio_crypt_dun_is_contiguous(bc, bio->bi_iter.bi_size, next_dun);
> +}
> +EXPORT_SYMBOL_GPL(fscrypt_mergeable_bio);
> +
> +/**
> + * fscrypt_mergeable_bio_bh() - test whether data can be added to a bio
> + * @bio: the bio being built up
> + * @next_bh: the next buffer_head for which I/O will be submitted
> + *
> + * Same as fscrypt_mergeable_bio(), except this takes a buffer_head instead of
> + * an inode and block number directly.
> + *
> + * Return: true iff the I/O is mergeable
> + */
> +bool fscrypt_mergeable_bio_bh(struct bio *bio,
> +			      const struct buffer_head *next_bh)
> +{
> +	const struct inode *inode;
> +	u64 next_lblk;
> +
> +	if (!bh_get_inode_and_lblk_num(next_bh, &inode, &next_lblk))
> +		return !bio->bi_crypt_context;
> +
> +	return fscrypt_mergeable_bio(bio, inode, next_lblk);
> +}
> +EXPORT_SYMBOL_GPL(fscrypt_mergeable_bio_bh);
> diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
> index e24eb48bfbe1..7f8ac61a20d6 100644
> --- a/fs/crypto/keyring.c
> +++ b/fs/crypto/keyring.c
> @@ -45,9 +45,9 @@ static void free_master_key(struct fscrypt_master_key *mk)
>  	wipe_master_key_secret(&mk->mk_secret);
>  
>  	for (i = 0; i <= __FSCRYPT_MODE_MAX; i++) {
> -		crypto_free_skcipher(mk->mk_direct_keys[i]);
> -		crypto_free_skcipher(mk->mk_iv_ino_lblk_64_keys[i]);
> -		crypto_free_skcipher(mk->mk_iv_ino_lblk_32_keys[i]);
> +		fscrypt_destroy_prepared_key(&mk->mk_direct_keys[i]);
> +		fscrypt_destroy_prepared_key(&mk->mk_iv_ino_lblk_64_keys[i]);
> +		fscrypt_destroy_prepared_key(&mk->mk_iv_ino_lblk_32_keys[i]);
>  	}
>  
>  	key_put(mk->mk_users);
> diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
> index 1129adfa097d..c789ac30af7c 100644
> --- a/fs/crypto/keysetup.c
> +++ b/fs/crypto/keysetup.c
> @@ -19,6 +19,7 @@ struct fscrypt_mode fscrypt_modes[] = {
>  		.cipher_str = "xts(aes)",
>  		.keysize = 64,
>  		.ivsize = 16,
> +		.blk_crypto_mode = BLK_ENCRYPTION_MODE_AES_256_XTS,
>  	},
>  	[FSCRYPT_MODE_AES_256_CTS] = {
>  		.friendly_name = "AES-256-CTS-CBC",
> @@ -31,6 +32,7 @@ struct fscrypt_mode fscrypt_modes[] = {
>  		.cipher_str = "essiv(cbc(aes),sha256)",
>  		.keysize = 16,
>  		.ivsize = 16,
> +		.blk_crypto_mode = BLK_ENCRYPTION_MODE_AES_128_CBC_ESSIV,
>  	},
>  	[FSCRYPT_MODE_AES_128_CTS] = {
>  		.friendly_name = "AES-128-CTS-CBC",
> @@ -43,6 +45,7 @@ struct fscrypt_mode fscrypt_modes[] = {
>  		.cipher_str = "adiantum(xchacha12,aes)",
>  		.keysize = 32,
>  		.ivsize = 32,
> +		.blk_crypto_mode = BLK_ENCRYPTION_MODE_ADIANTUM,
>  	},
>  };
>  
> @@ -64,9 +67,9 @@ select_encryption_mode(const union fscrypt_policy *policy,
>  }
>  
>  /* Create a symmetric cipher object for the given encryption mode and key */
> -struct crypto_skcipher *fscrypt_allocate_skcipher(struct fscrypt_mode *mode,
> -						  const u8 *raw_key,
> -						  const struct inode *inode)
> +static struct crypto_skcipher *
> +fscrypt_allocate_skcipher(struct fscrypt_mode *mode, const u8 *raw_key,
> +			  const struct inode *inode)
>  {
>  	struct crypto_skcipher *tfm;
>  	int err;
> @@ -109,30 +112,54 @@ struct crypto_skcipher *fscrypt_allocate_skcipher(struct fscrypt_mode *mode,
>  	return ERR_PTR(err);
>  }
>  
> -/* 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)
> +/*
> + * Prepare the crypto transform object or blk-crypto key in @prep_key, given the
> + * raw key, encryption mode, and flag indicating which encryption implementation
> + * (fs-layer or blk-crypto) will be used.
> + */
> +int fscrypt_prepare_key(struct fscrypt_prepared_key *prep_key,
> +			const u8 *raw_key, const struct fscrypt_info *ci)
>  {
>  	struct crypto_skcipher *tfm;
>  
> +	if (fscrypt_using_inline_encryption(ci))
> +		return fscrypt_prepare_inline_crypt_key(prep_key, raw_key, ci);
> +
>  	tfm = fscrypt_allocate_skcipher(ci->ci_mode, raw_key, ci->ci_inode);
>  	if (IS_ERR(tfm))
>  		return PTR_ERR(tfm);
> +	/*
> +	 * Pairs with READ_ONCE() in fscrypt_is_key_prepared().  (Only matters
> +	 * for the per-mode keys, which are shared by multiple inodes.)
> +	 */
> +	smp_store_release(&prep_key->tfm, tfm);
> +	return 0;
> +}
> +
> +/* Destroy a crypto transform object and/or blk-crypto key. */
> +void fscrypt_destroy_prepared_key(struct fscrypt_prepared_key *prep_key)
> +{
> +	crypto_free_skcipher(prep_key->tfm);
> +	fscrypt_destroy_inline_crypt_key(prep_key);
> +}
>  
> -	ci->ci_ctfm = tfm;
> +/* 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;
> -	return 0;
> +	return fscrypt_prepare_key(&ci->ci_enc_key, raw_key, ci);
>  }
>  
>  static int setup_per_mode_enc_key(struct fscrypt_info *ci,
>  				  struct fscrypt_master_key *mk,
> -				  struct crypto_skcipher **tfms,
> +				  struct fscrypt_prepared_key *keys,
>  				  u8 hkdf_context, bool include_fs_uuid)
>  {
>  	const struct inode *inode = ci->ci_inode;
>  	const struct super_block *sb = inode->i_sb;
>  	struct fscrypt_mode *mode = ci->ci_mode;
>  	const u8 mode_num = mode - fscrypt_modes;
> -	struct crypto_skcipher *tfm;
> +	struct fscrypt_prepared_key *prep_key;
>  	u8 mode_key[FSCRYPT_MAX_KEY_SIZE];
>  	u8 hkdf_info[sizeof(mode_num) + sizeof(sb->s_uuid)];
>  	unsigned int hkdf_infolen = 0;
> @@ -141,16 +168,15 @@ static int setup_per_mode_enc_key(struct fscrypt_info *ci,
>  	if (WARN_ON(mode_num > __FSCRYPT_MODE_MAX))
>  		return -EINVAL;
>  
> -	/* pairs with smp_store_release() below */
> -	tfm = READ_ONCE(tfms[mode_num]);
> -	if (likely(tfm != NULL)) {
> -		ci->ci_ctfm = tfm;
> +	prep_key = &keys[mode_num];
> +	if (fscrypt_is_key_prepared(prep_key, ci)) {
> +		ci->ci_enc_key = *prep_key;
>  		return 0;
>  	}
>  
>  	mutex_lock(&fscrypt_mode_key_setup_mutex);
>  
> -	if (tfms[mode_num])
> +	if (fscrypt_is_key_prepared(prep_key, ci))
>  		goto done_unlock;
>  
>  	BUILD_BUG_ON(sizeof(mode_num) != 1);
> @@ -167,16 +193,12 @@ static int setup_per_mode_enc_key(struct fscrypt_info *ci,
>  				  mode_key, mode->keysize);
>  	if (err)
>  		goto out_unlock;
> -	tfm = fscrypt_allocate_skcipher(mode, mode_key, inode);
> +	err = fscrypt_prepare_key(prep_key, mode_key, ci);
>  	memzero_explicit(mode_key, mode->keysize);
> -	if (IS_ERR(tfm)) {
> -		err = PTR_ERR(tfm);
> +	if (err)
>  		goto out_unlock;
> -	}
> -	/* pairs with READ_ONCE() above */
> -	smp_store_release(&tfms[mode_num], tfm);
>  done_unlock:
> -	ci->ci_ctfm = tfm;
> +	ci->ci_enc_key = *prep_key;
>  	err = 0;
>  out_unlock:
>  	mutex_unlock(&fscrypt_mode_key_setup_mutex);
> @@ -310,6 +332,8 @@ static int setup_file_encryption_key(struct fscrypt_info *ci,
>  	struct fscrypt_key_specifier mk_spec;
>  	int err;
>  
> +	fscrypt_select_encryption_impl(ci);
> +
>  	switch (ci->ci_policy.version) {
>  	case FSCRYPT_POLICY_V1:
>  		mk_spec.type = FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR;
> @@ -402,7 +426,7 @@ static void put_crypt_info(struct fscrypt_info *ci)
>  	if (ci->ci_direct_key)
>  		fscrypt_put_direct_key(ci->ci_direct_key);
>  	else if (ci->ci_owns_key)
> -		crypto_free_skcipher(ci->ci_ctfm);
> +		fscrypt_destroy_prepared_key(&ci->ci_enc_key);
>  
>  	key = ci->ci_master_key;
>  	if (key) {
> diff --git a/fs/crypto/keysetup_v1.c b/fs/crypto/keysetup_v1.c
> index 801b48c0cd7f..a52686729a67 100644
> --- a/fs/crypto/keysetup_v1.c
> +++ b/fs/crypto/keysetup_v1.c
> @@ -146,7 +146,7 @@ struct fscrypt_direct_key {
>  	struct hlist_node		dk_node;
>  	refcount_t			dk_refcount;
>  	const struct fscrypt_mode	*dk_mode;
> -	struct crypto_skcipher		*dk_ctfm;
> +	struct fscrypt_prepared_key	dk_key;
>  	u8				dk_descriptor[FSCRYPT_KEY_DESCRIPTOR_SIZE];
>  	u8				dk_raw[FSCRYPT_MAX_KEY_SIZE];
>  };
> @@ -154,7 +154,7 @@ struct fscrypt_direct_key {
>  static void free_direct_key(struct fscrypt_direct_key *dk)
>  {
>  	if (dk) {
> -		crypto_free_skcipher(dk->dk_ctfm);
> +		fscrypt_destroy_prepared_key(&dk->dk_key);
>  		kzfree(dk);
>  	}
>  }
> @@ -199,6 +199,8 @@ find_or_insert_direct_key(struct fscrypt_direct_key *to_insert,
>  			continue;
>  		if (ci->ci_mode != dk->dk_mode)
>  			continue;
> +		if (!fscrypt_is_key_prepared(&dk->dk_key, ci))
> +			continue;
>  		if (crypto_memneq(raw_key, dk->dk_raw, ci->ci_mode->keysize))
>  			continue;
>  		/* using existing tfm with same (descriptor, mode, raw_key) */
> @@ -231,13 +233,9 @@ fscrypt_get_direct_key(const struct fscrypt_info *ci, const u8 *raw_key)
>  		return ERR_PTR(-ENOMEM);
>  	refcount_set(&dk->dk_refcount, 1);
>  	dk->dk_mode = ci->ci_mode;
> -	dk->dk_ctfm = fscrypt_allocate_skcipher(ci->ci_mode, raw_key,
> -						ci->ci_inode);
> -	if (IS_ERR(dk->dk_ctfm)) {
> -		err = PTR_ERR(dk->dk_ctfm);
> -		dk->dk_ctfm = NULL;
> +	err = fscrypt_prepare_key(&dk->dk_key, raw_key, ci);
> +	if (err)
>  		goto err_free_dk;
> -	}
>  	memcpy(dk->dk_descriptor, ci->ci_policy.v1.master_key_descriptor,
>  	       FSCRYPT_KEY_DESCRIPTOR_SIZE);
>  	memcpy(dk->dk_raw, raw_key, ci->ci_mode->keysize);
> @@ -259,7 +257,7 @@ static int setup_v1_file_key_direct(struct fscrypt_info *ci,
>  	if (IS_ERR(dk))
>  		return PTR_ERR(dk);
>  	ci->ci_direct_key = dk;
> -	ci->ci_ctfm = dk->dk_ctfm;
> +	ci->ci_enc_key = dk->dk_key;
>  	return 0;
>  }
>  
> diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
> index 2862ca5fea33..bb257411365f 100644
> --- a/include/linux/fscrypt.h
> +++ b/include/linux/fscrypt.h
> @@ -69,6 +69,9 @@ struct fscrypt_operations {
>  	bool (*has_stable_inodes)(struct super_block *sb);
>  	void (*get_ino_and_lblk_bits)(struct super_block *sb,
>  				      int *ino_bits_ret, int *lblk_bits_ret);
> +	int (*get_num_devices)(struct super_block *sb);
> +	void (*get_devices)(struct super_block *sb,
> +			    struct request_queue **devs);
>  };
>  
>  static inline bool fscrypt_has_encryption_key(const struct inode *inode)
> @@ -537,6 +540,85 @@ static inline void fscrypt_set_ops(struct super_block *sb,
>  
>  #endif	/* !CONFIG_FS_ENCRYPTION */
>  
> +/* inline_crypt.c */
> +#ifdef CONFIG_FS_ENCRYPTION_INLINE_CRYPT
> +
> +bool __fscrypt_inode_uses_inline_crypto(const struct inode *inode);
> +
> +void fscrypt_set_bio_crypt_ctx(struct bio *bio,
> +			       const struct inode *inode, u64 first_lblk,
> +			       gfp_t gfp_mask);
> +
> +void fscrypt_set_bio_crypt_ctx_bh(struct bio *bio,
> +				  const struct buffer_head *first_bh,
> +				  gfp_t gfp_mask);
> +
> +bool fscrypt_mergeable_bio(struct bio *bio, const struct inode *inode,
> +			   u64 next_lblk);
> +
> +bool fscrypt_mergeable_bio_bh(struct bio *bio,
> +			      const struct buffer_head *next_bh);
> +
> +#else /* CONFIG_FS_ENCRYPTION_INLINE_CRYPT */
> +
> +static inline bool __fscrypt_inode_uses_inline_crypto(const struct inode *inode)
> +{
> +	return false;
> +}
> +
> +static inline void fscrypt_set_bio_crypt_ctx(struct bio *bio,
> +					     const struct inode *inode,
> +					     u64 first_lblk, gfp_t gfp_mask) { }
> +
> +static inline void fscrypt_set_bio_crypt_ctx_bh(
> +					 struct bio *bio,
> +					 const struct buffer_head *first_bh,
> +					 gfp_t gfp_mask) { }
> +
> +static inline bool fscrypt_mergeable_bio(struct bio *bio,
> +					 const struct inode *inode,
> +					 u64 next_lblk)
> +{
> +	return true;
> +}
> +
> +static inline bool fscrypt_mergeable_bio_bh(struct bio *bio,
> +					    const struct buffer_head *next_bh)
> +{
> +	return true;
> +}
> +#endif /* !CONFIG_FS_ENCRYPTION_INLINE_CRYPT */
> +
> +/**
> + * fscrypt_inode_uses_inline_crypto() - test whether an inode uses inline
> + *					encryption
> + * @inode: an inode. If encrypted, its key must be set up.
> + *
> + * Return: true if the inode requires file contents encryption and if the
> + *	   encryption should be done in the block layer via blk-crypto rather
> + *	   than in the filesystem layer.
> + */
> +static inline bool fscrypt_inode_uses_inline_crypto(const struct inode *inode)
> +{
> +	return fscrypt_needs_contents_encryption(inode) &&
> +	       __fscrypt_inode_uses_inline_crypto(inode);
> +}
> +
> +/**
> + * fscrypt_inode_uses_fs_layer_crypto() - test whether an inode uses fs-layer
> + *					  encryption
> + * @inode: an inode. If encrypted, its key must be set up.
> + *
> + * Return: true if the inode requires file contents encryption and if the
> + *	   encryption should be done in the filesystem layer rather than in the
> + *	   block layer via blk-crypto.
> + */
> +static inline bool fscrypt_inode_uses_fs_layer_crypto(const struct inode *inode)
> +{
> +	return fscrypt_needs_contents_encryption(inode) &&
> +	       !__fscrypt_inode_uses_inline_crypto(inode);
> +}
> +
>  /**
>   * fscrypt_require_key() - require an inode's encryption key
>   * @inode: the inode we need the key for
> -- 
> 2.27.0.290.gba653c62da-goog

Re: [PATCH 2/4] fscrypt: add inline encryption support

[Eric Biggers]

A few nits:

On Wed, Jun 17, 2020 at 07:57:30AM +0000, Satya Tangirala wrote:
> To use inline encryption, the filesystem needs to be mounted with
> '-o inlinecrypt'.  The contents of any encrypted files will then be
> encrypted using blk-crypto, instead of using the traditional
> filesystem-layer crypto.

This isn't clear about what happens when blk-crypto isn't supported on a
file.  How about:

"To use inline encryption, the filesystem needs to be mounted with
'-o inlinecrypt'.  blk-crypto will then be used to encrypt the contents
of any encrypted files where it can be used instead of the traditional
filesystem-layer crypto."

> diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
> index eb7fcd2b7fb8..1572186b0db4 100644
> --- a/fs/crypto/fscrypt_private.h
> +++ b/fs/crypto/fscrypt_private.h
> @@ -14,6 +14,7 @@
>  #include <linux/fscrypt.h>
>  #include <linux/siphash.h>
>  #include <crypto/hash.h>
> +#include <linux/blk-crypto.h>
>  
>  #define CONST_STRLEN(str)	(sizeof(str) - 1)
>  
> @@ -166,6 +167,20 @@ struct fscrypt_symlink_data {
>  	char encrypted_path[1];
>  } __packed;
>  
> +/**
> + * struct fscrypt_prepared_key - a key prepared for actual encryption/decryption
> + * @tfm: crypto API transform object
> + * @blk_key: key for blk-crypto
> + *
> + * Normally only one of the fields will be non-NULL.
> + */
> +struct fscrypt_prepared_key {
> +	struct crypto_skcipher *tfm;
> +#ifdef CONFIG_FS_ENCRYPTION_INLINE_CRYPT
> +	struct fscrypt_blk_crypto_key *blk_key;
> +#endif
> +};
> +
>  /*
>   * fscrypt_info - the "encryption key" for an inode
>   *
> @@ -175,12 +190,23 @@ struct fscrypt_symlink_data {
>   */
>  struct fscrypt_info {
>  
> -	/* The actual crypto transform used for encryption and decryption */
> -	struct crypto_skcipher *ci_ctfm;
> +	/* The key in a form prepared for actual encryption/decryption */
> +	struct fscrypt_prepared_key	ci_enc_key;

Space instead of tab before ci_enc_key, to match the other fields of
this struct.

>  
> -	/* True if the key should be freed when this fscrypt_info is freed */
> +	/*
> +	 * True if the ci_enc_key should be freed when this fscrypt_info is
> +	 * freed
> +	 */
>  	bool ci_owns_key;

This comment would fit nicely on one line if "the" was deleted:

	/* True if ci_enc_key should be freed when this fscrypt_info is freed */

> +/* inline_crypt.c */
> +#ifdef CONFIG_FS_ENCRYPTION_INLINE_CRYPT
> +void fscrypt_select_encryption_impl(struct fscrypt_info *ci);
> +
> +static inline bool
> +fscrypt_using_inline_encryption(const struct fscrypt_info *ci)
> +{
> +	return ci->ci_inlinecrypt;
> +}
> +
> +int fscrypt_prepare_inline_crypt_key(struct fscrypt_prepared_key *prep_key,
> +				     const u8 *raw_key,
> +				     const struct fscrypt_info *ci);
> +
> +void fscrypt_destroy_inline_crypt_key(struct fscrypt_prepared_key *prep_key);
> +
> +/*
> + * Check whether the crypto transform or blk-crypto key has been allocated in
> + * @prep_key, depending on which encryption implementation the file will use.
> + */
> +static inline bool
> +fscrypt_is_key_prepared(struct fscrypt_prepared_key *prep_key,
> +			const struct fscrypt_info *ci)
> +{
> +	/*
> +	 * The READ_ONCE() here pairs with the smp_store_release() in
> +	 * fscrypt_prepare_key().  (This only matters for the per-mode keys,
> +	 * which are shared by multiple inodes.)
> +	 */
> +	if (fscrypt_using_inline_encryption(ci))
> +		return READ_ONCE(prep_key->blk_key) != NULL;
> +	return READ_ONCE(prep_key->tfm) != NULL;
> +}
> +
> +#else /* CONFIG_FS_ENCRYPTION_INLINE_CRYPT */
> +
> +static inline void fscrypt_select_encryption_impl(struct fscrypt_info *ci)
> +{
> +}
> +
> +static inline bool fscrypt_using_inline_encryption(
> +					const struct fscrypt_info *ci)
> +{
> +	return false;
> +}

fscrypt_using_inline_encryption() here is formatted differently from the
CONFIG_FS_ENCRYPTION_INLINE_CRYPT=y case.  I'd use for both:

static inline bool
fscrypt_using_inline_encryption(const struct fscrypt_info *ci)

> +int fscrypt_prepare_key(struct fscrypt_prepared_key *prep_key,
> +			const u8 *raw_key,
> +			const struct fscrypt_info *ci);

'raw_key' and 'ci' fit on one line.  (Note: the definition already does that.)

> +/* Enable inline encryption for this file if supported. */
> +void fscrypt_select_encryption_impl(struct fscrypt_info *ci)
> +{

This function should return an error code (0 or -ENOMEM) so that
failure of kmalloc_array() can be reported.

> +	/* The crypto mode must be valid */
> +	if (ci->ci_mode->blk_crypto_mode == BLK_ENCRYPTION_MODE_INVALID)
> +		return;

The comment "The crypto mode must be valid" is confusing, since the mode
*is* valid for fscrypt, just not for blk-crypto.  How about:

	/* The crypto mode must have a blk-crypto counterpart */

> +	/*
> +	 * blk-crypto must support the crypto configuration we'll use for the
> +	 * inode on all devices in the sb
> +	 */

I think the following would be a slightly clearer and more consistent
with other comments:

	/*
	 * On all the filesystem's devices, blk-crypto must support the crypto
	 * configuration that the file would use.
	 */

> +/**
> + * fscrypt_mergeable_bio() - test whether data can be added to a bio
> + * @bio: the bio being built up
> + * @inode: the inode for the next part of the I/O
> + * @next_lblk: the next file logical block number in the I/O
> + *
> + * When building a bio which may contain data which should undergo inline
> + * encryption (or decryption) via fscrypt, filesystems should call this function
> + * to ensure that the resulting bio contains only logically contiguous data.
> + * This will return false if the next part of the I/O cannot be merged with the
> + * bio because either the encryption key would be different or the encryption
> + * data unit numbers would be discontiguous.
> + *
> + * fscrypt_set_bio_crypt_ctx() must have already been called on the bio.
> + *
> + * Return: true iff the I/O is mergeable
> + */

The mention of "logically contiguous data" here is now technically wrong
due to the new IV_INO_LBLK_32 IV generation method.  For that, logically
contiguous blocks don't necessarily have contiguous DUNs.

I'd replace in this comment:
	"logically contiguous data" => "contiguous data unit numbers"

- Eric

[PATCH 3/4] f2fs: add inline encryption support

[Satya Tangirala]

Wire up f2fs to support inline encryption via the helper functions which
fs/crypto/ now provides.  This includes:

- Adding a mount option 'inlinecrypt' which enables inline encryption
  on encrypted files where it can be used.

- Setting the bio_crypt_ctx on bios that will be submitted to an
  inline-encrypted file.

- Not adding logically discontiguous data to bios that will be submitted
  to an inline-encrypted file.

- Not doing filesystem-layer crypto on inline-encrypted files.

This patch includes a fix for a race during IPU by
Sahitya Tummala <stummala@codeaurora.org>

Co-developed-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Satya Tangirala <satyat@google.com>
---
 Documentation/filesystems/f2fs.rst |  7 ++-
 fs/f2fs/compress.c                 |  2 +-
 fs/f2fs/data.c                     | 81 ++++++++++++++++++++++++------
 fs/f2fs/super.c                    | 32 ++++++++++++
 4 files changed, 104 insertions(+), 18 deletions(-)

diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
index 099d45ac8d8f..4dc36143ff82 100644
--- a/Documentation/filesystems/f2fs.rst
+++ b/Documentation/filesystems/f2fs.rst
@@ -258,7 +258,12 @@ compress_extension=%s  Support adding specified extension, so that f2fs can enab
                        on compression extension list and enable compression on
                        these file by default rather than to enable it via ioctl.
                        For other files, we can still enable compression via ioctl.
-====================== ============================================================
+inlinecrypt
+                       Encrypt/decrypt the contents of encrypted files using the
+                       blk-crypto framework rather than filesystem-layer encryption.
+                       This allows the use of inline encryption hardware. The on-disk
+                       format is unaffected. For more details, see
+                       Documentation/block/inline-encryption.rst.
 
 Debugfs Entries
 ===============
diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index 1e02a8c106b0..29e50fbe7eca 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -1086,7 +1086,7 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
 		.submitted = false,
 		.io_type = io_type,
 		.io_wbc = wbc,
-		.encrypted = f2fs_encrypted_file(cc->inode),
+		.encrypted = fscrypt_inode_uses_fs_layer_crypto(cc->inode),
 	};
 	struct dnode_of_data dn;
 	struct node_info ni;
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 326c63879ddc..6955ea6fa1b6 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -14,6 +14,7 @@
 #include <linux/pagevec.h>
 #include <linux/blkdev.h>
 #include <linux/bio.h>
+#include <linux/blk-crypto.h>
 #include <linux/swap.h>
 #include <linux/prefetch.h>
 #include <linux/uio.h>
@@ -459,6 +460,33 @@ static struct bio *__bio_alloc(struct f2fs_io_info *fio, int npages)
 	return bio;
 }
 
+static void f2fs_set_bio_crypt_ctx(struct bio *bio, const struct inode *inode,
+				  pgoff_t first_idx,
+				  const struct f2fs_io_info *fio,
+				  gfp_t gfp_mask)
+{
+	/*
+	 * The f2fs garbage collector sets ->encrypted_page when it wants to
+	 * read/write raw data without encryption.
+	 */
+	if (!fio || !fio->encrypted_page)
+		fscrypt_set_bio_crypt_ctx(bio, inode, first_idx, gfp_mask);
+}
+
+static bool f2fs_crypt_mergeable_bio(struct bio *bio, const struct inode *inode,
+				     pgoff_t next_idx,
+				     const struct f2fs_io_info *fio)
+{
+	/*
+	 * The f2fs garbage collector sets ->encrypted_page when it wants to
+	 * read/write raw data without encryption.
+	 */
+	if (fio && fio->encrypted_page)
+		return !bio_has_crypt_ctx(bio);
+
+	return fscrypt_mergeable_bio(bio, inode, next_idx);
+}
+
 static inline void __submit_bio(struct f2fs_sb_info *sbi,
 				struct bio *bio, enum page_type type)
 {
@@ -684,6 +712,9 @@ int f2fs_submit_page_bio(struct f2fs_io_info *fio)
 	/* Allocate a new bio */
 	bio = __bio_alloc(fio, 1);
 
+	f2fs_set_bio_crypt_ctx(bio, fio->page->mapping->host,
+			       fio->page->index, fio, GFP_NOIO);
+
 	if (bio_add_page(bio, page, PAGE_SIZE, 0) < PAGE_SIZE) {
 		bio_put(bio);
 		return -EFAULT;
@@ -763,9 +794,10 @@ static void del_bio_entry(struct bio_entry *be)
 	kmem_cache_free(bio_entry_slab, be);
 }
 
-static int add_ipu_page(struct f2fs_sb_info *sbi, struct bio **bio,
+static int add_ipu_page(struct f2fs_io_info *fio, struct bio **bio,
 							struct page *page)
 {
+	struct f2fs_sb_info *sbi = fio->sbi;
 	enum temp_type temp;
 	bool found = false;
 	int ret = -EAGAIN;
@@ -782,13 +814,18 @@ static int add_ipu_page(struct f2fs_sb_info *sbi, struct bio **bio,
 
 			found = true;
 
-			if (bio_add_page(*bio, page, PAGE_SIZE, 0) ==
-							PAGE_SIZE) {
+			if (page_is_mergeable(sbi, *bio, *fio->last_block,
+					fio->new_blkaddr) &&
+			    f2fs_crypt_mergeable_bio(*bio,
+					fio->page->mapping->host,
+					fio->page->index, fio) &&
+			    bio_add_page(*bio, page, PAGE_SIZE, 0) ==
+					PAGE_SIZE) {
 				ret = 0;
 				break;
 			}
 
-			/* bio is full */
+			 /* page can't be merged into bio; submit the bio */
 			del_bio_entry(be);
 			__submit_bio(sbi, *bio, DATA);
 			break;
@@ -873,18 +910,17 @@ int f2fs_merge_page_bio(struct f2fs_io_info *fio)
 	trace_f2fs_submit_page_bio(page, fio);
 	f2fs_trace_ios(fio, 0);
 
-	if (bio && !page_is_mergeable(fio->sbi, bio, *fio->last_block,
-						fio->new_blkaddr))
-		f2fs_submit_merged_ipu_write(fio->sbi, &bio, NULL);
 alloc_new:
 	if (!bio) {
 		bio = __bio_alloc(fio, BIO_MAX_PAGES);
 		__attach_io_flag(fio);
+		f2fs_set_bio_crypt_ctx(bio, fio->page->mapping->host,
+				       fio->page->index, fio, GFP_NOIO);
 		bio_set_op_attrs(bio, fio->op, fio->op_flags);
 
 		add_bio_entry(fio->sbi, bio, page, fio->temp);
 	} else {
-		if (add_ipu_page(fio->sbi, &bio, page))
+		if (add_ipu_page(fio, &bio, page))
 			goto alloc_new;
 	}
 
@@ -936,8 +972,11 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
 
 	inc_page_count(sbi, WB_DATA_TYPE(bio_page));
 
-	if (io->bio && !io_is_mergeable(sbi, io->bio, io, fio,
-			io->last_block_in_bio, fio->new_blkaddr))
+	if (io->bio &&
+	    (!io_is_mergeable(sbi, io->bio, io, fio, io->last_block_in_bio,
+			      fio->new_blkaddr) ||
+	     !f2fs_crypt_mergeable_bio(io->bio, fio->page->mapping->host,
+				       fio->page->index, fio)))
 		__submit_merged_bio(io);
 alloc_new:
 	if (io->bio == NULL) {
@@ -949,6 +988,8 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
 			goto skip;
 		}
 		io->bio = __bio_alloc(fio, BIO_MAX_PAGES);
+		f2fs_set_bio_crypt_ctx(io->bio, fio->page->mapping->host,
+				       fio->page->index, fio, GFP_NOIO);
 		io->fio = *fio;
 	}
 
@@ -993,11 +1034,14 @@ static struct bio *f2fs_grab_read_bio(struct inode *inode, block_t blkaddr,
 								for_write);
 	if (!bio)
 		return ERR_PTR(-ENOMEM);
+
+	f2fs_set_bio_crypt_ctx(bio, inode, first_idx, NULL, GFP_NOFS);
+
 	f2fs_target_device(sbi, blkaddr, bio);
 	bio->bi_end_io = f2fs_read_end_io;
 	bio_set_op_attrs(bio, REQ_OP_READ, op_flag);
 
-	if (f2fs_encrypted_file(inode))
+	if (fscrypt_inode_uses_fs_layer_crypto(inode))
 		post_read_steps |= 1 << STEP_DECRYPT;
 	if (f2fs_compressed_file(inode))
 		post_read_steps |= 1 << STEP_DECOMPRESS_NOWQ;
@@ -2073,8 +2117,9 @@ static int f2fs_read_single_page(struct inode *inode, struct page *page,
 	 * This page will go to BIO.  Do we need to send this
 	 * BIO off first?
 	 */
-	if (bio && !page_is_mergeable(F2FS_I_SB(inode), bio,
-				*last_block_in_bio, block_nr)) {
+	if (bio && (!page_is_mergeable(F2FS_I_SB(inode), bio,
+				       *last_block_in_bio, block_nr) ||
+		    !f2fs_crypt_mergeable_bio(bio, inode, page->index, NULL))) {
 submit_and_realloc:
 		__submit_bio(F2FS_I_SB(inode), bio, DATA);
 		bio = NULL;
@@ -2204,8 +2249,9 @@ int f2fs_read_multi_pages(struct compress_ctx *cc, struct bio **bio_ret,
 		blkaddr = data_blkaddr(dn.inode, dn.node_page,
 						dn.ofs_in_node + i + 1);
 
-		if (bio && !page_is_mergeable(sbi, bio,
-					*last_block_in_bio, blkaddr)) {
+		if (bio && (!page_is_mergeable(sbi, bio,
+					*last_block_in_bio, blkaddr) ||
+		    !f2fs_crypt_mergeable_bio(bio, inode, page->index, NULL))) {
 submit_and_realloc:
 			__submit_bio(sbi, bio, DATA);
 			bio = NULL;
@@ -2421,6 +2467,9 @@ int f2fs_encrypt_one_page(struct f2fs_io_info *fio)
 	/* wait for GCed page writeback via META_MAPPING */
 	f2fs_wait_on_block_writeback(inode, fio->old_blkaddr);
 
+	if (fscrypt_inode_uses_inline_crypto(inode))
+		return 0;
+
 retry_encrypt:
 	fio->encrypted_page = fscrypt_encrypt_pagecache_blocks(page,
 					PAGE_SIZE, 0, gfp_flags);
@@ -2594,7 +2643,7 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
 			f2fs_unlock_op(fio->sbi);
 		err = f2fs_inplace_write_data(fio);
 		if (err) {
-			if (f2fs_encrypted_file(inode))
+			if (fscrypt_inode_uses_fs_layer_crypto(inode))
 				fscrypt_finalize_bounce_page(&fio->encrypted_page);
 			if (PageWriteback(page))
 				end_page_writeback(page);
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 20e56b0fa46a..3621969b2665 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -138,6 +138,7 @@ enum {
 	Opt_alloc,
 	Opt_fsync,
 	Opt_test_dummy_encryption,
+	Opt_inlinecrypt,
 	Opt_checkpoint_disable,
 	Opt_checkpoint_disable_cap,
 	Opt_checkpoint_disable_cap_perc,
@@ -204,6 +205,7 @@ static match_table_t f2fs_tokens = {
 	{Opt_fsync, "fsync_mode=%s"},
 	{Opt_test_dummy_encryption, "test_dummy_encryption=%s"},
 	{Opt_test_dummy_encryption, "test_dummy_encryption"},
+	{Opt_inlinecrypt, "inlinecrypt"},
 	{Opt_checkpoint_disable, "checkpoint=disable"},
 	{Opt_checkpoint_disable_cap, "checkpoint=disable:%u"},
 	{Opt_checkpoint_disable_cap_perc, "checkpoint=disable:%u%%"},
@@ -833,6 +835,13 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
 			if (ret)
 				return ret;
 			break;
+		case Opt_inlinecrypt:
+#ifdef CONFIG_FS_ENCRYPTION_INLINE_CRYPT
+			sb->s_flags |= SB_INLINECRYPT;
+#else
+			f2fs_info(sbi, "inline encryption not supported");
+#endif
+			break;
 		case Opt_checkpoint_disable_cap_perc:
 			if (args->from && match_int(args, &arg))
 				return -EINVAL;
@@ -1624,6 +1633,8 @@ static void default_options(struct f2fs_sb_info *sbi)
 	F2FS_OPTION(sbi).compress_ext_cnt = 0;
 	F2FS_OPTION(sbi).bggc_mode = BGGC_MODE_ON;
 
+	sbi->sb->s_flags &= ~SB_INLINECRYPT;
+
 	set_opt(sbi, INLINE_XATTR);
 	set_opt(sbi, INLINE_DATA);
 	set_opt(sbi, INLINE_DENTRY);
@@ -2470,6 +2481,25 @@ static void f2fs_get_ino_and_lblk_bits(struct super_block *sb,
 	*lblk_bits_ret = 8 * sizeof(block_t);
 }
 
+static int f2fs_get_num_devices(struct super_block *sb)
+{
+	struct f2fs_sb_info *sbi = F2FS_SB(sb);
+
+	if (f2fs_is_multi_device(sbi))
+		return sbi->s_ndevs;
+	return 1;
+}
+
+static void f2fs_get_devices(struct super_block *sb,
+			     struct request_queue **devs)
+{
+	struct f2fs_sb_info *sbi = F2FS_SB(sb);
+	int i;
+
+	for (i = 0; i < sbi->s_ndevs; i++)
+		devs[i] = bdev_get_queue(FDEV(i).bdev);
+}
+
 static const struct fscrypt_operations f2fs_cryptops = {
 	.key_prefix		= "f2fs:",
 	.get_context		= f2fs_get_context,
@@ -2479,6 +2509,8 @@ static const struct fscrypt_operations f2fs_cryptops = {
 	.max_namelen		= F2FS_NAME_LEN,
 	.has_stable_inodes	= f2fs_has_stable_inodes,
 	.get_ino_and_lblk_bits	= f2fs_get_ino_and_lblk_bits,
+	.get_num_devices	= f2fs_get_num_devices,
+	.get_devices		= f2fs_get_devices,
 };
 #endif
 
-- 
2.27.0.290.gba653c62da-goog

Re: [PATCH 3/4] f2fs: add inline encryption support

[Jaegeuk Kim]

On 06/17, Satya Tangirala wrote:
> Wire up f2fs to support inline encryption via the helper functions which
> fs/crypto/ now provides.  This includes:
> 
> - Adding a mount option 'inlinecrypt' which enables inline encryption
>   on encrypted files where it can be used.
> 
> - Setting the bio_crypt_ctx on bios that will be submitted to an
>   inline-encrypted file.
> 
> - Not adding logically discontiguous data to bios that will be submitted
>   to an inline-encrypted file.
> 
> - Not doing filesystem-layer crypto on inline-encrypted files.
> 
> This patch includes a fix for a race during IPU by
> Sahitya Tummala <stummala@codeaurora.org>
> 
> Co-developed-by: Eric Biggers <ebiggers@google.com>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> Signed-off-by: Satya Tangirala <satyat@google.com>

Acked-by: Jaegeuk Kim <jaegeuk@kernel.org>

> ---
>  Documentation/filesystems/f2fs.rst |  7 ++-
>  fs/f2fs/compress.c                 |  2 +-
>  fs/f2fs/data.c                     | 81 ++++++++++++++++++++++++------
>  fs/f2fs/super.c                    | 32 ++++++++++++
>  4 files changed, 104 insertions(+), 18 deletions(-)
> 
> diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
> index 099d45ac8d8f..4dc36143ff82 100644
> --- a/Documentation/filesystems/f2fs.rst
> +++ b/Documentation/filesystems/f2fs.rst
> @@ -258,7 +258,12 @@ compress_extension=%s  Support adding specified extension, so that f2fs can enab
>                         on compression extension list and enable compression on
>                         these file by default rather than to enable it via ioctl.
>                         For other files, we can still enable compression via ioctl.
> -====================== ============================================================
> +inlinecrypt
> +                       Encrypt/decrypt the contents of encrypted files using the
> +                       blk-crypto framework rather than filesystem-layer encryption.
> +                       This allows the use of inline encryption hardware. The on-disk
> +                       format is unaffected. For more details, see
> +                       Documentation/block/inline-encryption.rst.
>  
>  Debugfs Entries
>  ===============
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index 1e02a8c106b0..29e50fbe7eca 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -1086,7 +1086,7 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
>  		.submitted = false,
>  		.io_type = io_type,
>  		.io_wbc = wbc,
> -		.encrypted = f2fs_encrypted_file(cc->inode),
> +		.encrypted = fscrypt_inode_uses_fs_layer_crypto(cc->inode),
>  	};
>  	struct dnode_of_data dn;
>  	struct node_info ni;
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 326c63879ddc..6955ea6fa1b6 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -14,6 +14,7 @@
>  #include <linux/pagevec.h>
>  #include <linux/blkdev.h>
>  #include <linux/bio.h>
> +#include <linux/blk-crypto.h>
>  #include <linux/swap.h>
>  #include <linux/prefetch.h>
>  #include <linux/uio.h>
> @@ -459,6 +460,33 @@ static struct bio *__bio_alloc(struct f2fs_io_info *fio, int npages)
>  	return bio;
>  }
>  
> +static void f2fs_set_bio_crypt_ctx(struct bio *bio, const struct inode *inode,
> +				  pgoff_t first_idx,
> +				  const struct f2fs_io_info *fio,
> +				  gfp_t gfp_mask)
> +{
> +	/*
> +	 * The f2fs garbage collector sets ->encrypted_page when it wants to
> +	 * read/write raw data without encryption.
> +	 */
> +	if (!fio || !fio->encrypted_page)
> +		fscrypt_set_bio_crypt_ctx(bio, inode, first_idx, gfp_mask);
> +}
> +
> +static bool f2fs_crypt_mergeable_bio(struct bio *bio, const struct inode *inode,
> +				     pgoff_t next_idx,
> +				     const struct f2fs_io_info *fio)
> +{
> +	/*
> +	 * The f2fs garbage collector sets ->encrypted_page when it wants to
> +	 * read/write raw data without encryption.
> +	 */
> +	if (fio && fio->encrypted_page)
> +		return !bio_has_crypt_ctx(bio);
> +
> +	return fscrypt_mergeable_bio(bio, inode, next_idx);
> +}
> +
>  static inline void __submit_bio(struct f2fs_sb_info *sbi,
>  				struct bio *bio, enum page_type type)
>  {
> @@ -684,6 +712,9 @@ int f2fs_submit_page_bio(struct f2fs_io_info *fio)
>  	/* Allocate a new bio */
>  	bio = __bio_alloc(fio, 1);
>  
> +	f2fs_set_bio_crypt_ctx(bio, fio->page->mapping->host,
> +			       fio->page->index, fio, GFP_NOIO);
> +
>  	if (bio_add_page(bio, page, PAGE_SIZE, 0) < PAGE_SIZE) {
>  		bio_put(bio);
>  		return -EFAULT;
> @@ -763,9 +794,10 @@ static void del_bio_entry(struct bio_entry *be)
>  	kmem_cache_free(bio_entry_slab, be);
>  }
>  
> -static int add_ipu_page(struct f2fs_sb_info *sbi, struct bio **bio,
> +static int add_ipu_page(struct f2fs_io_info *fio, struct bio **bio,
>  							struct page *page)
>  {
> +	struct f2fs_sb_info *sbi = fio->sbi;
>  	enum temp_type temp;
>  	bool found = false;
>  	int ret = -EAGAIN;
> @@ -782,13 +814,18 @@ static int add_ipu_page(struct f2fs_sb_info *sbi, struct bio **bio,
>  
>  			found = true;
>  
> -			if (bio_add_page(*bio, page, PAGE_SIZE, 0) ==
> -							PAGE_SIZE) {
> +			if (page_is_mergeable(sbi, *bio, *fio->last_block,
> +					fio->new_blkaddr) &&
> +			    f2fs_crypt_mergeable_bio(*bio,
> +					fio->page->mapping->host,
> +					fio->page->index, fio) &&
> +			    bio_add_page(*bio, page, PAGE_SIZE, 0) ==
> +					PAGE_SIZE) {
>  				ret = 0;
>  				break;
>  			}
>  
> -			/* bio is full */
> +			 /* page can't be merged into bio; submit the bio */
>  			del_bio_entry(be);
>  			__submit_bio(sbi, *bio, DATA);
>  			break;
> @@ -873,18 +910,17 @@ int f2fs_merge_page_bio(struct f2fs_io_info *fio)
>  	trace_f2fs_submit_page_bio(page, fio);
>  	f2fs_trace_ios(fio, 0);
>  
> -	if (bio && !page_is_mergeable(fio->sbi, bio, *fio->last_block,
> -						fio->new_blkaddr))
> -		f2fs_submit_merged_ipu_write(fio->sbi, &bio, NULL);
>  alloc_new:
>  	if (!bio) {
>  		bio = __bio_alloc(fio, BIO_MAX_PAGES);
>  		__attach_io_flag(fio);
> +		f2fs_set_bio_crypt_ctx(bio, fio->page->mapping->host,
> +				       fio->page->index, fio, GFP_NOIO);
>  		bio_set_op_attrs(bio, fio->op, fio->op_flags);
>  
>  		add_bio_entry(fio->sbi, bio, page, fio->temp);
>  	} else {
> -		if (add_ipu_page(fio->sbi, &bio, page))
> +		if (add_ipu_page(fio, &bio, page))
>  			goto alloc_new;
>  	}
>  
> @@ -936,8 +972,11 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
>  
>  	inc_page_count(sbi, WB_DATA_TYPE(bio_page));
>  
> -	if (io->bio && !io_is_mergeable(sbi, io->bio, io, fio,
> -			io->last_block_in_bio, fio->new_blkaddr))
> +	if (io->bio &&
> +	    (!io_is_mergeable(sbi, io->bio, io, fio, io->last_block_in_bio,
> +			      fio->new_blkaddr) ||
> +	     !f2fs_crypt_mergeable_bio(io->bio, fio->page->mapping->host,
> +				       fio->page->index, fio)))
>  		__submit_merged_bio(io);
>  alloc_new:
>  	if (io->bio == NULL) {
> @@ -949,6 +988,8 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
>  			goto skip;
>  		}
>  		io->bio = __bio_alloc(fio, BIO_MAX_PAGES);
> +		f2fs_set_bio_crypt_ctx(io->bio, fio->page->mapping->host,
> +				       fio->page->index, fio, GFP_NOIO);
>  		io->fio = *fio;
>  	}
>  
> @@ -993,11 +1034,14 @@ static struct bio *f2fs_grab_read_bio(struct inode *inode, block_t blkaddr,
>  								for_write);
>  	if (!bio)
>  		return ERR_PTR(-ENOMEM);
> +
> +	f2fs_set_bio_crypt_ctx(bio, inode, first_idx, NULL, GFP_NOFS);
> +
>  	f2fs_target_device(sbi, blkaddr, bio);
>  	bio->bi_end_io = f2fs_read_end_io;
>  	bio_set_op_attrs(bio, REQ_OP_READ, op_flag);
>  
> -	if (f2fs_encrypted_file(inode))
> +	if (fscrypt_inode_uses_fs_layer_crypto(inode))
>  		post_read_steps |= 1 << STEP_DECRYPT;
>  	if (f2fs_compressed_file(inode))
>  		post_read_steps |= 1 << STEP_DECOMPRESS_NOWQ;
> @@ -2073,8 +2117,9 @@ static int f2fs_read_single_page(struct inode *inode, struct page *page,
>  	 * This page will go to BIO.  Do we need to send this
>  	 * BIO off first?
>  	 */
> -	if (bio && !page_is_mergeable(F2FS_I_SB(inode), bio,
> -				*last_block_in_bio, block_nr)) {
> +	if (bio && (!page_is_mergeable(F2FS_I_SB(inode), bio,
> +				       *last_block_in_bio, block_nr) ||
> +		    !f2fs_crypt_mergeable_bio(bio, inode, page->index, NULL))) {
>  submit_and_realloc:
>  		__submit_bio(F2FS_I_SB(inode), bio, DATA);
>  		bio = NULL;
> @@ -2204,8 +2249,9 @@ int f2fs_read_multi_pages(struct compress_ctx *cc, struct bio **bio_ret,
>  		blkaddr = data_blkaddr(dn.inode, dn.node_page,
>  						dn.ofs_in_node + i + 1);
>  
> -		if (bio && !page_is_mergeable(sbi, bio,
> -					*last_block_in_bio, blkaddr)) {
> +		if (bio && (!page_is_mergeable(sbi, bio,
> +					*last_block_in_bio, blkaddr) ||
> +		    !f2fs_crypt_mergeable_bio(bio, inode, page->index, NULL))) {
>  submit_and_realloc:
>  			__submit_bio(sbi, bio, DATA);
>  			bio = NULL;
> @@ -2421,6 +2467,9 @@ int f2fs_encrypt_one_page(struct f2fs_io_info *fio)
>  	/* wait for GCed page writeback via META_MAPPING */
>  	f2fs_wait_on_block_writeback(inode, fio->old_blkaddr);
>  
> +	if (fscrypt_inode_uses_inline_crypto(inode))
> +		return 0;
> +
>  retry_encrypt:
>  	fio->encrypted_page = fscrypt_encrypt_pagecache_blocks(page,
>  					PAGE_SIZE, 0, gfp_flags);
> @@ -2594,7 +2643,7 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
>  			f2fs_unlock_op(fio->sbi);
>  		err = f2fs_inplace_write_data(fio);
>  		if (err) {
> -			if (f2fs_encrypted_file(inode))
> +			if (fscrypt_inode_uses_fs_layer_crypto(inode))
>  				fscrypt_finalize_bounce_page(&fio->encrypted_page);
>  			if (PageWriteback(page))
>  				end_page_writeback(page);
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 20e56b0fa46a..3621969b2665 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -138,6 +138,7 @@ enum {
>  	Opt_alloc,
>  	Opt_fsync,
>  	Opt_test_dummy_encryption,
> +	Opt_inlinecrypt,
>  	Opt_checkpoint_disable,
>  	Opt_checkpoint_disable_cap,
>  	Opt_checkpoint_disable_cap_perc,
> @@ -204,6 +205,7 @@ static match_table_t f2fs_tokens = {
>  	{Opt_fsync, "fsync_mode=%s"},
>  	{Opt_test_dummy_encryption, "test_dummy_encryption=%s"},
>  	{Opt_test_dummy_encryption, "test_dummy_encryption"},
> +	{Opt_inlinecrypt, "inlinecrypt"},
>  	{Opt_checkpoint_disable, "checkpoint=disable"},
>  	{Opt_checkpoint_disable_cap, "checkpoint=disable:%u"},
>  	{Opt_checkpoint_disable_cap_perc, "checkpoint=disable:%u%%"},
> @@ -833,6 +835,13 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
>  			if (ret)
>  				return ret;
>  			break;
> +		case Opt_inlinecrypt:
> +#ifdef CONFIG_FS_ENCRYPTION_INLINE_CRYPT
> +			sb->s_flags |= SB_INLINECRYPT;
> +#else
> +			f2fs_info(sbi, "inline encryption not supported");
> +#endif
> +			break;
>  		case Opt_checkpoint_disable_cap_perc:
>  			if (args->from && match_int(args, &arg))
>  				return -EINVAL;
> @@ -1624,6 +1633,8 @@ static void default_options(struct f2fs_sb_info *sbi)
>  	F2FS_OPTION(sbi).compress_ext_cnt = 0;
>  	F2FS_OPTION(sbi).bggc_mode = BGGC_MODE_ON;
>  
> +	sbi->sb->s_flags &= ~SB_INLINECRYPT;
> +
>  	set_opt(sbi, INLINE_XATTR);
>  	set_opt(sbi, INLINE_DATA);
>  	set_opt(sbi, INLINE_DENTRY);
> @@ -2470,6 +2481,25 @@ static void f2fs_get_ino_and_lblk_bits(struct super_block *sb,
>  	*lblk_bits_ret = 8 * sizeof(block_t);
>  }
>  
> +static int f2fs_get_num_devices(struct super_block *sb)
> +{
> +	struct f2fs_sb_info *sbi = F2FS_SB(sb);
> +
> +	if (f2fs_is_multi_device(sbi))
> +		return sbi->s_ndevs;
> +	return 1;
> +}
> +
> +static void f2fs_get_devices(struct super_block *sb,
> +			     struct request_queue **devs)
> +{
> +	struct f2fs_sb_info *sbi = F2FS_SB(sb);
> +	int i;
> +
> +	for (i = 0; i < sbi->s_ndevs; i++)
> +		devs[i] = bdev_get_queue(FDEV(i).bdev);
> +}
> +
>  static const struct fscrypt_operations f2fs_cryptops = {
>  	.key_prefix		= "f2fs:",
>  	.get_context		= f2fs_get_context,
> @@ -2479,6 +2509,8 @@ static const struct fscrypt_operations f2fs_cryptops = {
>  	.max_namelen		= F2FS_NAME_LEN,
>  	.has_stable_inodes	= f2fs_has_stable_inodes,
>  	.get_ino_and_lblk_bits	= f2fs_get_ino_and_lblk_bits,
> +	.get_num_devices	= f2fs_get_num_devices,
> +	.get_devices		= f2fs_get_devices,
>  };
>  #endif
>  
> -- 
> 2.27.0.290.gba653c62da-goog

Re: [PATCH 3/4] f2fs: add inline encryption support

[Chao Yu]

On 2020/6/17 15:57, Satya Tangirala wrote:
> Wire up f2fs to support inline encryption via the helper functions which
> fs/crypto/ now provides.  This includes:
> 
> - Adding a mount option 'inlinecrypt' which enables inline encryption
>   on encrypted files where it can be used.
> 
> - Setting the bio_crypt_ctx on bios that will be submitted to an
>   inline-encrypted file.
> 
> - Not adding logically discontiguous data to bios that will be submitted
>   to an inline-encrypted file.
> 
> - Not doing filesystem-layer crypto on inline-encrypted files.
> 
> This patch includes a fix for a race during IPU by
> Sahitya Tummala <stummala@codeaurora.org>
> 
> Co-developed-by: Eric Biggers <ebiggers@google.com>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> Signed-off-by: Satya Tangirala <satyat@google.com>
> ---
>  Documentation/filesystems/f2fs.rst |  7 ++-
>  fs/f2fs/compress.c                 |  2 +-
>  fs/f2fs/data.c                     | 81 ++++++++++++++++++++++++------
>  fs/f2fs/super.c                    | 32 ++++++++++++
>  4 files changed, 104 insertions(+), 18 deletions(-)
> 
> diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
> index 099d45ac8d8f..4dc36143ff82 100644
> --- a/Documentation/filesystems/f2fs.rst
> +++ b/Documentation/filesystems/f2fs.rst
> @@ -258,7 +258,12 @@ compress_extension=%s  Support adding specified extension, so that f2fs can enab
>                         on compression extension list and enable compression on
>                         these file by default rather than to enable it via ioctl.
>                         For other files, we can still enable compression via ioctl.
> -====================== ============================================================
> +inlinecrypt
> +                       Encrypt/decrypt the contents of encrypted files using the
> +                       blk-crypto framework rather than filesystem-layer encryption.
> +                       This allows the use of inline encryption hardware. The on-disk
> +                       format is unaffected. For more details, see
> +                       Documentation/block/inline-encryption.rst.
>  
>  Debugfs Entries
>  ===============
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index 1e02a8c106b0..29e50fbe7eca 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -1086,7 +1086,7 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
>  		.submitted = false,
>  		.io_type = io_type,
>  		.io_wbc = wbc,
> -		.encrypted = f2fs_encrypted_file(cc->inode),
> +		.encrypted = fscrypt_inode_uses_fs_layer_crypto(cc->inode),
>  	};
>  	struct dnode_of_data dn;
>  	struct node_info ni;
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 326c63879ddc..6955ea6fa1b6 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -14,6 +14,7 @@
>  #include <linux/pagevec.h>
>  #include <linux/blkdev.h>
>  #include <linux/bio.h>
> +#include <linux/blk-crypto.h>
>  #include <linux/swap.h>
>  #include <linux/prefetch.h>
>  #include <linux/uio.h>
> @@ -459,6 +460,33 @@ static struct bio *__bio_alloc(struct f2fs_io_info *fio, int npages)
>  	return bio;
>  }
>  
> +static void f2fs_set_bio_crypt_ctx(struct bio *bio, const struct inode *inode,
> +				  pgoff_t first_idx,
> +				  const struct f2fs_io_info *fio,
> +				  gfp_t gfp_mask)
> +{
> +	/*
> +	 * The f2fs garbage collector sets ->encrypted_page when it wants to
> +	 * read/write raw data without encryption.
> +	 */
> +	if (!fio || !fio->encrypted_page)
> +		fscrypt_set_bio_crypt_ctx(bio, inode, first_idx, gfp_mask);
> +}
> +
> +static bool f2fs_crypt_mergeable_bio(struct bio *bio, const struct inode *inode,
> +				     pgoff_t next_idx,
> +				     const struct f2fs_io_info *fio)
> +{
> +	/*
> +	 * The f2fs garbage collector sets ->encrypted_page when it wants to
> +	 * read/write raw data without encryption.
> +	 */
> +	if (fio && fio->encrypted_page)
> +		return !bio_has_crypt_ctx(bio);
> +
> +	return fscrypt_mergeable_bio(bio, inode, next_idx);
> +}
> +
>  static inline void __submit_bio(struct f2fs_sb_info *sbi,
>  				struct bio *bio, enum page_type type)
>  {
> @@ -684,6 +712,9 @@ int f2fs_submit_page_bio(struct f2fs_io_info *fio)
>  	/* Allocate a new bio */
>  	bio = __bio_alloc(fio, 1);
>  
> +	f2fs_set_bio_crypt_ctx(bio, fio->page->mapping->host,
> +			       fio->page->index, fio, GFP_NOIO);
> +
>  	if (bio_add_page(bio, page, PAGE_SIZE, 0) < PAGE_SIZE) {
>  		bio_put(bio);
>  		return -EFAULT;
> @@ -763,9 +794,10 @@ static void del_bio_entry(struct bio_entry *be)
>  	kmem_cache_free(bio_entry_slab, be);
>  }
>  
> -static int add_ipu_page(struct f2fs_sb_info *sbi, struct bio **bio,
> +static int add_ipu_page(struct f2fs_io_info *fio, struct bio **bio,
>  							struct page *page)
>  {
> +	struct f2fs_sb_info *sbi = fio->sbi;
>  	enum temp_type temp;
>  	bool found = false;
>  	int ret = -EAGAIN;
> @@ -782,13 +814,18 @@ static int add_ipu_page(struct f2fs_sb_info *sbi, struct bio **bio,
>  
>  			found = true;
>  
> -			if (bio_add_page(*bio, page, PAGE_SIZE, 0) ==
> -							PAGE_SIZE) {
> +			if (page_is_mergeable(sbi, *bio, *fio->last_block,
> +					fio->new_blkaddr) &&
> +			    f2fs_crypt_mergeable_bio(*bio,
> +					fio->page->mapping->host,
> +					fio->page->index, fio) &&
> +			    bio_add_page(*bio, page, PAGE_SIZE, 0) ==
> +					PAGE_SIZE) {
>  				ret = 0;
>  				break;
>  			}
>  
> -			/* bio is full */
> +			 /* page can't be merged into bio; submit the bio */

One more unneeded space before '/'.

>  			del_bio_entry(be);
>  			__submit_bio(sbi, *bio, DATA);
>  			break;
> @@ -873,18 +910,17 @@ int f2fs_merge_page_bio(struct f2fs_io_info *fio)
>  	trace_f2fs_submit_page_bio(page, fio);
>  	f2fs_trace_ios(fio, 0);
>  
> -	if (bio && !page_is_mergeable(fio->sbi, bio, *fio->last_block,
> -						fio->new_blkaddr))
> -		f2fs_submit_merged_ipu_write(fio->sbi, &bio, NULL);

I prefer to keep this condition for non-inlinecrypt case to avoid unneeded lock
contention in add_ipu_page().

>  alloc_new:
>  	if (!bio) {
>  		bio = __bio_alloc(fio, BIO_MAX_PAGES);
>  		__attach_io_flag(fio);
> +		f2fs_set_bio_crypt_ctx(bio, fio->page->mapping->host,
> +				       fio->page->index, fio, GFP_NOIO);
>  		bio_set_op_attrs(bio, fio->op, fio->op_flags);
>  
>  		add_bio_entry(fio->sbi, bio, page, fio->temp);
>  	} else {
> -		if (add_ipu_page(fio->sbi, &bio, page))
> +		if (add_ipu_page(fio, &bio, page))
>  			goto alloc_new;
>  	}
>  
> @@ -936,8 +972,11 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
>  
>  	inc_page_count(sbi, WB_DATA_TYPE(bio_page));
>  
> -	if (io->bio && !io_is_mergeable(sbi, io->bio, io, fio,
> -			io->last_block_in_bio, fio->new_blkaddr))
> +	if (io->bio &&
> +	    (!io_is_mergeable(sbi, io->bio, io, fio, io->last_block_in_bio,
> +			      fio->new_blkaddr) ||
> +	     !f2fs_crypt_mergeable_bio(io->bio, fio->page->mapping->host,
> +				       fio->page->index, fio)))

bio_page->index, fio)))

>  		__submit_merged_bio(io);
>  alloc_new:
>  	if (io->bio == NULL) {
> @@ -949,6 +988,8 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
>  			goto skip;
>  		}
>  		io->bio = __bio_alloc(fio, BIO_MAX_PAGES);
> +		f2fs_set_bio_crypt_ctx(io->bio, fio->page->mapping->host,
> +				       fio->page->index, fio, GFP_NOIO);

bio_page->index, fio, GFP_NOIO);

Thanks,

>  		io->fio = *fio;
>  	}
>  
> @@ -993,11 +1034,14 @@ static struct bio *f2fs_grab_read_bio(struct inode *inode, block_t blkaddr,
>  								for_write);
>  	if (!bio)
>  		return ERR_PTR(-ENOMEM);
> +
> +	f2fs_set_bio_crypt_ctx(bio, inode, first_idx, NULL, GFP_NOFS);
> +
>  	f2fs_target_device(sbi, blkaddr, bio);
>  	bio->bi_end_io = f2fs_read_end_io;
>  	bio_set_op_attrs(bio, REQ_OP_READ, op_flag);
>  
> -	if (f2fs_encrypted_file(inode))
> +	if (fscrypt_inode_uses_fs_layer_crypto(inode))
>  		post_read_steps |= 1 << STEP_DECRYPT;
>  	if (f2fs_compressed_file(inode))
>  		post_read_steps |= 1 << STEP_DECOMPRESS_NOWQ;
> @@ -2073,8 +2117,9 @@ static int f2fs_read_single_page(struct inode *inode, struct page *page,
>  	 * This page will go to BIO.  Do we need to send this
>  	 * BIO off first?
>  	 */
> -	if (bio && !page_is_mergeable(F2FS_I_SB(inode), bio,
> -				*last_block_in_bio, block_nr)) {
> +	if (bio && (!page_is_mergeable(F2FS_I_SB(inode), bio,
> +				       *last_block_in_bio, block_nr) ||
> +		    !f2fs_crypt_mergeable_bio(bio, inode, page->index, NULL))) {
>  submit_and_realloc:
>  		__submit_bio(F2FS_I_SB(inode), bio, DATA);
>  		bio = NULL;
> @@ -2204,8 +2249,9 @@ int f2fs_read_multi_pages(struct compress_ctx *cc, struct bio **bio_ret,
>  		blkaddr = data_blkaddr(dn.inode, dn.node_page,
>  						dn.ofs_in_node + i + 1);
>  
> -		if (bio && !page_is_mergeable(sbi, bio,
> -					*last_block_in_bio, blkaddr)) {
> +		if (bio && (!page_is_mergeable(sbi, bio,
> +					*last_block_in_bio, blkaddr) ||
> +		    !f2fs_crypt_mergeable_bio(bio, inode, page->index, NULL))) {
>  submit_and_realloc:
>  			__submit_bio(sbi, bio, DATA);
>  			bio = NULL;
> @@ -2421,6 +2467,9 @@ int f2fs_encrypt_one_page(struct f2fs_io_info *fio)
>  	/* wait for GCed page writeback via META_MAPPING */
>  	f2fs_wait_on_block_writeback(inode, fio->old_blkaddr);
>  
> +	if (fscrypt_inode_uses_inline_crypto(inode))
> +		return 0;
> +
>  retry_encrypt:
>  	fio->encrypted_page = fscrypt_encrypt_pagecache_blocks(page,
>  					PAGE_SIZE, 0, gfp_flags);
> @@ -2594,7 +2643,7 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
>  			f2fs_unlock_op(fio->sbi);
>  		err = f2fs_inplace_write_data(fio);
>  		if (err) {
> -			if (f2fs_encrypted_file(inode))
> +			if (fscrypt_inode_uses_fs_layer_crypto(inode))
>  				fscrypt_finalize_bounce_page(&fio->encrypted_page);
>  			if (PageWriteback(page))
>  				end_page_writeback(page);
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 20e56b0fa46a..3621969b2665 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -138,6 +138,7 @@ enum {
>  	Opt_alloc,
>  	Opt_fsync,
>  	Opt_test_dummy_encryption,
> +	Opt_inlinecrypt,
>  	Opt_checkpoint_disable,
>  	Opt_checkpoint_disable_cap,
>  	Opt_checkpoint_disable_cap_perc,
> @@ -204,6 +205,7 @@ static match_table_t f2fs_tokens = {
>  	{Opt_fsync, "fsync_mode=%s"},
>  	{Opt_test_dummy_encryption, "test_dummy_encryption=%s"},
>  	{Opt_test_dummy_encryption, "test_dummy_encryption"},
> +	{Opt_inlinecrypt, "inlinecrypt"},
>  	{Opt_checkpoint_disable, "checkpoint=disable"},
>  	{Opt_checkpoint_disable_cap, "checkpoint=disable:%u"},
>  	{Opt_checkpoint_disable_cap_perc, "checkpoint=disable:%u%%"},
> @@ -833,6 +835,13 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
>  			if (ret)
>  				return ret;
>  			break;
> +		case Opt_inlinecrypt:
> +#ifdef CONFIG_FS_ENCRYPTION_INLINE_CRYPT
> +			sb->s_flags |= SB_INLINECRYPT;
> +#else
> +			f2fs_info(sbi, "inline encryption not supported");
> +#endif
> +			break;
>  		case Opt_checkpoint_disable_cap_perc:
>  			if (args->from && match_int(args, &arg))
>  				return -EINVAL;
> @@ -1624,6 +1633,8 @@ static void default_options(struct f2fs_sb_info *sbi)
>  	F2FS_OPTION(sbi).compress_ext_cnt = 0;
>  	F2FS_OPTION(sbi).bggc_mode = BGGC_MODE_ON;
>  
> +	sbi->sb->s_flags &= ~SB_INLINECRYPT;
> +
>  	set_opt(sbi, INLINE_XATTR);
>  	set_opt(sbi, INLINE_DATA);
>  	set_opt(sbi, INLINE_DENTRY);
> @@ -2470,6 +2481,25 @@ static void f2fs_get_ino_and_lblk_bits(struct super_block *sb,
>  	*lblk_bits_ret = 8 * sizeof(block_t);
>  }
>  
> +static int f2fs_get_num_devices(struct super_block *sb)
> +{
> +	struct f2fs_sb_info *sbi = F2FS_SB(sb);
> +
> +	if (f2fs_is_multi_device(sbi))
> +		return sbi->s_ndevs;
> +	return 1;
> +}
> +
> +static void f2fs_get_devices(struct super_block *sb,
> +			     struct request_queue **devs)
> +{
> +	struct f2fs_sb_info *sbi = F2FS_SB(sb);
> +	int i;
> +
> +	for (i = 0; i < sbi->s_ndevs; i++)
> +		devs[i] = bdev_get_queue(FDEV(i).bdev);
> +}
> +
>  static const struct fscrypt_operations f2fs_cryptops = {
>  	.key_prefix		= "f2fs:",
>  	.get_context		= f2fs_get_context,
> @@ -2479,6 +2509,8 @@ static const struct fscrypt_operations f2fs_cryptops = {
>  	.max_namelen		= F2FS_NAME_LEN,
>  	.has_stable_inodes	= f2fs_has_stable_inodes,
>  	.get_ino_and_lblk_bits	= f2fs_get_ino_and_lblk_bits,
> +	.get_num_devices	= f2fs_get_num_devices,
> +	.get_devices		= f2fs_get_devices,
>  };
>  #endif
>  
> 

Re: [PATCH 3/4] f2fs: add inline encryption support

[Eric Biggers]

Hi Chao,

On Thu, Jun 18, 2020 at 06:06:02PM +0800, Chao Yu wrote:
> > @@ -936,8 +972,11 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
> >  
> >  	inc_page_count(sbi, WB_DATA_TYPE(bio_page));
> >  
> > -	if (io->bio && !io_is_mergeable(sbi, io->bio, io, fio,
> > -			io->last_block_in_bio, fio->new_blkaddr))
> > +	if (io->bio &&
> > +	    (!io_is_mergeable(sbi, io->bio, io, fio, io->last_block_in_bio,
> > +			      fio->new_blkaddr) ||
> > +	     !f2fs_crypt_mergeable_bio(io->bio, fio->page->mapping->host,
> > +				       fio->page->index, fio)))
> 
> bio_page->index, fio)))
> 
> >  		__submit_merged_bio(io);
> >  alloc_new:
> >  	if (io->bio == NULL) {
> > @@ -949,6 +988,8 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
> >  			goto skip;
> >  		}
> >  		io->bio = __bio_alloc(fio, BIO_MAX_PAGES);
> > +		f2fs_set_bio_crypt_ctx(io->bio, fio->page->mapping->host,
> > +				       fio->page->index, fio, GFP_NOIO);
> 
> bio_page->index, fio, GFP_NOIO);
> 

We're using ->mapping->host and ->index.  Ordinarily that would mean the page
needs to be a pagecache page.  But bio_page can also be a compressed page or a
bounce page containing fs-layer encrypted contents.

Is your suggestion to keep using fio->page->mapping->host (since encrypted pages
don't have a mapping), but start using bio_page->index (since f2fs apparently
*does* set ->index for compressed pages, and if the file uses fs-layer
encryption then f2fs_set_bio_crypt_ctx() won't use the index anyway)?

Does this mean the code is currently broken for compression + inline encryption
because it's using the wrong ->index?  I think the answer is no, since
f2fs_write_compressed_pages() will still pass the first 'nr_cpages' pagecache
pages along with the compressed pages.  In that case, your suggestion would be a
cleanup rather than a fix?

It would be helpful if there was an f2fs mount option to auto-enable compression
on all files (similar to how test_dummy_encryption auto-enables encryption on
all files) so that it could be tested more easily.

- Eric

Re: [PATCH 3/4] f2fs: add inline encryption support

[Jaegeuk Kim]

On 06/18, Eric Biggers wrote:
> Hi Chao,
> 
> On Thu, Jun 18, 2020 at 06:06:02PM +0800, Chao Yu wrote:
> > > @@ -936,8 +972,11 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
> > >  
> > >  	inc_page_count(sbi, WB_DATA_TYPE(bio_page));
> > >  
> > > -	if (io->bio && !io_is_mergeable(sbi, io->bio, io, fio,
> > > -			io->last_block_in_bio, fio->new_blkaddr))
> > > +	if (io->bio &&
> > > +	    (!io_is_mergeable(sbi, io->bio, io, fio, io->last_block_in_bio,
> > > +			      fio->new_blkaddr) ||
> > > +	     !f2fs_crypt_mergeable_bio(io->bio, fio->page->mapping->host,
> > > +				       fio->page->index, fio)))
> > 
> > bio_page->index, fio)))
> > 
> > >  		__submit_merged_bio(io);
> > >  alloc_new:
> > >  	if (io->bio == NULL) {
> > > @@ -949,6 +988,8 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
> > >  			goto skip;
> > >  		}
> > >  		io->bio = __bio_alloc(fio, BIO_MAX_PAGES);
> > > +		f2fs_set_bio_crypt_ctx(io->bio, fio->page->mapping->host,
> > > +				       fio->page->index, fio, GFP_NOIO);
> > 
> > bio_page->index, fio, GFP_NOIO);
> > 
> 
> We're using ->mapping->host and ->index.  Ordinarily that would mean the page
> needs to be a pagecache page.  But bio_page can also be a compressed page or a
> bounce page containing fs-layer encrypted contents.
> 
> Is your suggestion to keep using fio->page->mapping->host (since encrypted pages
> don't have a mapping), but start using bio_page->index (since f2fs apparently
> *does* set ->index for compressed pages, and if the file uses fs-layer
> encryption then f2fs_set_bio_crypt_ctx() won't use the index anyway)?
> 
> Does this mean the code is currently broken for compression + inline encryption
> because it's using the wrong ->index?  I think the answer is no, since
> f2fs_write_compressed_pages() will still pass the first 'nr_cpages' pagecache
> pages along with the compressed pages.  In that case, your suggestion would be a
> cleanup rather than a fix?
> 
> It would be helpful if there was an f2fs mount option to auto-enable compression
> on all files (similar to how test_dummy_encryption auto-enables encryption on
> all files) so that it could be tested more easily.

Eric, you can use "-o compress_extension=*".

> 
> - Eric

Re: [PATCH 3/4] f2fs: add inline encryption support

[Eric Biggers]

On Thu, Jun 18, 2020 at 12:28:04PM -0700, Jaegeuk Kim wrote:
> > 
> > It would be helpful if there was an f2fs mount option to auto-enable compression
> > on all files (similar to how test_dummy_encryption auto-enables encryption on
> > all files) so that it could be tested more easily.
> 
> Eric, you can use "-o compress_extension=*".
> 

Okay, great!  This isn't mentioned in the documentation:

compress_extension=%s  Support adding specified extension, so that f2fs can enable
                       compression on those corresponding files, e.g. if all files
                       with '.ext' has high compression rate, we can set the '.ext'
                       on compression extension list and enable compression on
                       these file by default rather than to enable it via ioctl.
                       For other files, we can still enable compression via ioctl.

Re: [PATCH 3/4] f2fs: add inline encryption support

[Chao Yu]

On 2020/6/19 3:28, Jaegeuk Kim wrote:
> On 06/18, Eric Biggers wrote:
>> Hi Chao,
>>
>> On Thu, Jun 18, 2020 at 06:06:02PM +0800, Chao Yu wrote:
>>>> @@ -936,8 +972,11 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
>>>>  
>>>>  	inc_page_count(sbi, WB_DATA_TYPE(bio_page));
>>>>  
>>>> -	if (io->bio && !io_is_mergeable(sbi, io->bio, io, fio,
>>>> -			io->last_block_in_bio, fio->new_blkaddr))
>>>> +	if (io->bio &&
>>>> +	    (!io_is_mergeable(sbi, io->bio, io, fio, io->last_block_in_bio,
>>>> +			      fio->new_blkaddr) ||
>>>> +	     !f2fs_crypt_mergeable_bio(io->bio, fio->page->mapping->host,
>>>> +				       fio->page->index, fio)))
>>>
>>> bio_page->index, fio)))
>>>
>>>>  		__submit_merged_bio(io);
>>>>  alloc_new:
>>>>  	if (io->bio == NULL) {
>>>> @@ -949,6 +988,8 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
>>>>  			goto skip;
>>>>  		}
>>>>  		io->bio = __bio_alloc(fio, BIO_MAX_PAGES);
>>>> +		f2fs_set_bio_crypt_ctx(io->bio, fio->page->mapping->host,
>>>> +				       fio->page->index, fio, GFP_NOIO);
>>>
>>> bio_page->index, fio, GFP_NOIO);
>>>
>>
>> We're using ->mapping->host and ->index.  Ordinarily that would mean the page
>> needs to be a pagecache page.  But bio_page can also be a compressed page or a
>> bounce page containing fs-layer encrypted contents.
>>
>> Is your suggestion to keep using fio->page->mapping->host (since encrypted pages
>> don't have a mapping), but start using bio_page->index (since f2fs apparently
>> *does* set ->index for compressed pages, and if the file uses fs-layer
>> encryption then f2fs_set_bio_crypt_ctx() won't use the index anyway)?
>>
>> Does this mean the code is currently broken for compression + inline encryption
>> because it's using the wrong ->index?  I think the answer is no, since
>> f2fs_write_compressed_pages() will still pass the first 'nr_cpages' pagecache
>> pages along with the compressed pages.  In that case, your suggestion would be a
>> cleanup rather than a fix?
>>
>> It would be helpful if there was an f2fs mount option to auto-enable compression
>> on all files (similar to how test_dummy_encryption auto-enables encryption on
>> all files) so that it could be tested more easily.
> 
> Eric, you can use "-o compress_extension=*".

Cool, we should update documentation when merge that patch...

> 
>>
>> - Eric
> .
> 

Re: [PATCH 3/4] f2fs: add inline encryption support

[Chao Yu]

Hi Eric,

On 2020/6/19 2:13, Eric Biggers wrote:
> Hi Chao,
> 
> On Thu, Jun 18, 2020 at 06:06:02PM +0800, Chao Yu wrote:
>>> @@ -936,8 +972,11 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
>>>  
>>>  	inc_page_count(sbi, WB_DATA_TYPE(bio_page));
>>>  
>>> -	if (io->bio && !io_is_mergeable(sbi, io->bio, io, fio,
>>> -			io->last_block_in_bio, fio->new_blkaddr))
>>> +	if (io->bio &&
>>> +	    (!io_is_mergeable(sbi, io->bio, io, fio, io->last_block_in_bio,
>>> +			      fio->new_blkaddr) ||
>>> +	     !f2fs_crypt_mergeable_bio(io->bio, fio->page->mapping->host,
>>> +				       fio->page->index, fio)))
>>
>> bio_page->index, fio)))
>>
>>>  		__submit_merged_bio(io);
>>>  alloc_new:
>>>  	if (io->bio == NULL) {
>>> @@ -949,6 +988,8 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
>>>  			goto skip;
>>>  		}
>>>  		io->bio = __bio_alloc(fio, BIO_MAX_PAGES);
>>> +		f2fs_set_bio_crypt_ctx(io->bio, fio->page->mapping->host,
>>> +				       fio->page->index, fio, GFP_NOIO);
>>
>> bio_page->index, fio, GFP_NOIO);
>>
> 
> We're using ->mapping->host and ->index.  Ordinarily that would mean the page
> needs to be a pagecache page.  But bio_page can also be a compressed page or a
> bounce page containing fs-layer encrypted contents.

I'm concerning about compression + inlinecrypt case.

> 
> Is your suggestion to keep using fio->page->mapping->host (since encrypted pages

Yup,

> don't have a mapping), but start using bio_page->index (since f2fs apparently

I meant that we need to use bio_page->index as tweak value in write path to
keep consistent as we did in read path, otherwise we may read the wrong
decrypted data later to incorrect tweak value.

- f2fs_read_multi_pages (only comes from compression inode)
 - f2fs_alloc_dic
  - f2fs_set_compressed_page(page, cc->inode,
					start_idx + i + 1, dic);
                                        ^^^^^^^^^^^^^^^^^
  - dic->cpages[i] = page;
 - for ()
     struct page *page = dic->cpages[i];
     if (!bio)
       - f2fs_grab_read_bio(..., page->index,..)
        - f2fs_set_bio_crypt_ctx(..., first_idx, ..)   /* first_idx == cpage->index */

You can see that cpage->index was set to page->index + 1, that's why we need
to use one of cpage->index/page->index as tweak value all the time rather than
using both index mixed in read/write path.

But note that for fs-layer encryption, we have used cpage->index as tweak value,
so here I suggest we can keep consistent to use cpage->index in inlinecrypt case.

> *does* set ->index for compressed pages, and if the file uses fs-layer
> encryption then f2fs_set_bio_crypt_ctx() won't use the index anyway)?
> 
> Does this mean the code is currently broken for compression + inline encryption
> because it's using the wrong ->index?  I think the answer is no, since

I guess it's broken now for compression + inlinecrypt case.

> f2fs_write_compressed_pages() will still pass the first 'nr_cpages' pagecache
> pages along with the compressed pages.  In that case, your suggestion would be a
> cleanup rather than a fix?

That's a fix.

> 
> It would be helpful if there was an f2fs mount option to auto-enable compression
> on all files (similar to how test_dummy_encryption auto-enables encryption on
> all files) so that it could be tested more easily.

Agreed.

Previously I changed mkfs to allow to add compression flag to root inode for
compression test. :P

Thanks,

> 
> - Eric
> .
> 

Re: [PATCH 3/4] f2fs: add inline encryption support

[Eric Biggers]

On Fri, Jun 19, 2020 at 10:39:34AM +0800, Chao Yu wrote:
> Hi Eric,
> 
> On 2020/6/19 2:13, Eric Biggers wrote:
> > Hi Chao,
> > 
> > On Thu, Jun 18, 2020 at 06:06:02PM +0800, Chao Yu wrote:
> >>> @@ -936,8 +972,11 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
> >>>  
> >>>  	inc_page_count(sbi, WB_DATA_TYPE(bio_page));
> >>>  
> >>> -	if (io->bio && !io_is_mergeable(sbi, io->bio, io, fio,
> >>> -			io->last_block_in_bio, fio->new_blkaddr))
> >>> +	if (io->bio &&
> >>> +	    (!io_is_mergeable(sbi, io->bio, io, fio, io->last_block_in_bio,
> >>> +			      fio->new_blkaddr) ||
> >>> +	     !f2fs_crypt_mergeable_bio(io->bio, fio->page->mapping->host,
> >>> +				       fio->page->index, fio)))
> >>
> >> bio_page->index, fio)))
> >>
> >>>  		__submit_merged_bio(io);
> >>>  alloc_new:
> >>>  	if (io->bio == NULL) {
> >>> @@ -949,6 +988,8 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
> >>>  			goto skip;
> >>>  		}
> >>>  		io->bio = __bio_alloc(fio, BIO_MAX_PAGES);
> >>> +		f2fs_set_bio_crypt_ctx(io->bio, fio->page->mapping->host,
> >>> +				       fio->page->index, fio, GFP_NOIO);
> >>
> >> bio_page->index, fio, GFP_NOIO);
> >>
> > 
> > We're using ->mapping->host and ->index.  Ordinarily that would mean the page
> > needs to be a pagecache page.  But bio_page can also be a compressed page or a
> > bounce page containing fs-layer encrypted contents.
> 
> I'm concerning about compression + inlinecrypt case.
> 
> > 
> > Is your suggestion to keep using fio->page->mapping->host (since encrypted pages
> 
> Yup,
> 
> > don't have a mapping), but start using bio_page->index (since f2fs apparently
> 
> I meant that we need to use bio_page->index as tweak value in write path to
> keep consistent as we did in read path, otherwise we may read the wrong
> decrypted data later to incorrect tweak value.
> 
> - f2fs_read_multi_pages (only comes from compression inode)
>  - f2fs_alloc_dic
>   - f2fs_set_compressed_page(page, cc->inode,
> 					start_idx + i + 1, dic);
>                                         ^^^^^^^^^^^^^^^^^
>   - dic->cpages[i] = page;
>  - for ()
>      struct page *page = dic->cpages[i];
>      if (!bio)
>        - f2fs_grab_read_bio(..., page->index,..)
>         - f2fs_set_bio_crypt_ctx(..., first_idx, ..)   /* first_idx == cpage->index */
> 
> You can see that cpage->index was set to page->index + 1, that's why we need
> to use one of cpage->index/page->index as tweak value all the time rather than
> using both index mixed in read/write path.
> 
> But note that for fs-layer encryption, we have used cpage->index as tweak value,
> so here I suggest we can keep consistent to use cpage->index in inlinecrypt case.

Yes, inlinecrypt mustn't change the ciphertext that gets written to disk.

> 
> > *does* set ->index for compressed pages, and if the file uses fs-layer
> > encryption then f2fs_set_bio_crypt_ctx() won't use the index anyway)?
> > 
> > Does this mean the code is currently broken for compression + inline encryption
> > because it's using the wrong ->index?  I think the answer is no, since
> 
> I guess it's broken now for compression + inlinecrypt case.
> 
> > f2fs_write_compressed_pages() will still pass the first 'nr_cpages' pagecache
> > pages along with the compressed pages.  In that case, your suggestion would be a
> > cleanup rather than a fix?
> 
> That's a fix.
> 

FWIW, I tested this, and it actually works both before and after your suggested
change.  The reason is that f2fs_write_compressed_pages() actually passes the
pagecache pages sequentially starting at 'start_idx_of_cluster(cc) + 1' for the
length of the compressed cluster.  That matches the '+ 1' adjustment elsewhere,
so we have fio->page->index == bio_page->index.

I personally think the way the f2fs compression code works is really confusing.
Compressed pages don't have a 1:1 correspondence to pagecache pages, so there
should *not* be a pagecache page passed around when writing a compressed page.

Anyway, here's the test script I used in case anyone else wants to use it.  But
we really need to write a proper f2fs compression + encryption test for xfstests
which decrypts and decompresses a file in userspace and verifies we get back the
original data.  (There are already ciphertext verification tests, but they don't
cover compression.)  Note that this test is needed even for the filesystem-layer
encryption which is currently supported.

#!/bin/bash

set -e

DEV=/dev/vdb

umount /mnt &> /dev/null || true
mkfs.f2fs -f -O encrypt,compression,extra_attr $DEV
head -c 1000000 /dev/zero > /tmp/testdata

for opt1 in '-o inlinecrypt' ''; do
        mount $DEV /mnt $opt1
        rm -rf /mnt/.fscrypt /mnt/dir
        fscrypt setup /mnt
        mkdir /mnt/dir
        chattr +c /mnt/dir
        echo hunter2 | fscrypt encrypt /mnt/dir --quiet --source=custom_passphrase --name=secret
        cp /tmp/testdata /mnt/dir/file
        umount /mnt
        for opt2 in '-o inlinecrypt' ''; do
                mount $DEV /mnt $opt2
                echo hunter2 | fscrypt unlock /mnt/dir --quiet
                cmp /mnt/dir/file /tmp/testdata
                umount /mnt
        done
done

Re: [PATCH 3/4] f2fs: add inline encryption support

[Chao Yu]

On 2020/6/19 12:20, Eric Biggers wrote:
> On Fri, Jun 19, 2020 at 10:39:34AM +0800, Chao Yu wrote:
>> Hi Eric,
>>
>> On 2020/6/19 2:13, Eric Biggers wrote:
>>> Hi Chao,
>>>
>>> On Thu, Jun 18, 2020 at 06:06:02PM +0800, Chao Yu wrote:
>>>>> @@ -936,8 +972,11 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
>>>>>  
>>>>>  	inc_page_count(sbi, WB_DATA_TYPE(bio_page));
>>>>>  
>>>>> -	if (io->bio && !io_is_mergeable(sbi, io->bio, io, fio,
>>>>> -			io->last_block_in_bio, fio->new_blkaddr))
>>>>> +	if (io->bio &&
>>>>> +	    (!io_is_mergeable(sbi, io->bio, io, fio, io->last_block_in_bio,
>>>>> +			      fio->new_blkaddr) ||
>>>>> +	     !f2fs_crypt_mergeable_bio(io->bio, fio->page->mapping->host,
>>>>> +				       fio->page->index, fio)))
>>>>
>>>> bio_page->index, fio)))
>>>>
>>>>>  		__submit_merged_bio(io);
>>>>>  alloc_new:
>>>>>  	if (io->bio == NULL) {
>>>>> @@ -949,6 +988,8 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
>>>>>  			goto skip;
>>>>>  		}
>>>>>  		io->bio = __bio_alloc(fio, BIO_MAX_PAGES);
>>>>> +		f2fs_set_bio_crypt_ctx(io->bio, fio->page->mapping->host,
>>>>> +				       fio->page->index, fio, GFP_NOIO);
>>>>
>>>> bio_page->index, fio, GFP_NOIO);
>>>>
>>>
>>> We're using ->mapping->host and ->index.  Ordinarily that would mean the page
>>> needs to be a pagecache page.  But bio_page can also be a compressed page or a
>>> bounce page containing fs-layer encrypted contents.
>>
>> I'm concerning about compression + inlinecrypt case.
>>
>>>
>>> Is your suggestion to keep using fio->page->mapping->host (since encrypted pages
>>
>> Yup,
>>
>>> don't have a mapping), but start using bio_page->index (since f2fs apparently
>>
>> I meant that we need to use bio_page->index as tweak value in write path to
>> keep consistent as we did in read path, otherwise we may read the wrong
>> decrypted data later to incorrect tweak value.
>>
>> - f2fs_read_multi_pages (only comes from compression inode)
>>  - f2fs_alloc_dic
>>   - f2fs_set_compressed_page(page, cc->inode,
>> 					start_idx + i + 1, dic);
>>                                         ^^^^^^^^^^^^^^^^^
>>   - dic->cpages[i] = page;
>>  - for ()
>>      struct page *page = dic->cpages[i];
>>      if (!bio)
>>        - f2fs_grab_read_bio(..., page->index,..)
>>         - f2fs_set_bio_crypt_ctx(..., first_idx, ..)   /* first_idx == cpage->index */
>>
>> You can see that cpage->index was set to page->index + 1, that's why we need
>> to use one of cpage->index/page->index as tweak value all the time rather than
>> using both index mixed in read/write path.
>>
>> But note that for fs-layer encryption, we have used cpage->index as tweak value,
>> so here I suggest we can keep consistent to use cpage->index in inlinecrypt case.
> 
> Yes, inlinecrypt mustn't change the ciphertext that gets written to disk.
> 
>>
>>> *does* set ->index for compressed pages, and if the file uses fs-layer
>>> encryption then f2fs_set_bio_crypt_ctx() won't use the index anyway)?
>>>
>>> Does this mean the code is currently broken for compression + inline encryption
>>> because it's using the wrong ->index?  I think the answer is no, since
>>
>> I guess it's broken now for compression + inlinecrypt case.
>>
>>> f2fs_write_compressed_pages() will still pass the first 'nr_cpages' pagecache
>>> pages along with the compressed pages.  In that case, your suggestion would be a
>>> cleanup rather than a fix?
>>
>> That's a fix.
>>
> 
> FWIW, I tested this, and it actually works both before and after your suggested
> change.  The reason is that f2fs_write_compressed_pages() actually passes the
> pagecache pages sequentially starting at 'start_idx_of_cluster(cc) + 1' for the
> length of the compressed cluster.  That matches the '+ 1' adjustment elsewhere,
> so we have fio->page->index == bio_page->index.

I've checked the code, yes, that's correct.

> 
> I personally think the way the f2fs compression code works is really confusing.
> Compressed pages don't have a 1:1 correspondence to pagecache pages, so there
> should *not* be a pagecache page passed around when writing a compressed page.

The only place we always use fio->page is:
- f2fs_submit_page_write
 - trace_f2fs_submit_page_write(fio->page,..)
  - f2fs__submit_page_bio
   __entry->dev		= page_file_mapping(page)->host->i_sb->s_dev;
   __entry->ino		= page_file_mapping(page)->host->i_ino;

For compression case, we can get rid of using this parameter because bio_page
(fio->compressed_page) has correct mapping info, however for fs-layer encryption
case, bio_page (fio->encrypted_page, allocated by fscrypt_alloc_bounce_page())
has not correct mapping info.

> 
> Anyway, here's the test script I used in case anyone else wants to use it.  But
> we really need to write a proper f2fs compression + encryption test for xfstests
> which decrypts and decompresses a file in userspace and verifies we get back the
> original data.  (There are already ciphertext verification tests, but they don't
> cover compression.)  Note that this test is needed even for the filesystem-layer
> encryption which is currently supported.

Yes, let me check how to make this testcase a bit later.

> 
> #!/bin/bash
> 
> set -e
> 
> DEV=/dev/vdb
> 
> umount /mnt &> /dev/null || true
> mkfs.f2fs -f -O encrypt,compression,extra_attr $DEV
> head -c 1000000 /dev/zero > /tmp/testdata
> 
> for opt1 in '-o inlinecrypt' ''; do
>         mount $DEV /mnt $opt1
>         rm -rf /mnt/.fscrypt /mnt/dir
>         fscrypt setup /mnt
>         mkdir /mnt/dir
>         chattr +c /mnt/dir
>         echo hunter2 | fscrypt encrypt /mnt/dir --quiet --source=custom_passphrase --name=secret
>         cp /tmp/testdata /mnt/dir/file
>         umount /mnt
>         for opt2 in '-o inlinecrypt' ''; do
>                 mount $DEV /mnt $opt2
>                 echo hunter2 | fscrypt unlock /mnt/dir --quiet
>                 cmp /mnt/dir/file /tmp/testdata
>                 umount /mnt
>         done
> done
> .
> 

Re: [PATCH 3/4] f2fs: add inline encryption support

[Eric Biggers]

On Wed, Jun 17, 2020 at 07:57:31AM +0000, Satya Tangirala wrote:
> Wire up f2fs to support inline encryption via the helper functions which
> fs/crypto/ now provides.  This includes:
> 
> - Adding a mount option 'inlinecrypt' which enables inline encryption
>   on encrypted files where it can be used.
> 
> - Setting the bio_crypt_ctx on bios that will be submitted to an
>   inline-encrypted file.
> 
> - Not adding logically discontiguous data to bios that will be submitted
>   to an inline-encrypted file.
> 
> - Not doing filesystem-layer crypto on inline-encrypted files.
> 
> This patch includes a fix for a race during IPU by
> Sahitya Tummala <stummala@codeaurora.org>
> 
> Co-developed-by: Eric Biggers <ebiggers@google.com>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> Signed-off-by: Satya Tangirala <satyat@google.com>
> ---
>  Documentation/filesystems/f2fs.rst |  7 ++-
>  fs/f2fs/compress.c                 |  2 +-
>  fs/f2fs/data.c                     | 81 ++++++++++++++++++++++++------
>  fs/f2fs/super.c                    | 32 ++++++++++++
>  4 files changed, 104 insertions(+), 18 deletions(-)
> 
> diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
> index 099d45ac8d8f..4dc36143ff82 100644
> --- a/Documentation/filesystems/f2fs.rst
> +++ b/Documentation/filesystems/f2fs.rst
> @@ -258,7 +258,12 @@ compress_extension=%s  Support adding specified extension, so that f2fs can enab
>                         on compression extension list and enable compression on
>                         these file by default rather than to enable it via ioctl.
>                         For other files, we can still enable compression via ioctl.
> -====================== ============================================================

The above line being deleted marks the end of a table, so it shouldn't be
deleted (it should go after the part below).

> +inlinecrypt
> +                       Encrypt/decrypt the contents of encrypted files using the
> +                       blk-crypto framework rather than filesystem-layer encryption.
> +                       This allows the use of inline encryption hardware. The on-disk
> +                       format is unaffected. For more details, see
> +                       Documentation/block/inline-encryption.rst.

Like I commented on one of the commit messages -- this doesn't make it clear
what happens in cases where blk-crypto can't be used.  Maybe just replace:
"Encrypt/decrypt" => "When possible, encrypt/decrypt".

Likewise for the ext4 documentation for this same mount option.

- Eric

[PATCH 4/4] ext4: add inline encryption support

[Satya Tangirala]

From: Eric Biggers <ebiggers@google.com>

Wire up ext4 to support inline encryption via the helper functions which
fs/crypto/ now provides.  This includes:

- Adding a mount option 'inlinecrypt' which enables inline encryption
  on encrypted files where it can be used.

- Setting the bio_crypt_ctx on bios that will be submitted to an
  inline-encrypted file.

  Note: submit_bh_wbc() in fs/buffer.c also needed to be patched for
  this part, since ext4 sometimes uses ll_rw_block() on file data.

- Not adding logically discontiguous data to bios that will be submitted
  to an inline-encrypted file.

- Not doing filesystem-layer crypto on inline-encrypted files.

Signed-off-by: Eric Biggers <ebiggers@google.com>
Co-developed-by: Satya Tangirala <satyat@google.com>
Signed-off-by: Satya Tangirala <satyat@google.com>
---
 Documentation/admin-guide/ext4.rst |  6 ++++++
 fs/buffer.c                        |  7 ++++---
 fs/ext4/inode.c                    |  4 ++--
 fs/ext4/page-io.c                  |  6 ++++--
 fs/ext4/readpage.c                 | 11 ++++++++---
 fs/ext4/super.c                    |  9 +++++++++
 6 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/Documentation/admin-guide/ext4.rst b/Documentation/admin-guide/ext4.rst
index 9443fcef1876..ed997e376678 100644
--- a/Documentation/admin-guide/ext4.rst
+++ b/Documentation/admin-guide/ext4.rst
@@ -395,6 +395,12 @@ When mounting an ext4 filesystem, the following option are accepted:
         Documentation/filesystems/dax.txt.  Note that this option is
         incompatible with data=journal.
 
+  inlinecrypt
+        Encrypt/decrypt the contents of encrypted files using the blk-crypto
+        framework rather than filesystem-layer encryption. This allows the use
+        of inline encryption hardware. The on-disk format is unaffected. For
+        more details, see Documentation/block/inline-encryption.rst.
+
 Data Mode
 =========
 There are 3 different data modes:
diff --git a/fs/buffer.c b/fs/buffer.c
index 64fe82ec65ff..dc5e05b47646 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -320,9 +320,8 @@ static void decrypt_bh(struct work_struct *work)
 static void end_buffer_async_read_io(struct buffer_head *bh, int uptodate)
 {
 	/* Decrypt if needed */
-	if (uptodate && IS_ENABLED(CONFIG_FS_ENCRYPTION) &&
-	    IS_ENCRYPTED(bh->b_page->mapping->host) &&
-	    S_ISREG(bh->b_page->mapping->host->i_mode)) {
+	if (uptodate &&
+	    fscrypt_inode_uses_fs_layer_crypto(bh->b_page->mapping->host)) {
 		struct decrypt_bh_ctx *ctx = kmalloc(sizeof(*ctx), GFP_ATOMIC);
 
 		if (ctx) {
@@ -3046,6 +3045,8 @@ static int submit_bh_wbc(int op, int op_flags, struct buffer_head *bh,
 	 */
 	bio = bio_alloc(GFP_NOIO, 1);
 
+	fscrypt_set_bio_crypt_ctx_bh(bio, bh, GFP_NOIO);
+
 	bio->bi_iter.bi_sector = bh->b_blocknr * (bh->b_size >> 9);
 	bio_set_dev(bio, bh->b_bdev);
 	bio->bi_write_hint = write_hint;
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 40ec5c7ef0d3..54a027489c9c 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1096,7 +1096,7 @@ static int ext4_block_write_begin(struct page *page, loff_t pos, unsigned len,
 	}
 	if (unlikely(err)) {
 		page_zero_new_buffers(page, from, to);
-	} else if (IS_ENCRYPTED(inode) && S_ISREG(inode->i_mode)) {
+	} else if (fscrypt_inode_uses_fs_layer_crypto(inode)) {
 		for (i = 0; i < nr_wait; i++) {
 			int err2;
 
@@ -3737,7 +3737,7 @@ static int __ext4_block_zero_page_range(handle_t *handle,
 		/* Uhhuh. Read error. Complain and punt. */
 		if (!buffer_uptodate(bh))
 			goto unlock;
-		if (S_ISREG(inode->i_mode) && IS_ENCRYPTED(inode)) {
+		if (fscrypt_inode_uses_fs_layer_crypto(inode)) {
 			/* We expect the key to be set. */
 			BUG_ON(!fscrypt_has_encryption_key(inode));
 			err = fscrypt_decrypt_pagecache_blocks(page, blocksize,
diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
index de6fe969f773..defd2e10dfd1 100644
--- a/fs/ext4/page-io.c
+++ b/fs/ext4/page-io.c
@@ -402,6 +402,7 @@ static void io_submit_init_bio(struct ext4_io_submit *io,
 	 * __GFP_DIRECT_RECLAIM is set, see comments for bio_alloc_bioset().
 	 */
 	bio = bio_alloc(GFP_NOIO, BIO_MAX_PAGES);
+	fscrypt_set_bio_crypt_ctx_bh(bio, bh, GFP_NOIO);
 	bio->bi_iter.bi_sector = bh->b_blocknr * (bh->b_size >> 9);
 	bio_set_dev(bio, bh->b_bdev);
 	bio->bi_end_io = ext4_end_bio;
@@ -418,7 +419,8 @@ static void io_submit_add_bh(struct ext4_io_submit *io,
 {
 	int ret;
 
-	if (io->io_bio && bh->b_blocknr != io->io_next_block) {
+	if (io->io_bio && (bh->b_blocknr != io->io_next_block ||
+			   !fscrypt_mergeable_bio_bh(io->io_bio, bh))) {
 submit_and_retry:
 		ext4_io_submit(io);
 	}
@@ -506,7 +508,7 @@ int ext4_bio_write_page(struct ext4_io_submit *io,
 	 * (e.g. holes) to be unnecessarily encrypted, but this is rare and
 	 * can't happen in the common case of blocksize == PAGE_SIZE.
 	 */
-	if (IS_ENCRYPTED(inode) && S_ISREG(inode->i_mode) && nr_to_submit) {
+	if (fscrypt_inode_uses_fs_layer_crypto(inode) && nr_to_submit) {
 		gfp_t gfp_flags = GFP_NOFS;
 		unsigned int enc_bytes = round_up(len, i_blocksize(inode));
 
diff --git a/fs/ext4/readpage.c b/fs/ext4/readpage.c
index 5761e9961682..f2df2db0786c 100644
--- a/fs/ext4/readpage.c
+++ b/fs/ext4/readpage.c
@@ -195,7 +195,7 @@ static void ext4_set_bio_post_read_ctx(struct bio *bio,
 {
 	unsigned int post_read_steps = 0;
 
-	if (IS_ENCRYPTED(inode) && S_ISREG(inode->i_mode))
+	if (fscrypt_inode_uses_fs_layer_crypto(inode))
 		post_read_steps |= 1 << STEP_DECRYPT;
 
 	if (ext4_need_verity(inode, first_idx))
@@ -230,6 +230,7 @@ int ext4_mpage_readpages(struct inode *inode,
 	const unsigned blkbits = inode->i_blkbits;
 	const unsigned blocks_per_page = PAGE_SIZE >> blkbits;
 	const unsigned blocksize = 1 << blkbits;
+	sector_t next_block;
 	sector_t block_in_file;
 	sector_t last_block;
 	sector_t last_block_in_file;
@@ -258,7 +259,8 @@ int ext4_mpage_readpages(struct inode *inode,
 		if (page_has_buffers(page))
 			goto confused;
 
-		block_in_file = (sector_t)page->index << (PAGE_SHIFT - blkbits);
+		block_in_file = next_block =
+			(sector_t)page->index << (PAGE_SHIFT - blkbits);
 		last_block = block_in_file + nr_pages * blocks_per_page;
 		last_block_in_file = (ext4_readpage_limit(inode) +
 				      blocksize - 1) >> blkbits;
@@ -358,7 +360,8 @@ int ext4_mpage_readpages(struct inode *inode,
 		 * This page will go to BIO.  Do we need to send this
 		 * BIO off first?
 		 */
-		if (bio && (last_block_in_bio != blocks[0] - 1)) {
+		if (bio && (last_block_in_bio != blocks[0] - 1 ||
+			    !fscrypt_mergeable_bio(bio, inode, next_block))) {
 		submit_and_realloc:
 			submit_bio(bio);
 			bio = NULL;
@@ -370,6 +373,8 @@ int ext4_mpage_readpages(struct inode *inode,
 			 */
 			bio = bio_alloc(GFP_KERNEL,
 				min_t(int, nr_pages, BIO_MAX_PAGES));
+			fscrypt_set_bio_crypt_ctx(bio, inode, next_block,
+						  GFP_KERNEL);
 			ext4_set_bio_post_read_ctx(bio, inode, page->index);
 			bio_set_dev(bio, bdev);
 			bio->bi_iter.bi_sector = blocks[0] << (blkbits - 9);
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index c668f6b42374..faf9e5eaa029 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1511,6 +1511,7 @@ enum {
 	Opt_journal_path, Opt_journal_checksum, Opt_journal_async_commit,
 	Opt_abort, Opt_data_journal, Opt_data_ordered, Opt_data_writeback,
 	Opt_data_err_abort, Opt_data_err_ignore, Opt_test_dummy_encryption,
+	Opt_inlinecrypt,
 	Opt_usrjquota, Opt_grpjquota, Opt_offusrjquota, Opt_offgrpjquota,
 	Opt_jqfmt_vfsold, Opt_jqfmt_vfsv0, Opt_jqfmt_vfsv1, Opt_quota,
 	Opt_noquota, Opt_barrier, Opt_nobarrier, Opt_err,
@@ -1609,6 +1610,7 @@ static const match_table_t tokens = {
 	{Opt_max_dir_size_kb, "max_dir_size_kb=%u"},
 	{Opt_test_dummy_encryption, "test_dummy_encryption=%s"},
 	{Opt_test_dummy_encryption, "test_dummy_encryption"},
+	{Opt_inlinecrypt, "inlinecrypt"},
 	{Opt_nombcache, "nombcache"},
 	{Opt_nombcache, "no_mbcache"},	/* for backward compatibility */
 	{Opt_removed, "check=none"},	/* mount option from ext2/3 */
@@ -1938,6 +1940,13 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
 	case Opt_nolazytime:
 		sb->s_flags &= ~SB_LAZYTIME;
 		return 1;
+	case Opt_inlinecrypt:
+#ifdef CONFIG_FS_ENCRYPTION_INLINE_CRYPT
+		sb->s_flags |= SB_INLINECRYPT;
+#else
+		ext4_msg(sb, KERN_ERR, "inline encryption not supported");
+#endif
+		return 1;
 	}
 
 	for (m = ext4_mount_opts; m->token != Opt_err; m++)
-- 
2.27.0.290.gba653c62da-goog

Re: [PATCH 0/4] Inline Encryption Support for fscrypt

[Eric Biggers]

On Wed, Jun 17, 2020 at 07:57:28AM +0000, Satya Tangirala wrote:
> This patch series adds support for Inline Encryption to fscrypt, f2fs
> and ext4. It builds on the inline encryption support now present in
> the block layer, and has been rebased on v5.8-rc1.
> 
> Patch 1 introduces the SB_INLINECRYPT sb options, which filesystems
> should set if they want to use blk-crypto for file content en/decryption.
> 
> Patch 2 adds inline encryption support to fscrypt. To use inline
> encryption with fscrypt, the filesystem must set the above mentioned
> SB_INLINECRYPT sb option. When this option is set, the contents of
> encrypted files will be en/decrypted using blk-crypto.
> 
> Patches 3 and 4 wire up f2fs and ext4 respectively to fscrypt support for
> inline encryption, and e.g ensure that bios are submitted with blocks
> that not only are contiguous, but also have contiguous DUNs.
> 
> Eric Biggers (1):
>   ext4: add inline encryption support
> 
> Satya Tangirala (3):
>   fs: introduce SB_INLINECRYPT
>   fscrypt: add inline encryption support
>   f2fs: add inline encryption support
> 

Like I said on the UFS patchset: as this previously went through a number of
iterations as part of the "Inline Encryption Support" patchset (latest v13:
https://lkml.kernel.org/r/20200514003727.69001-1-satyat@google.com), it would be
helpful to list the changelog from v13 (though I can see that not too much
changed).  And I probably would have called it v14, but it doesn't matter much.

Explicit mentioning how this was tested would also be helpful.  And for that
matter, we should update the "Tests" section of the fscrypt documentation file
to mention also using the inlinecrypt mount option, e.g.:

diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst
index f517af8ec11c..f5d8b0303ddf 100644
--- a/Documentation/filesystems/fscrypt.rst
+++ b/Documentation/filesystems/fscrypt.rst
@@ -1255,6 +1255,7 @@ f2fs encryption using `kvm-xfstests
 <https://github.com/tytso/xfstests-bld/blob/master/Documentation/kvm-quickstart.md>`_::
 
     kvm-xfstests -c ext4,f2fs -g encrypt
+    kvm-xfstests -c ext4,f2fs -g encrypt -m inlinecrypt
 
 UBIFS encryption can also be tested this way, but it should be done in
 a separate command, and it takes some time for kvm-xfstests to set up
@@ -1276,6 +1277,7 @@ This tests the encrypted I/O paths more thoroughly.  To do this with
 kvm-xfstests, use the "encrypt" filesystem configuration::
 
     kvm-xfstests -c ext4/encrypt,f2fs/encrypt -g auto
+    kvm-xfstests -c ext4/encrypt,f2fs/encrypt -g auto -m inlinecrypt
 
 Because this runs many more tests than "-g encrypt" does, it takes
 much longer to run; so also consider using `gce-xfstests
@@ -1283,3 +1285,4 @@ much longer to run; so also consider using `gce-xfstests
 instead of kvm-xfstests::
 
     gce-xfstests -c ext4/encrypt,f2fs/encrypt -g auto
+    gce-xfstests -c ext4/encrypt,f2fs/encrypt -g auto -m inlinecrypt