linux-kernel - Re: [PATCH] btrfs: fix an assertion failure related to squota feature.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAL3q7H5DhmjX_G7PGQQ40O7wtaV7TqFp_7L9yL7asjLA27J8pA@mail.gmail.com>
Date: Mon, 16 Dec 2024 17:35:34 +0000
From: Filipe Manana <fdmanana@...nel.org>
To: Julian Sun <sunjunchao2870@...il.com>
Cc: linux-btrfs@...r.kernel.org, linux-kernel@...r.kernel.org, clm@...com, 
	josef@...icpanda.com, dsterba@...e.com, wqu@...e.com
Subject: Re: [PATCH] btrfs: fix an assertion failure related to squota feature.

On Wed, Dec 11, 2024 at 11:13 AM Julian Sun <sunjunchao2870@...il.com> wrote:
>
> With the config CONFIG_BTRFS_ASSERT enabled, an assertion
> failure occurs regarding the simple quota feature.
>
> [    5.596534] assertion failed: btrfs_fs_incompat(fs_info, SIMPLE_QUOTA), in fs/btrfs/qgroup.c:365
> [    5.597098] ------------[ cut here ]------------
> [    5.597371] kernel BUG at fs/btrfs/qgroup.c:365!
> [    5.597946] CPU: 1 UID: 0 PID: 268 Comm: mount Not tainted 6.13.0-rc2-00031-gf92f4749861b #146
> [    5.598450] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
> [    5.599008] RIP: 0010:btrfs_read_qgroup_config+0x74d/0x7a0
> [    5.604303]  <TASK>
> [    5.605230]  ? btrfs_read_qgroup_config+0x74d/0x7a0
> [    5.605538]  ? exc_invalid_op+0x56/0x70
> [    5.605775]  ? btrfs_read_qgroup_config+0x74d/0x7a0
> [    5.606066]  ? asm_exc_invalid_op+0x1f/0x30
> [    5.606441]  ? btrfs_read_qgroup_config+0x74d/0x7a0
> [    5.606741]  ? btrfs_read_qgroup_config+0x74d/0x7a0
> [    5.607038]  ? try_to_wake_up+0x317/0x760
> [    5.607286]  open_ctree+0xd9c/0x1710
> [    5.607509]  btrfs_get_tree+0x58a/0x7e0
> [    5.608002]  vfs_get_tree+0x2e/0x100
> [    5.608224]  fc_mount+0x16/0x60
> [    5.608420]  btrfs_get_tree+0x2f8/0x7e0
> [    5.608897]  vfs_get_tree+0x2e/0x100
> [    5.609121]  path_mount+0x4c8/0xbc0
> [    5.609538]  __x64_sys_mount+0x10d/0x150
>
> The issue can be easily reproduced using the following reproduer:
> root@q:linux# cat repro.sh
> set -e
>
> mkfs.btrfs -f /dev/sdb > /dev/null
> mount /dev/sdb /mnt/btrfs
> btrfs quota enable -s /mnt/btrfs
> umount /mnt/btrfs
> mount /dev/sdb /mnt/btrfs
>
> The root cause of this issue is as follows:
> When simple quota is enabled, the BTRFS_FEATURE_INCOMPAT_SIMPLE_QUOTA
> flag is set after btrfs_commit_transaction(), whereas the
> BTRFS_QGROUP_STATUS_FLAG_SIMPLE_MODE flag is set before btrfs_commit_transaction(),
> which led to the first flag not being flushed to disk, and the second
> flag is successfully flushed. Finally causes this assertion failure
> after umount && mount again.
>
> To resolve this issue, the BTRFS_FEATURE_INCOMPAT_SIMPLE_QUOTA flag
> is set immediately after setting the BTRFS_QGROUP_STATUS_FLAG_SIMPLE_MODE.
> This ensures that both flags are flushed to disk within the same
> transaction.
>
> Signed-off-by: Julian Sun <sunjunchao2870@...il.com>

Fixes: 182940f4f4db ("btrfs: qgroup: add new quota mode for simple quotas")
Reviewed-by: Filipe Manana <fdmanana@...e.com>

Looks good, thanks.
We should also turn the reproducer into a test case for fstests.

> ---
>  fs/btrfs/qgroup.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index a6f92836c9b1..f9b214992212 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -1121,6 +1121,7 @@ int btrfs_quota_enable(struct btrfs_fs_info *fs_info,
>         fs_info->qgroup_flags = BTRFS_QGROUP_STATUS_FLAG_ON;
>         if (simple) {
>                 fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_SIMPLE_MODE;
> +               btrfs_set_fs_incompat(fs_info, SIMPLE_QUOTA);
>                 btrfs_set_qgroup_status_enable_gen(leaf, ptr, trans->transid);
>         } else {
>                 fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
> @@ -1254,8 +1255,6 @@ int btrfs_quota_enable(struct btrfs_fs_info *fs_info,
>         spin_lock(&fs_info->qgroup_lock);
>         fs_info->quota_root = quota_root;
>         set_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags);
> -       if (simple)
> -               btrfs_set_fs_incompat(fs_info, SIMPLE_QUOTA);
>         spin_unlock(&fs_info->qgroup_lock);
>
>         /* Skip rescan for simple qgroups. */
> --
> 2.39.5
>
>