[PATCH v7 00/43] btrfs: add fscrypt support

Daniel Vacek posted 43 patches 1 month ago
Documentation/filesystems/fscrypt.rst |  41 +++
block/blk-crypto-fallback.c           |  41 +++
block/blk-crypto-internal.h           |   8 +
block/blk-crypto-profile.c            |   2 +
block/blk-crypto.c                    |   6 +-
fs/btrfs/Kconfig                      |   4 +
fs/btrfs/Makefile                     |   1 +
fs/btrfs/accessors.h                  |   2 +
fs/btrfs/backref.c                    |  43 ++-
fs/btrfs/bio.c                        | 155 +++++++++-
fs/btrfs/bio.h                        |  14 +-
fs/btrfs/btrfs_inode.h                |   7 +-
fs/btrfs/compression.c                |   6 +
fs/btrfs/ctree.h                      |   3 +
fs/btrfs/defrag.c                     |  14 +
fs/btrfs/delayed-inode.c              |  25 +-
fs/btrfs/delayed-inode.h              |   5 +-
fs/btrfs/dir-item.c                   | 110 ++++++-
fs/btrfs/dir-item.h                   |  10 +-
fs/btrfs/direct-io.c                  |  28 +-
fs/btrfs/disk-io.c                    |   3 +-
fs/btrfs/extent_io.c                  | 115 ++++++-
fs/btrfs/extent_io.h                  |   3 +
fs/btrfs/extent_map.c                 | 102 ++++++-
fs/btrfs/extent_map.h                 |  26 ++
fs/btrfs/file-item.c                  |  28 +-
fs/btrfs/file-item.h                  |   2 +-
fs/btrfs/file.c                       |  79 +++++
fs/btrfs/fs.h                         |   6 +-
fs/btrfs/fscrypt.c                    | 413 ++++++++++++++++++++++++++
fs/btrfs/fscrypt.h                    |  86 ++++++
fs/btrfs/inode.c                      | 404 +++++++++++++++++++------
fs/btrfs/ioctl.c                      |  41 ++-
fs/btrfs/ordered-data.c               |  35 ++-
fs/btrfs/ordered-data.h               |  14 +
fs/btrfs/reflink.c                    |  43 ++-
fs/btrfs/root-tree.c                  |   9 +-
fs/btrfs/root-tree.h                  |   2 +-
fs/btrfs/send.c                       | 134 ++++++++-
fs/btrfs/super.c                      |  99 +++++-
fs/btrfs/super.h                      |   3 +-
fs/btrfs/sysfs.c                      |   6 +
fs/btrfs/tree-checker.c               |  64 +++-
fs/btrfs/tree-log.c                   |  34 ++-
fs/btrfs/volumes.c                    |   5 +
fs/crypto/crypto.c                    |  10 +-
fs/crypto/fname.c                     |  36 ---
fs/crypto/fscrypt_private.h           |  51 +++-
fs/crypto/hooks.c                     |  38 ++-
fs/crypto/inline_crypt.c              |  91 +++++-
fs/crypto/keyring.c                   |  18 +-
fs/crypto/keysetup.c                  | 164 ++++++++++
fs/crypto/policy.c                    |  47 +++
include/linux/blk-crypto.h            |  15 +-
include/linux/fscrypt.h               | 127 ++++++++
include/uapi/linux/btrfs.h            |   1 +
include/uapi/linux/btrfs_tree.h       |  26 +-
57 files changed, 2665 insertions(+), 240 deletions(-)
create mode 100644 fs/btrfs/fscrypt.c
create mode 100644 fs/btrfs/fscrypt.h
[PATCH v7 00/43] btrfs: add fscrypt support
Posted by Daniel Vacek 1 month ago
Hello,

These are the remaining parts from former series [1] from Omar, Sweet Tea
and Josef.  Some bits of it were split into the separate set [2] before.

