[Jfs-discussion] [PATCH RFC 10/28] hfsplus: don't set SB_RDONLY when creating the attributes tree

[Viacheslav Dubeyko]

On Mon, 2026-10-12 at 12:20 +0200, Christian Brauner via Jfs-discussion
wrote:
> hfs_btree_open() sets SB_RDONLY when the map record of a b-tree
> doesn't
> mark the header node as used. That's fine while mounting where the
> mounter holds sb->s_umount. But hfsplus_create_attributes_file()
> calls
> it too when the first extended attribute gets set on a filesystem
> that
> doesn't have an attributes file yet. That runs on the live filesystem
> without s_umount and a filesystem must not change SB_RDONLY behind
> the
> VFS' back.
> 
> hfsplus_create_attributes_file() opens a tree it has just created
> itself, with the header node marked as used. So there's nothing to
> check there. Only do the check while the filesystem is being mounted.
> 
> No functional changes.
> 
> Signed-off-by: Christian Brauner (Amutable) <brauner at kernel.org>
> ---
>  fs/hfsplus/btree.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/hfsplus/btree.c b/fs/hfsplus/btree.c
> index 2ea8cd5658e1..967c457d843d 100644
> --- a/fs/hfsplus/btree.c
> +++ b/fs/hfsplus/btree.c
> @@ -385,7 +385,12 @@ struct hfs_btree *hfs_btree_open(struct
> super_block *sb, u32 id)
>  	if (IS_ERR(node))
>  		goto free_inode;
>  
> -	if (!hfs_bmap_test_bit(node, 0)) {
> +	/*
> +	 * Only a mount may set SB_RDONLY, later that needs
> s_umount. After the
> +	 * mount this only opens the attributes tree that
> +	 * hfsplus_create_attributes_file() just created with the
> bit set.
> +	 */
> +	if (!(sb->s_flags & SB_BORN) && !hfs_bmap_test_bit(node, 0))
> {
>  		pr_warn("(%s): %s (cnid 0x%x) map record invalid or
> bitmap corruption detected, forcing read-only.\n",
>  				sb->s_id, hfs_btree_name(id), id);
>  		pr_warn("Run fsck.hfsplus to repair.\n");

Yes, we can do this. But, maybe, we need to move this check:

	node = hfs_bnode_find(tree, HFSPLUS_TREE_HEAD);
	if (IS_ERR(node))
		goto free_inode;

	if (!hfs_bmap_test_bit(node, 0)) {
		pr_warn("(%s): %s (cnid 0x%x) map record invalid or
bitmap corruption detected, forcing read-only.\n",
				sb->s_id, hfs_btree_name(id), id);
		pr_warn("Run fsck.hfsplus to repair.\n");
		sb->s_flags |= SB_RDONLY;
	}

	hfs_bnode_put(node);

into hfsplus_fill_super(). Because, then main point of this check
namely check the b-trees during mount. If we create Attributes File
during setting a first xattr, then map record is always valid. It
sounds to me like more clean solution. What do you think?

Thanks,
Slava.