Re: [PATCH v2 00/20] btrfs: add fscrypt integration

David Sterba
2022-09-07T19:43:37Z

On Mon, Sep 05, 2022 at 08:35:15PM -0400, Sweet Tea Dorminy wrote:
> This is a changeset adding encryption to btrfs.
>
> Last October, Omar Sandoval sent out a design document for having fscrypt
> integration with btrfs [1]. In summary, it proposes btrfs storing its
> own encryption IVs on a per-file-extent basis. fscrypt usually encrypts
> files using an IV derived from per-inode information; this would prevent
> snapshotting or reflinking or data relocation for btrfs. We have
> refined this into a fscrypt extent context object, opaque to the
> filesystem, which fscrypt uses to generate an IV associated with each
> block in an extent. Thus, all the inodes sharing a particular
> key and file extent may decrypt the extent.
>
> This series implements this integration for the simple
> case, non-compressed data extents. Followup changes will allow
> encryption of compressed extents, inline extents, and verity items, and
> will add tests around subvolume encryption. This series should provide
> encryption for the simplest cases, but this series should not be used in
> production, as there are likely bugs.
>
> Preliminary btrfs-progs changes are available at [2]; fstests changes
> are available at [3].

I did a quick pass to check if there's anything that could be merged to
6.1 as preparatory, the fname is a candidate but I've also seen random
coding style issues that I'd like to get fixed first.

One thing I've noticed is that the incompat bit is only defined but not
used anywhere. Any functional change should be guarded behind it, and
once the implementation is complete the enabling part is in a separate
patch.

Regarding the build config options, I assume that the fscrypt support is
optional, so it should build with and without the option. I'm not sure
I've see enough ifdefs for that. For such features there should be a
line in btrfs_print_mod_info, like we have eg. for verity.

I'll post other comments under the patches.