Notably, at this stage encryption is not supported with RAID5/6 setup
and send is also disabled for now.

For straight git access you can find this series as the `fscrypt-v7` tag
e85358ef9fba here:

[0] https://github.com/dvacek/linux-btrfs/tree/fscrypt-v7

Changes since v6 [3]
 * Rebased onto linux v7.1-rc3.
 * Adapted to the v7.0 fscrypt API changes, mostly following commit
   bb8e2019ad613 ("blk-crypto: handle the fallback above the block layer")
 * Addressed all the review feedback, thanks to Eric Biggers, Chris Mason
   (and his LLM review prompts) and Neal Gompa.
 * Adapted to the v7.1 fscrypt API cleanups, using byte offsets as function
   arguments instead of logical block numbers for newly introduced functions.
   This should match https://lore.kernel.org/linux-fscrypt/20260218061531.3318130-1-hch@lst.de/
   As a result btrfs_set_bio_crypt_ctx_from_extent() and btrfs_mergeable_encrypted_bio()
   helpers were no longer needed and they got removed.

There are a few changes since v5 [1]:
 * Rebased to btrfs/for-next branch.  Couple things changed in the last
   years.  A few patches were dropped as the code cleaned up or refactored.
   More details in the patches themselves.
 * As suggested by Qu and Dave, the on-disk format of storing the extent
   encryption context was re-designed.  Now, a new tree item with dedicated
   key is inserted instead of extending the file extent item.  As a result
   a special care needs to be taken when removing the encrypted extents
   to also remove the related encryption context item.
 * Fixed bugs found during testing.

Have a nice day,
Daniel

[1] v5 https://lore.kernel.org/linux-btrfs/cover.1706116485.git.josef@toxicpanda.com/
[2]    https://lore.kernel.org/linux-btrfs/20251112193611.2536093-1-neelx@suse.com/
[3] v6 https://lore.kernel.org/linux-btrfs/20260206182336.1397715-1-neelx@suse.com/

Josef Bacik (33):
  fscrypt: add per-extent encryption support
  fscrypt: allow inline encryption for extent based encryption
  fscrypt: add a __fscrypt_file_open helper
  fscrypt: conditionally don't wipe mk secret until the last active user
    is done
  blk-crypto: add a process bio callback
  fscrypt: add a process_bio hook to fscrypt_operations
  fscrypt: add documentation about extent encryption
  btrfs: add infrastructure for safe em freeing
  btrfs: select encryption dependencies if FS_ENCRYPTION
  btrfs: add fscrypt_info and encryption_type to ordered_extent
  btrfs: plumb through setting the fscrypt_info for ordered extents
  btrfs: populate the ordered_extent with the fscrypt context
  btrfs: keep track of fscrypt info and orig_start for dio reads
  btrfs: add extent encryption context tree item type
  btrfs: pass through fscrypt_extent_info to the file extent helpers
  btrfs: implement the fscrypt extent encryption hooks
  btrfs: setup fscrypt_extent_info for new extents
  btrfs: populate ordered_extent with the orig offset
  btrfs: set the bio fscrypt context when applicable
  btrfs: add a bio argument to btrfs_csum_one_bio
  btrfs: limit encrypted writes to 256 segments
  btrfs: implement process_bio cb for fscrypt
  btrfs: implement read repair for encryption
  btrfs: add test_dummy_encryption support
  btrfs: make btrfs_ref_to_path handle encrypted filenames
  btrfs: deal with encrypted symlinks in send
  btrfs: decrypt file names for send
  btrfs: load the inode context before sending writes
  btrfs: set the appropriate free space settings in reconfigure
  btrfs: support encryption with log replay
  btrfs: disable auto defrag on encrypted files
  btrfs: disable encryption on RAID5/6
  btrfs: disable send if we have encryption enabled

