On 2025/07/10 7:03, Tetsuo Handa wrote: > On 2025/07/10 3:33, Viacheslav Dubeyko wrote: >> My worry that we could have a race condition here. Let's imagine that two >> threads are trying to call __hfsplus_setxattr() and both will try to create the >> Attributes File. Potentially, we could end in situation when inode could have >> not zero size during hfsplus_create_attributes_file() in one thread because >> another thread in the middle of Attributes File creation. Could we double check >> that we don't have the race condition here? Otherwise, we need to make much >> cleaner fix of this issue. > > I think that there is some sort of race window, for > https://elixir.bootlin.com/linux/v6.15.5/source/fs/hfsplus/xattr.c#L145 > explains that if more than one thread concurrently reached > > if (!HFSPLUS_SB(inode->i_sb)->attr_tree) { > err = hfsplus_create_attributes_file(inode->i_sb); > if (unlikely(err)) > goto end_setxattr; > } > > path, all threads except one thread will fail with -EAGAIN. > Do you prefer stricter mount-time validation shown below? Is vhdr->attr_file.total_blocks == 0 when sbi->attr_tree exists and is empty? diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c index 948b8aaee33e..f6324a0458f3 100644 --- a/fs/hfsplus/super.c +++ b/fs/hfsplus/super.c @@ -482,13 +482,17 @@ static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc) goto out_close_ext_tree; } atomic_set(&sbi->attr_tree_state, HFSPLUS_EMPTY_ATTR_TREE); - if (vhdr->attr_file.total_blocks != 0) { - sbi->attr_tree = hfs_btree_open(sb, HFSPLUS_ATTR_CNID); - if (!sbi->attr_tree) { - pr_err("failed to load attributes file\n"); - goto out_close_cat_tree; + sbi->attr_tree = hfs_btree_open(sb, HFSPLUS_ATTR_CNID); + if (sbi->attr_tree) { + if (vhdr->attr_file.total_blocks != 0) { + atomic_set(&sbi->attr_tree_state, HFSPLUS_VALID_ATTR_TREE); + } else { + pr_err("found attributes file despite total blocks is 0\n"); + goto out_close_attr_tree; } - atomic_set(&sbi->attr_tree_state, HFSPLUS_VALID_ATTR_TREE); + } else if (vhdr->attr_file.total_blocks != 0) { + pr_err("failed to load attributes file\n"); + goto out_close_cat_tree; } sb->s_xattr = hfsplus_xattr_handlers;
