Re: [PATCH v6 42/43] btrfs: disable encryption on RAID5/6

[Chris Mason <clm@meta.com>]

Daniel Vacek <neelx@suse.com> wrote:
> From: Josef Bacik <josef@toxicpanda.com>
> 
> 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 <josef@toxicpanda.com>

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 <josef@toxicpanda.com>
> 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

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?