Omar Sandoval (6):
  fscrypt: expose fscrypt_nokey_name
  btrfs: start using fscrypt hooks
  btrfs: add inode encryption contexts
  btrfs: add new FEATURE_INCOMPAT_ENCRYPT flag
  btrfs: adapt readdir for encrypted and nokey names
  btrfs: implement fscrypt ioctls

Sweet Tea Dorminy (4):
  btrfs: handle nokey names
  btrfs: add get_devices hook for fscrypt
  btrfs: set file extent encryption excplicitly
  btrfs: add fscrypt_info and encryption_type to extent_map

 Documentation/filesystems/fscrypt.rst |  41 +++
 block/blk-crypto-fallback.c           |  41 +++
 block/blk-crypto-internal.h           |   8 +
 block/blk-crypto-profile.c            |   2 +
 block/blk-crypto.c                    |   6 +-
 fs/btrfs/Kconfig                      |   4 +
 fs/btrfs/Makefile                     |   1 +
 fs/btrfs/accessors.h                  |   2 +
 fs/btrfs/backref.c                    |  43 ++-
 fs/btrfs/bio.c                        | 155 +++++++++-
 fs/btrfs/bio.h                        |  14 +-
 fs/btrfs/btrfs_inode.h                |   7 +-
 fs/btrfs/compression.c                |   6 +
 fs/btrfs/ctree.h                      |   3 +
 fs/btrfs/defrag.c                     |  14 +
 fs/btrfs/delayed-inode.c              |  25 +-
 fs/btrfs/delayed-inode.h              |   5 +-
 fs/btrfs/dir-item.c                   | 110 ++++++-
 fs/btrfs/dir-item.h                   |  10 +-
 fs/btrfs/direct-io.c                  |  28 +-
 fs/btrfs/disk-io.c                    |   3 +-
 fs/btrfs/extent_io.c                  | 115 ++++++-
 fs/btrfs/extent_io.h                  |   3 +
 fs/btrfs/extent_map.c                 | 102 ++++++-
 fs/btrfs/extent_map.h                 |  26 ++
 fs/btrfs/file-item.c                  |  28 +-
 fs/btrfs/file-item.h                  |   2 +-
 fs/btrfs/file.c                       |  79 +++++
 fs/btrfs/fs.h                         |   6 +-
 fs/btrfs/fscrypt.c                    | 413 ++++++++++++++++++++++++++
 fs/btrfs/fscrypt.h                    |  86 ++++++
 fs/btrfs/inode.c                      | 404 +++++++++++++++++++------
 fs/btrfs/ioctl.c                      |  41 ++-
 fs/btrfs/ordered-data.c               |  35 ++-
 fs/btrfs/ordered-data.h               |  14 +
 fs/btrfs/reflink.c                    |  43 ++-
 fs/btrfs/root-tree.c                  |   9 +-
 fs/btrfs/root-tree.h                  |   2 +-
 fs/btrfs/send.c                       | 134 ++++++++-
 fs/btrfs/super.c                      |  99 +++++-
 fs/btrfs/super.h                      |   3 +-
 fs/btrfs/sysfs.c                      |   6 +
 fs/btrfs/tree-checker.c               |  64 +++-
 fs/btrfs/tree-log.c                   |  34 ++-
 fs/btrfs/volumes.c                    |   5 +
 fs/crypto/crypto.c                    |  10 +-
 fs/crypto/fname.c                     |  36 ---
 fs/crypto/fscrypt_private.h           |  51 +++-
 fs/crypto/hooks.c                     |  38 ++-
 fs/crypto/inline_crypt.c              |  91 +++++-
 fs/crypto/keyring.c                   |  18 +-
 fs/crypto/keysetup.c                  | 164 ++++++++++
 fs/crypto/policy.c                    |  47 +++
 include/linux/blk-crypto.h            |  15 +-
 include/linux/fscrypt.h               | 127 ++++++++
 include/uapi/linux/btrfs.h            |   1 +
 include/uapi/linux/btrfs_tree.h       |  26 +-
 57 files changed, 2665 insertions(+), 240 deletions(-)
 create mode 100644 fs/btrfs/fscrypt.c
 create mode 100644 fs/btrfs/fscrypt.h

