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>] [day] [month] [year] [list]
Date:   Sat, 11 Nov 2023 17:24:42 +1030
From:   Qu Wenruo <quwenruo.btrfs@....com>
To:     Edward Adam Davis <eadavis@...com>,
        syzbot+4d81015bc10889fd12ea@...kaller.appspotmail.com
Cc:     boris@....io, clm@...com, dsterba@...e.com, josef@...icpanda.com,
        linux-btrfs@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org, syzkaller-bugs@...glegroups.com
Subject: Re: [PATCH] btrfs: fix warning in create_pending_snapshot



On 2023/11/11 15:36, Edward Adam Davis wrote:
> The create_snapshot will use the objectid that already exists in the qgroup_tree
> tree, so when calculating the free_ojectid, it is added to determine whether it
> exists in the qgroup_tree tree.
>
> Reported-and-tested-by: syzbot+4d81015bc10889fd12ea@...kaller.appspotmail.com
> Fixes: 6ed05643ddb1 ("btrfs: create qgroup earlier in snapshot creation")
> Signed-off-by: Edward Adam Davis <eadavis@...com>
> ---
>   fs/btrfs/disk-io.c | 3 ++-
>   fs/btrfs/qgroup.c  | 2 +-
>   fs/btrfs/qgroup.h  | 2 ++
>   3 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 401ea09ae4b8..97050a3edc32 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -4931,7 +4931,8 @@ int btrfs_get_free_objectid(struct btrfs_root *root, u64 *objectid)
>   		goto out;
>   	}
>
> -	*objectid = root->free_objectid++;
> +	while (find_qgroup_rb(root->fs_info, root->free_objectid++));

I don't think this is correct.

Firstly you didn't take qgroup_ioctl_lock.

Secondly, please explain why you believe the free objectid of a
subvolume is related to the qgroup id?


For any one who really wants to fix the syzbot bug, please explain the
bug clearly before doing any fix.

If you can not explain the bug clearly, then you're doing it wrong.

Thanks,
Qu

> +	*objectid = root->free_objectid;
>   	ret = 0;
>   out:
>   	mutex_unlock(&root->objectid_mutex);
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index edb84cc03237..3705e7d57057 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -171,7 +171,7 @@ qgroup_rescan_init(struct btrfs_fs_info *fs_info, u64 progress_objectid,
>   static void qgroup_rescan_zero_tracking(struct btrfs_fs_info *fs_info);
>
>   /* must be called with qgroup_ioctl_lock held */
> -static struct btrfs_qgroup *find_qgroup_rb(struct btrfs_fs_info *fs_info,
> +struct btrfs_qgroup *find_qgroup_rb(struct btrfs_fs_info *fs_info,
>   					   u64 qgroupid)
>   {
>   	struct rb_node *n = fs_info->qgroup_tree.rb_node;
> diff --git a/fs/btrfs/qgroup.h b/fs/btrfs/qgroup.h
> index 855a4f978761..96c6aa31ca91 100644
> --- a/fs/btrfs/qgroup.h
> +++ b/fs/btrfs/qgroup.h
> @@ -425,4 +425,6 @@ bool btrfs_check_quota_leak(struct btrfs_fs_info *fs_info);
>   int btrfs_record_squota_delta(struct btrfs_fs_info *fs_info,
>   			      struct btrfs_squota_delta *delta);
>
> +struct btrfs_qgroup *find_qgroup_rb(struct btrfs_fs_info *fs_info,
> +                                            u64 qgroupid);
>   #endif

Powered by blists - more mailing lists