| 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
| ||
|
Message-ID: <CAL3q7H44KsXkB4Q=y5YnGrAVW91S73mac+9ObvC+S6oO6uih2Q@mail.gmail.com>
Date: Tue, 17 Dec 2024 13:20:38 +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 Mon, Dec 16, 2024 at 5:35 PM Filipe Manana <fdmanana@...nel.org> wrote:
>
> 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.
Pushed it to the for-next branch with a few changes to the changelog:
1) Renamed the subject to "btrfs: fix transaction atomicity bug when
enabling simple quotas".
Note that we don't add punctuation at the end of the subject line.
2) Changed the paragraphs that explain the bug below the reproducer to:
"The issue is that when enabling quotas, at btrfs_quota_enable(), we set
BTRFS_QGROUP_STATUS_FLAG_SIMPLE_MODE at fs_info->qgroup_flags and persist
it in the quota root in the item with the key BTRFS_QGROUP_STATUS_KEY, but
we only set the incompat bit BTRFS_FEATURE_INCOMPAT_SIMPLE_QUOTA after we
commit the transaction used to enable simple quotas.
This means that if after that transaction commit we unmount the filesystem
without starting and committing any other transaction, or we have a power
failure, the next time we mount the filesystem we will find the flag
BTRFS_QGROUP_STATUS_FLAG_SIMPLE_MODE set in the item with the key
BTRFS_QGROUP_STATUS_KEY but we will not find the incompat bit
BTRFS_FEATURE_INCOMPAT_SIMPLE_QUOTA set in the superblock, triggering an
assertion failure at:
btrfs_read_qgroup_config() -> qgroup_read_enable_gen()
To fix this issue, set the BTRFS_FEATURE_INCOMPAT_SIMPLE_QUOTA flag
immediately after setting the BTRFS_QGROUP_STATUS_FLAG_SIMPLE_MODE.
This ensures that both flags are flushed to disk within the same
transaction."
Thanks.
>
> > ---
> > 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
> >
> >
Powered by blists - more mailing lists