-- 
2.53.0
Re: [PATCH v7 00/43] btrfs: add fscrypt support
Posted by Daniel Vacek 3 weeks ago
On Wed, 13 May 2026 at 10:54, Daniel Vacek <neelx@suse.com> wrote:
>
> Hello,
>
> These are the remaining parts from former series [1] from Omar, Sweet Tea
> and Josef.  Some bits of it were split into the separate set [2] before.
>
> Notably, at this stage encryption is not supported with RAID5/6 setup
> and send is also disabled for now.
>
> For straight git access you can find this series as the `fscrypt-v7` tag
> e85358ef9fba here:
>
> [0] https://github.com/dvacek/linux-btrfs/tree/fscrypt-v7
>
> Changes since v6 [3]
>  * Rebased onto linux v7.1-rc3.
>  * Adapted to the v7.0 fscrypt API changes, mostly following commit
>    bb8e2019ad613 ("blk-crypto: handle the fallback above the block layer")
>  * Addressed all the review feedback, thanks to Eric Biggers, Chris Mason
>    (and his LLM review prompts) and Neal Gompa.
>  * Adapted to the v7.1 fscrypt API cleanups, using byte offsets as function
>    arguments instead of logical block numbers for newly introduced functions.
>    This should match https://lore.kernel.org/linux-fscrypt/20260218061531.3318130-1-hch@lst.de/
>    As a result btrfs_set_bio_crypt_ctx_from_extent() and btrfs_mergeable_encrypted_bio()
>    helpers were no longer needed and they got removed.

Hi Eric,

This is just a gentle ping.
I was wondering if you had a chance to look at this version?
I believe all your previous feedback has been addressed and this
version is solid.
Please, let me know your thoughts.

Regards,
Daniel

