[PATCH v2] fuse: fix inode initialization race

Horst Birthelmer posted 1 patch 6 days, 7 hours ago
There is a newer version of this series
fs/fuse/inode.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
[PATCH v2] fuse: fix inode initialization race
Posted by Horst Birthelmer 6 days, 7 hours ago
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);
+
 	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>
Re: [PATCH v2] fuse: fix inode initialization race
Posted by Joanne Koong 5 days, 23 hours ago
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.

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>
>
Re: Re: [PATCH v2] fuse: fix inode initialization race
Posted by Horst Birthelmer 5 days, 22 hours ago
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>
> >