Re: Re: [PATCH] fuse: fix inode initialization race

[Joanne Koong]

On Thu, Mar 26, 2026 at 8:48 AM Horst Birthelmer <horst@xxxxxxxxxxxxx> wrote:
>
> On Thu, Mar 26, 2026 at 04:19:24PM +0100, Miklos Szeredi wrote:
> > On Thu, 26 Mar 2026 at 16:13, Bernd Schubert <bernd@xxxxxxxxxxx> wrote:
> > >
> > >
> > >
> > > On 3/26/26 15:26, Christian Brauner wrote:
> > > > On Wed, Mar 25, 2026 at 08:54:57AM +0100, Bernd Schubert wrote:
> > > >>
> > > >>
> > > >> On 3/18/26 14:43, Horst Birthelmer wrote:
> > > >>> From: Horst Birthelmer <hbirthelmer@xxxxxxx>
> >
> > > >>>     fi->attr_version = atomic64_inc_return(&fc->attr_version);
> > > >>> +   wake_up_all(&fc->attr_version_waitq);
> > > >>>     fi->i_time = attr_valid;
> > >
> > >
> > > While I'm looking at this again, wouldn't it make sense to make this
> > > conditional? Because we wake this queue on every attr change for every
> > > inode. And the conditional in fuse_iget() based on I_NEW?
> >
> > Right, should only wake if fi->attr_version old value was zero.
> >
> > BTW I have a hunch that there are better solutions, but it's simple
> > enough as a stopgap measure.
>
> OK, I'll send a new version.
>
> Just out of curiosity, what would be a better solution?

I'm probably missing something here but why can't we just call the

        fi = get_fuse_inode(inode);
        spin_lock(&fi->lock);
        fi->nlookup++;
        spin_unlock(&fi->lock);
        fuse_change_attributes_i(inode, attr, NULL, attr_valid, attr_version,
                                 evict_ctr);

logic before releasing the inode lock (the unlock_new_inode() call) in
fuse_iget() to address the race? unlock_new_inode() clears I_NEW so
fuse_reverse_inval_inode()'s fuse_ilookup() would only get the inode
after the attributes initialization has finished.

As I understand it, fuse_change_attributes_i() would be pretty
straightforward / fast for I_NEW inodes, as it doesn't send any
synchronous requests and for the I_NEW case the
invalidate_inode_pages2() and truncate_pagecache() calls would get
skipped. (truncate_pagecache() getting skipped because inode->i_size
is already attr->size from fuse_init_inode(), so "oldsize !=
attr->size" is never true; and invalidate_inode_pages2() getting
skipped because "oldsize != attr->size" is never true and "if
(!timespec64_equal(&old_mtime, &new_mtime))" is never true because
fuse_init_inode() initialized the inode's mtime to attr->mtime).

Thanks,
Joanne

>
> >
> > Thanks,
> > Miklos
>
> Thanks,
> Horst
>