> There are a few changes since v5 [1]:
>  * Rebased to btrfs/for-next branch.  Couple things changed in the last
>    years.  A few patches were dropped as the code cleaned up or refactored.
>    More details in the patches themselves.
>  * As suggested by Qu and Dave, the on-disk format of storing the extent
>    encryption context was re-designed.  Now, a new tree item with dedicated
>    key is inserted instead of extending the file extent item.  As a result
>    a special care needs to be taken when removing the encrypted extents
>    to also remove the related encryption context item.
>  * Fixed bugs found during testing.
>
> Have a nice day,
> Daniel
>
> [1] v5 https://lore.kernel.org/linux-btrfs/cover.1706116485.git.josef@toxicpanda.com/
> [2]    https://lore.kernel.org/linux-btrfs/20251112193611.2536093-1-neelx@suse.com/
> [3] v6 https://lore.kernel.org/linux-btrfs/20260206182336.1397715-1-neelx@suse.com/
>
> Josef Bacik (33):
>   fscrypt: add per-extent encryption support
>   fscrypt: allow inline encryption for extent based encryption
>   fscrypt: add a __fscrypt_file_open helper
>   fscrypt: conditionally don't wipe mk secret until the last active user
>     is done
>   blk-crypto: add a process bio callback
>   fscrypt: add a process_bio hook to fscrypt_operations
>   fscrypt: add documentation about extent encryption
>   btrfs: add infrastructure for safe em freeing
>   btrfs: select encryption dependencies if FS_ENCRYPTION
>   btrfs: add fscrypt_info and encryption_type to ordered_extent
>   btrfs: plumb through setting the fscrypt_info for ordered extents
>   btrfs: populate the ordered_extent with the fscrypt context
>   btrfs: keep track of fscrypt info and orig_start for dio reads
>   btrfs: add extent encryption context tree item type
>   btrfs: pass through fscrypt_extent_info to the file extent helpers
>   btrfs: implement the fscrypt extent encryption hooks
>   btrfs: setup fscrypt_extent_info for new extents
>   btrfs: populate ordered_extent with the orig offset
>   btrfs: set the bio fscrypt context when applicable
>   btrfs: add a bio argument to btrfs_csum_one_bio
>   btrfs: limit encrypted writes to 256 segments
>   btrfs: implement process_bio cb for fscrypt
>   btrfs: implement read repair for encryption
>   btrfs: add test_dummy_encryption support
>   btrfs: make btrfs_ref_to_path handle encrypted filenames
>   btrfs: deal with encrypted symlinks in send
>   btrfs: decrypt file names for send
>   btrfs: load the inode context before sending writes
>   btrfs: set the appropriate free space settings in reconfigure
>   btrfs: support encryption with log replay
>   btrfs: disable auto defrag on encrypted files
>   btrfs: disable encryption on RAID5/6
>   btrfs: disable send if we have encryption enabled
>
> Omar Sandoval (6):
>   fscrypt: expose fscrypt_nokey_name
>   btrfs: start using fscrypt hooks
>   btrfs: add inode encryption contexts
>   btrfs: add new FEATURE_INCOMPAT_ENCRYPT flag
>   btrfs: adapt readdir for encrypted and nokey names
>   btrfs: implement fscrypt ioctls
>
> Sweet Tea Dorminy (4):
>   btrfs: handle nokey names
>   btrfs: add get_devices hook for fscrypt
>   btrfs: set file extent encryption excplicitly
>   btrfs: add fscrypt_info and encryption_type to extent_map
>
>  Documentation/filesystems/fscrypt.rst |  41 +++
>  block/blk-crypto-fallback.c           |  41 +++
>  block/blk-crypto-internal.h           |   8 +
>  block/blk-crypto-profile.c            |   2 +
>  block/blk-crypto.c                    |   6 +-
>  fs/btrfs/Kconfig                      |   4 +
>  fs/btrfs/Makefile                     |   1 +
>  fs/btrfs/accessors.h                  |   2 +
>  fs/btrfs/backref.c                    |  43 ++-
>  fs/btrfs/bio.c                        | 155 +++++++++-
>  fs/btrfs/bio.h                        |  14 +-
>  fs/btrfs/btrfs_inode.h                |   7 +-
>  fs/btrfs/compression.c                |   6 +
>  fs/btrfs/ctree.h                      |   3 +
>  fs/btrfs/defrag.c                     |  14 +
>  fs/btrfs/delayed-inode.c              |  25 +-
>  fs/btrfs/delayed-inode.h              |   5 +-
>  fs/btrfs/dir-item.c                   | 110 ++++++-
>  fs/btrfs/dir-item.h                   |  10 +-
>  fs/btrfs/direct-io.c                  |  28 +-
>  fs/btrfs/disk-io.c                    |   3 +-
>  fs/btrfs/extent_io.c                  | 115 ++++++-
>  fs/btrfs/extent_io.h                  |   3 +
>  fs/btrfs/extent_map.c                 | 102 ++++++-
>  fs/btrfs/extent_map.h                 |  26 ++
>  fs/btrfs/file-item.c                  |  28 +-
>  fs/btrfs/file-item.h                  |   2 +-
>  fs/btrfs/file.c                       |  79 +++++
>  fs/btrfs/fs.h                         |   6 +-
>  fs/btrfs/fscrypt.c                    | 413 ++++++++++++++++++++++++++
>  fs/btrfs/fscrypt.h                    |  86 ++++++
>  fs/btrfs/inode.c                      | 404 +++++++++++++++++++------
>  fs/btrfs/ioctl.c                      |  41 ++-
>  fs/btrfs/ordered-data.c               |  35 ++-
>  fs/btrfs/ordered-data.h               |  14 +
>  fs/btrfs/reflink.c                    |  43 ++-
>  fs/btrfs/root-tree.c                  |   9 +-
>  fs/btrfs/root-tree.h                  |   2 +-
>  fs/btrfs/send.c                       | 134 ++++++++-
>  fs/btrfs/super.c                      |  99 +++++-
>  fs/btrfs/super.h                      |   3 +-
>  fs/btrfs/sysfs.c                      |   6 +
>  fs/btrfs/tree-checker.c               |  64 +++-
>  fs/btrfs/tree-log.c                   |  34 ++-
>  fs/btrfs/volumes.c                    |   5 +
>  fs/crypto/crypto.c                    |  10 +-
>  fs/crypto/fname.c                     |  36 ---
>  fs/crypto/fscrypt_private.h           |  51 +++-
>  fs/crypto/hooks.c                     |  38 ++-
>  fs/crypto/inline_crypt.c              |  91 +++++-
>  fs/crypto/keyring.c                   |  18 +-
>  fs/crypto/keysetup.c                  | 164 ++++++++++
>  fs/crypto/policy.c                    |  47 +++
>  include/linux/blk-crypto.h            |  15 +-
>  include/linux/fscrypt.h               | 127 ++++++++
>  include/uapi/linux/btrfs.h            |   1 +
>  include/uapi/linux/btrfs_tree.h       |  26 +-
>  57 files changed, 2665 insertions(+), 240 deletions(-)
>  create mode 100644 fs/btrfs/fscrypt.c
>  create mode 100644 fs/btrfs/fscrypt.h
>
> --
> 2.53.0
>
Re: [PATCH v7 00/43] btrfs: add fscrypt support
Posted by Eric Biggers 1 week, 5 days ago
On Fri, May 22, 2026 at 09:00:46AM +0200, Daniel Vacek wrote:
> Hi Eric,
> 
> This is just a gentle ping.
> I was wondering if you had a chance to look at this version?
> I believe all your previous feedback has been addressed and this
> version is solid.
> Please, let me know your thoughts.
> 
> Regards,
> Daniel

