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

[Horst Birthelmer <horst@birthelmer.de>]

On Fri, Mar 27, 2026 at 08:40:49AM -0700, Joanne Koong wrote:
> On Fri, Mar 27, 2026 at 12:36 AM Horst Birthelmer <horst@birthelmer.com> wrote:
> >
> > From: Horst Birthelmer <hbirthelmer@ddn.com>
> >
> > Fix a race between fuse_iget() and fuse_reverse_inval_inode() where
> > invalidation can arrive while an inode is being initialized, causing
> > the invalidation to be lost.
> > By keeping the inode state I_NEW as long as the attributes are not valid
> > the invalidation can wait until the inode is fully initialized.
> >
> > Suggested-by: Joanne Koong <joannelkoong@gmail.com>
> > Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
> > ---
> > Changes in v2:
> > - switch from waitq guided by attr_version to wait_on_new_inode() and unlock_new_inode() when
> > the inode is fully initialized
> > - Link to v1: https://lore.kernel.org/r/20260318-fix-inode-init-race-v1-1-a7e58b2ddb9a@ddn.com
> > ---
> >  fs/fuse/inode.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> > index e57b8af06be93ecc29c58864a9c9e99c68e3283b..fa0adc2bbe58cee6f63153c60c401b78ec3695bf 100644
> > --- a/fs/fuse/inode.c
> > +++ b/fs/fuse/inode.c
> > @@ -470,6 +470,7 @@ struct inode *fuse_iget(struct super_block *sb, u64 nodeid,
> >         struct inode *inode;
> >         struct fuse_inode *fi;
> >         struct fuse_conn *fc = get_fuse_conn_super(sb);
> > +       bool is_new_inode = false;
> >
> >         /*
> >          * Auto mount points get their node id from the submount root, which is
> > @@ -505,13 +506,13 @@ struct inode *fuse_iget(struct super_block *sb, u64 nodeid,
> >         if (!inode)
> >                 return NULL;
> >
> > -       if ((inode_state_read_once(inode) & I_NEW)) {
> > +       is_new_inode = inode_state_read_once(inode) & I_NEW;
> > +       if (is_new_inode) {
> >                 inode->i_flags |= S_NOATIME;
> >                 if (!fc->writeback_cache || !S_ISREG(attr->mode))
> >                         inode->i_flags |= S_NOCMTIME;
> >                 inode->i_generation = generation;
> >                 fuse_init_inode(inode, attr, fc);
> > -               unlock_new_inode(inode);
> >         } else if (fuse_stale_inode(inode, generation, attr)) {
> >                 /* nodeid was reused, any I/O on the old inode should fail */
> >                 fuse_make_bad(inode);
> > @@ -528,6 +529,8 @@ struct inode *fuse_iget(struct super_block *sb, u64 nodeid,
> >  done:
> >         fuse_change_attributes_i(inode, attr, NULL, attr_valid, attr_version,
> >                                  evict_ctr);
> > +       if (is_new_inode)
> > +               unlock_new_inode(inode);
> >         return inode;
> >  }
> >
> > @@ -565,6 +568,9 @@ int fuse_reverse_inval_inode(struct fuse_conn *fc, u64 nodeid,
> >         if (!inode)
> >                 return -ENOENT;
> >
> > +       /* Wait for inode initialization to complete */
> > +       wait_on_new_inode(inode);
> 
> This isn't needed.
> 
> The fuse_ilookup() -> ilookup5() already waits on I_NEW.

I actually suspected that and looked for it, but did not see it in ilookup5()
Sorry about that.

> 
> Thanks,
> Joanne
> 
> > +
> >         fi = get_fuse_inode(inode);
> >         spin_lock(&fi->lock);
> >         fi->attr_version = atomic64_inc_return(&fc->attr_version);
> >
> > ---
> > base-commit: f338e77383789c0cae23ca3d48adcc5e9e137e3c
> > change-id: 20260318-fix-inode-init-race-a47a7ba4af1e
> >
> > Best regards,
> > --
> > Horst Birthelmer <hbirthelmer@ddn.com>
> >