Re: [PATCH 0/3] add support for metadata encryption to F2FS

[Chao Yu]

On 2020/10/5 15:36, Satya Tangirala wrote:
> This patch series adds support for metadata encryption to F2FS using
> blk-crypto.

It looks this implementation is based on hardware crypto engine, could you
please add this info into f2fs.rst as well like inlinecrypt...

> Patch 1 replaces fscrypt_get_devices (which took an array of request_queues
> and filled it up) with fscrypt_get_device, which takes a index of the
> desired device and returns the device at that index (so the index passed
> to fscrypt_get_device must be between 0 and (fscrypt_get_num_devices() - 1)
> inclusive). This allows callers to avoid having to allocate an array to
> pass to fscrypt_get_devices() when they only need to iterate through
> each element in the array (and have no use for the array itself).
>
> Patch 2 introduces some functions to fscrypt that help filesystems perform
> metadata encryption. Any filesystem that wants to use metadata encryption
> can call fscrypt_setup_metadata_encryption() with the super_block of the
> filesystem, the encryption algorithm and the descriptor of the encryption
> key. The descriptor is looked up in the logon keyring of the current
> session with "fscrypt:" as the prefix of the descriptor.
>
> The patch also introduces fscrypt_metadata_crypt_bio() which an FS should
> call on a bio that the FS wants metadata crypted. The function will add
> an encryption context with the metadata encryption key set up by the call
> to the above mentioned fscrypt_setup_metadata_encryption().
>
> The patch also introduces fscrypt_metadata_crypt_prepare_all_devices().
> Filesystems that use multiple devices should call this function once all
> the underlying devices have been determined. An FS might only be able to
> determine all the underlying devices after some initial processing that
> might already require metadata en/decryption, which is why this function
> is separate from fscrypt_setup_metadata_encryption().
>
> Patch 3 wires up F2FS with the functions introduced in Patch 2. F2FS
> will encrypt every block (that's not being encrypted by some other
> encryption key, e.g. a per-file key) with the metadata encryption key
> except the superblock (and the redundant copy of the superblock). The DUN
> of a block is the offset of the block from the start of the F2FS
> filesystem.

Why not using nid as DUN, then GC could migrate encrypted node block directly via
meta inode's address space like we do for encrypted data block, rather than
decrypting node block to node page and then encrypting node page with DUN of new
blkaddr it migrates to.

Thanks,

> Please refer to the commit message for why the superblock was excluded from
> en/decryption, and other limitations. The superblock and its copy are
> stored in plaintext on disk. The encryption algorithm used for metadata
> encryption is stored within the superblock itself. Changes to the userspace
> tools (that are required to test out metadata encryption with F2FS) are
> also being sent out - I'll post a link as a reply to this mail once it's
> out.
>
> Satya Tangirala (3):
>    fscrypt, f2fs: replace fscrypt_get_devices with fscrypt_get_device
>    fscrypt: Add metadata encryption support
>    f2fs: Add metadata encryption support
>
>   Documentation/filesystems/f2fs.rst |  12 ++
>   fs/crypto/Kconfig                  |   6 +
>   fs/crypto/Makefile                 |   1 +
>   fs/crypto/fscrypt_private.h        |  19 +++
>   fs/crypto/inline_crypt.c           |  37 +----
>   fs/crypto/metadata_crypt.c         | 220 +++++++++++++++++++++++++++++
>   fs/f2fs/data.c                     |  24 ++--
>   fs/f2fs/f2fs.h                     |   2 +
>   fs/f2fs/super.c                    |  83 +++++++++--
>   include/linux/f2fs_fs.h            |   3 +-
>   include/linux/fs.h                 |   3 +
>   include/linux/fscrypt.h            |  51 ++++++-
>   12 files changed, 410 insertions(+), 51 deletions(-)
>   create mode 100644 fs/crypto/metadata_crypt.c