It's been really hard to find time to review this huge patchset.  I've
started going through it and will try to leave comments next week.

In the mean time it would be really helpful if you went through the
Sashiko reviews
(https://sashiko.dev/#/patchset/20260513085340.3673127-1-neelx%40suse.com)
and address the ones that make sense to.  It found 93 issues including
16 critical ones, which is kind of a lot.  Some of them are the same
things I'm noticing already.  Same for the issue that Christoph noticed
where new devices can be added; Sashiko had already found that too.

If I'm going to have to use my limited human review time to point out
issues that were already found, that's not a great use of time.

I also don't see any information about how this was tested (and will
continue to be tested in the future).

- Eric
Re: [PATCH v7 00/43] btrfs: add fscrypt support
Posted by Daniel Vacek 2 days, 3 hours ago
On Sun, 31 May 2026 at 02:29, Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Fri, May 22, 2026 at 09:00:46AM +0200, Daniel Vacek wrote:
> > Hi Eric,
> >
> > This is just a gentle ping.
> > I was wondering if you had a chance to look at this version?
> > I believe all your previous feedback has been addressed and this
> > version is solid.
> > Please, let me know your thoughts.
> >
> > Regards,
> > Daniel
>
> It's been really hard to find time to review this huge patchset.  I've
> started going through it and will try to leave comments next week.

Hi Eric,

First, thank you for looking into this.

> In the mean time it would be really helpful if you went through the
> Sashiko reviews
> (https://sashiko.dev/#/patchset/20260513085340.3673127-1-neelx%40suse.com)
> and address the ones that make sense to.  It found 93 issues including
> 16 critical ones, which is kind of a lot.  Some of them are the same
> things I'm noticing already.  Same for the issue that Christoph noticed
> where new devices can be added; Sashiko had already found that too.
>
> If I'm going to have to use my limited human review time to point out
> issues that were already found, that's not a great use of time.

I already went through some of the Sashiko reviews (they were slowly
coming up one by one) before leaving for my vacation. And I found them
mostly confusing or misleading. I'm gonna have a look into the rest I
haven't seen yet to see if there are any useful points. And I'll
compare it to your notes to see if it was Sashiko being confused or if
it was me.

> I also don't see any information about how this was tested (and will
> continue to be tested in the future).

