On Sun, 8 Feb 2026 at 14:18, Chris Mason <[email protected]> wrote:
> Daniel Vacek <[email protected]> wrote:
> > From: Josef Bacik <[email protected]>
> >
> > The RAID5/6 code will re-arrange bios and submit them through a
> > different mechanism. This is problematic with inline encryption as we
> > have to get the bio and csum it after it's been encrypted, and the
> > radi5/6 bio's don't have the btrfs_bio embedded, so we have no way to
> > get the csums put on disk.
> >
> > This isn't an unsolvable problem, but would require a bit of reworking.
> > Since we discourage users from using this code currently simply don't
> > allow encryption on RAID5/6 setups. If there's sufficient demand in the
> > future we can add the support for this.
> >
>
> Hi everyone,
>
> I'm running the btrfs for-next branch, along with the encryption patch set
> through my AI review prompts:
>
> https://github.com/masoncl/review-prompts
>
> I'm working on some btrfs specific instructions, but hopefully these reviews
> are already useful. I've kept the headers to make it easier to double check
> that I'm actually replying to the right email, but please let me know if any
> parts of this are incorrect:
>
>
> commit 6a39a21e95e116ad02c6ce9f832838acc111692f
> Author: Josef Bacik <[email protected]>
>
> btrfs: disable encryption on RAID5/6
>
> The RAID5/6 code will re-arrange bios and submit them through a
> different mechanism to handle parity. This is problematic with inline
> encryption as we have to get the bio and csum it after it's been
> encrypted, and the RAID5/6 bios don't have the btrfs_bio embedded, so
> there's no way to get the csums put on disk.
>
> > commit 6a39a21e95e116ad02c6ce9f832838acc111692f
> > Author: Josef Bacik <[email protected]>
> > Date: Fri Feb 6 19:23:14 2026 +0100
> >
> > btrfs: disable encryption on RAID5/6
> >
> > The RAID5/6 code will re-arrange bios and submit them through a
> > different mechanism. This is problematic with inline encryption as we
> > have to get the bio and csum it after it's been encrypted, and the
> > radi5/6 bio's don't have the btrfs_bio embedded, so we have no way to
> ^^^^
>
> This isn't a bug, but there's a typo: 'radi5/6' should be 'raid5/6'.
>
> [ ... ]
>
> >
> > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> > index a8adf99ad0a81..1bade8fea16e8 100644
> > --- a/fs/btrfs/ioctl.c
> > +++ b/fs/btrfs/ioctl.c
> > @@ -5198,6 +5198,10 @@ long btrfs_ioctl(struct file *file, unsigned int
> > return -EOPNOTSUPP;
> > if (sb_rdonly(fs_info->sb))
> > return -EROFS;
> > + if (btrfs_fs_incompat(fs_info, RAID56)) {
> > + btrfs_warn(fs_info, "can't enable encryption with
> > RAID5/6");
> > + return -EINVAL;
> > + }
>
> This check prevents enabling encryption when RAID56 is already active,
> but does NOT prevent converting to RAID5/6 after encryption is enabled.
> The check_raid56_incompat_flag() function in fs/btrfs/volumes.c:5558
> sets the RAID56 flag during balance operations without checking if
> ENCRYPT is already set. This allows users to:
>
> 1. Enable encryption via FS_IOC_SET_ENCRYPTION_POLICY (sets ENCRYPT at
> line 5210 in this function)
> 2. Later run btrfs balance to convert to RAID5/6 (sets RAID56 flag at
> volumes.c:5558)
> 3. End up with both ENCRYPT and RAID56 flags set
Indeed this should be sealed. I think this will do:
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -5742,6 +5742,11 @@ struct btrfs_block_group
*btrfs_create_chunk(struct btrfs_trans_handle *trans,
lockdep_assert_held(&info->chunk_mutex);
+ if (btrfs_fs_incompat(info, ENCRYPT)) {
+ btrfs_warn(info, "RAID5/6 not yet supported on encrypted filesystem");
+ return ERR_PTR(-EINVAL);
+ }
+
if (!alloc_profile_is_valid(type, 0)) {
DEBUG_WARN("invalid alloc profile for type %llu", type);
return ERR_PTR(-EINVAL);
> The commit message states "simply don't allow encryption on RAID5/6
> setups" which implies preventing the incompatible state entirely, not
> just one entry path. The volumes.c code path needs a corresponding
> check:
>
> static void check_raid56_incompat_flag(struct btrfs_fs_info *info, u64 type)
> {
> if (!(type & BTRFS_BLOCK_GROUP_RAID56_MASK))
> return;
> // MISSING: check for ENCRYPT flag here
> btrfs_set_fs_incompat(info, RAID56);
> }
>
> Without this check, data corruption can occur via the RAID5/6 bio path
> that cannot handle encrypted data properly, as explained in the commit
> message.
>
> > diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> > index 4a2887147ead4..aefcbe56e85ad 100644
> > --- a/fs/btrfs/super.c
> > +++ b/fs/btrfs/super.c
> > @@ -734,6 +734,12 @@ bool btrfs_check_options(const struct btrfs_fs_info
> > *info,
> > if (btrfs_check_mountopts_zoned(info, mount_opt))
> > ret = false;
> >
> > + if (btrfs_fs_incompat(info, RAID56) &&
> > + btrfs_raw_test_opt(*mount_opt, TEST_DUMMY_ENCRYPTION)) {
> > + btrfs_err(info, "cannot use test_dummy_encryption with
> > RAID5/6");
> > + ret = false;
> > + }
> > +
>
> Similar to the ioctl.c issue, this prevents mounting with
> test_dummy_encryption when RAID56 is already active, but does NOT
> prevent converting to RAID5/6 after mounting with test_dummy_encryption.
>
> The check_raid56_incompat_flag() function in fs/btrfs/volumes.c:5558
> sets the RAID56 flag during balance operations without checking if
> test_dummy_encryption is in use. Since test_dummy_encryption cannot be
> changed after mount (enforced by btrfs_check_test_dummy_encryption at
> line 1547-1560), a user can:
>
> 1. Mount filesystem with -o test_dummy_encryption
> 2. Later run btrfs balance to convert to RAID5/6 (sets RAID56 flag)
> 3. End up with both test_dummy_encryption active and RAID56 flag set
>
> While test_dummy_encryption is primarily a testing/development feature,
> the patch explicitly protects against this combination, indicating it's
> an important incompatibility. The volumes.c code path needs a check for
> the TEST_DUMMY_ENCRYPTION mount option:
>
> static void check_raid56_incompat_flag(struct btrfs_fs_info *info, u64 type)
> {
> if (!(type & BTRFS_BLOCK_GROUP_RAID56_MASK))
> return;
> // MISSING: check for TEST_DUMMY_ENCRYPTION mount option
> btrfs_set_fs_incompat(info, RAID56);
> }
>
> Does this allow the same encrypted bio / RAID5/6 bio incompatibility
> described in the commit message?
The above fix also covers this case.
Thanks.
--nX