I'm using the `encrypt` group of xfstests as the acceptance criteria
and the full test run to ensure no regressions.

General support for btrfs had to be added. And since we only support
the v2 fscrypt policy, some tests had to be split into two tests - one
testing v1 and the other testing v2 policy.
There are also additional new btrfs specific tests (reflinks,
snapshots). xfstests also need fscrypt support in btrfs-progs with
related changes to export nonces and other metadata like device
offsets of encrypted blocks for validation.

I'll send the fscrypt updated btrfs-progs and xfstests next. I just
need to clean them up a bit first. I'm sorry about the delay, but I
only managed to post the kernel part before my vacation. I was hoping
it would give you all more time to review.

Thanks,
Daniel

> - Eric
Re: [PATCH v7 00/43] btrfs: add fscrypt support
Posted by David Sterba 1 week, 3 days ago
On Sat, May 30, 2026 at 05:28:12PM -0700, Eric Biggers wrote:
> On Fri, May 22, 2026 at 09:00:46AM +0200, Daniel Vacek wrote:

> It's been really hard to find time to review this huge patchset.  I've
> started going through it and will try to leave comments next week.

Thank you. The patchset is huge but we'd like your feedback namely on
the crypto layer changes, i.e. the first 8 patches. As this is outside
of btrfs code we can't fix it incrementally later on. The current size
of btrfs-only can be slightly reduced but a lot of must stay so it's
clear how the fscrypt API is used.

I'm not sure if it was mentioned before wrt how to get this merged. My
preferred way is to get your Ack for the crypto changes and then we can
add it to linux-next via our btrfs git.

It's too late for 7.2 also because you've asked for some changes. So the
target is 7.3, hopefully giving all of us enough time to have the
mergeable version in ~7.2-rc3 timeframe.

If you have other ideas how to proceed with the merge process, please
let me know.

> In the mean time it would be really helpful if you went through the
> Sashiko reviews
> (https://sashiko.dev/#/patchset/20260513085340.3673127-1-neelx%40suse.com)
> and address the ones that make sense to.  It found 93 issues including
> 16 critical ones, which is kind of a lot.  Some of them are the same
> things I'm noticing already.  Same for the issue that Christoph noticed
> where new devices can be added; Sashiko had already found that too.
> 
> If I'm going to have to use my limited human review time to point out
> issues that were already found, that's not a great use of time.
> 
> I also don't see any information about how this was tested (and will
> continue to be tested in the future).

The testing is not straightforward as it needs 3 projects to
synchronize, kernel, fstests and btrfs-progs. Testing may need to use
custom git branches for all of them. For btrfs-progs the changes will ge
it in soon. For fstests it can be a chicken-egg problem, as they don't
accept tests for unmerged code. We've been using our fstests [1] with
additional fixups (not upstreamable, related to CI workarounds). Though
I'm not sure if Daniel has updated the branch with his most recent
version.

[1] https://github.com/btrfs/fstests branch staging
Re: [PATCH v7 00/43] btrfs: add fscrypt support
Posted by Eric Biggers 1 week, 3 days ago
On Mon, Jun 01, 2026 at 08:57:30PM +0200, David Sterba wrote:
> The testing is not straightforward as it needs 3 projects to
> synchronize, kernel, fstests and btrfs-progs. Testing may need to use
> custom git branches for all of them. For btrfs-progs the changes will ge
> it in soon. For fstests it can be a chicken-egg problem, as they don't
> accept tests for unmerged code. We've been using our fstests [1] with
> additional fixups (not upstreamable, related to CI workarounds). Though
> I'm not sure if Daniel has updated the branch with his most recent
> version.
> 
> [1] https://github.com/btrfs/fstests branch staging

Where are the btrfs-progs changes, then?  I'd like to try this out, but
there's no way to do it without the btrfs-progs changes.

- Eric
Re: [PATCH v7 00/43] btrfs: add fscrypt support
Posted by Eric Biggers 1 week, 3 days ago
On Mon, Jun 01, 2026 at 07:25:53PM -0700, Eric Biggers wrote:
> On Mon, Jun 01, 2026 at 08:57:30PM +0200, David Sterba wrote:
> > The testing is not straightforward as it needs 3 projects to
> > synchronize, kernel, fstests and btrfs-progs. Testing may need to use
> > custom git branches for all of them. For btrfs-progs the changes will ge
> > it in soon. For fstests it can be a chicken-egg problem, as they don't
> > accept tests for unmerged code. We've been using our fstests [1] with
> > additional fixups (not upstreamable, related to CI workarounds). Though
> > I'm not sure if Daniel has updated the branch with his most recent
> > version.
> > 
> > [1] https://github.com/btrfs/fstests branch staging
> 
> Where are the btrfs-progs changes, then?  I'd like to try this out, but
> there's no way to do it without the btrfs-progs changes.

Please make sure to test with debugging options enabled as well.  Trying
it with KASAN enabled, right away there's an out-of-bounds write in
base64_encode(), because btrfs_real_readdir() has an incorrect buffer
size calculation:

    nokey_len = DIV_ROUND_UP(name_len * 4, 3);

The other filesystems do it right by using fscrypt_fname_alloc_buffer().

Of course it turns out this is in the Sashiko comments as well, quoted
below:

    Is it possible for this check to allow a heap buffer overflow in the
    filldir buffer?

    The length check estimates the fscrypt nokey name size using
    DIV_ROUND_UP(name_len * 4, 3). However, fscrypt_fname_disk_to_usr()
    prepends an 8-byte dirhash before base64 encoding. Furthermore, if
    the name length exceeds 149 bytes, it hashes the name and encodes
    exactly 189 bytes, yielding a 252-byte output.

    If the remaining buffer space is large enough to pass this nokey_len
    check but smaller than the actual bytes produced by
    fscrypt_fname_disk_to_usr(), it looks like
    fscrypt_fname_disk_to_usr() could write past the end of the
    PAGE_SIZE allocation.

I think I've done enough with this series for now.  I'll take a look
again once it's in a better shape.

- Eric
Re: [PATCH v7 00/43] btrfs: add fscrypt support
Posted by Eric Biggers 1 week, 3 days ago
On Mon, Jun 01, 2026 at 08:57:30PM +0200, David Sterba wrote:
> On Sat, May 30, 2026 at 05:28:12PM -0700, Eric Biggers wrote:
> > On Fri, May 22, 2026 at 09:00:46AM +0200, Daniel Vacek wrote:
> 
> > It's been really hard to find time to review this huge patchset.  I've
> > started going through it and will try to leave comments next week.
> 
> Thank you. The patchset is huge but we'd like your feedback namely on
> the crypto layer changes, i.e. the first 8 patches.

I know, but properly reviewing those patches requires reviewing their
usage in btrfs too.  Plus the whole feature needs to be solid: if
there's a problem with it that's my problem too.  And some problems,
e.g. any in the on-disk format, would be unfixable later, so we have to
get them right from the beginning.

> It's too late for 7.2 also because you've asked for some changes.

I think the Sashiko reviews alone make it very clear that this needs
some more work.  So if you're considering this to be ready and just
waiting for me, I don't think that's the best way to move this forwards.

> I'm not sure if it was mentioned before wrt how to get this merged. My
> preferred way is to get your Ack for the crypto changes and then we can
> add it to linux-next via our btrfs git.

That will be fine once it's ready.

- Eric