Re: [PATCH 6/6] xfs: don't report half-built inodes to fserror

["Darrick J. Wong" <djwong@kernel.org>]

On Tue, Feb 24, 2026 at 12:39:08PM +0100, Carlos Maiolino wrote:
> On Thu, Feb 19, 2026 at 02:02:23PM -0800, Darrick J. Wong wrote:
> > On Thu, Feb 19, 2026 at 02:21:36PM +0100, Carlos Maiolino wrote:
> > > On Wed, Feb 18, 2026 at 10:02:17PM -0800, Darrick J. Wong wrote:
> > > > From: Darrick J. Wong <djwong@kernel.org>
> > > > 
> > > > Sam Sun apparently found a syzbot way to fuzz a filesystem such that
> > > > xfs_iget_cache_miss would free the inode before the fserror code could
> > > > catch up.  Frustratingly he doesn't use the syzbot dashboard so there's
> > > > no C reproducer and not even a full error report, so I'm guessing that:
> > > > 
> > > > Inodes that are being constructed or torn down inside XFS are not
> > > > visible to the VFS.  They should never be reported to fserror.
> > > > Also, any inode that has been freshly allocated in _cache_miss should be
> > > > marked INEW immediately because, well, it's an incompletely constructed
> > > > inode that isn't yet visible to the VFS.
> > > > 
> > > > Reported-by: Sam Sun <samsun1006219@gmail.com>
> > > > Fixes: 5eb4cb18e445d0 ("xfs: convey metadata health events to the health monitor")
> > > > Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> > > 
> > > The change looks ok to me. Would be nice though if we can be sure this
> > > fix the reporter's issue. Any idea if the reporter could reproduce it?
> > 
> > It sounds like syzbot found a reproducer for the reporter, but they will
> > not integrate with Google's syzbot dashboard or stand up their own
> > instance so I can't download it on my own; and they only posted a very
> > large strace so someone would have to turn that into a C program.
> > 
> > This is rude behavior, and egregiously so when the reporter **has an
> > automated fuzzer** that spat out a C program somewhere, but they won't
> > share.
> 
> Agreed.
> 
> > 
> > > Otherwise pointing this as a fix to a problem we can't be sure has
> > > actually been fixed, sounds misleading at best.
> > 
> > I don't know what to do unless the reporter builds a patched kernel and
> > tests it for us.
> 
> Chances are.... we never hear anything else again, but hopefully I'm
> wrong :)
> 
> My suggestion would be to leave the Fixes tag off, to avoid stable
> backports that "might not really fix" the problem, but this is a
> suggestion only, I'm ok to have it anyway.

I didn't cc stable, so this isn't going to get autobackported.  TBH, if
someone /does/ decide to backport all this manually, we ought to leave
them some breadcrumbs of what else needs to be pulled in.

FWIW the patch actually /does/ fix a real problem -- we shouldn't be
exposing XFS_INEW inodes to the VFS, but we do want the failure to be
recorded (with inumber) by the health monitoring system so that
xfs_healer can actually try to repair the inode, so we have to keep the
xfs_inode_mark_sick call in the iget miss code.

I don't know if that's the root cause of the problem that syzbot
reported, but I guess we'll never know.

--D

> > > For the fix itself though:
> > > 
> > > Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
> > 
> > Thanks for reviewing.
> > 
> > --D
> > 
> > > 
> > > > ---
> > > >  fs/xfs/xfs_health.c |    8 ++++++--
> > > >  fs/xfs/xfs_icache.c |    9 ++++++++-
> > > >  2 files changed, 14 insertions(+), 3 deletions(-)
> > > > 
> > > > 
> > > > diff --git a/fs/xfs/xfs_health.c b/fs/xfs/xfs_health.c
> > > > index 6475159eb9302c..239b843e83d42a 100644
> > > > --- a/fs/xfs/xfs_health.c
> > > > +++ b/fs/xfs/xfs_health.c
> > > > @@ -316,8 +316,12 @@ xfs_rgno_mark_sick(
> > > >  
> > > >  static inline void xfs_inode_report_fserror(struct xfs_inode *ip)
> > > >  {
> > > > -	/* Report metadata inodes as general filesystem corruption */
> > > > -	if (xfs_is_internal_inode(ip)) {
> > > > +	/*
> > > > +	 * Do not report inodes being constructed or freed, or metadata inodes,
> > > > +	 * to fsnotify.
> > > > +	 */
> > > > +	if (xfs_iflags_test(ip, XFS_INEW | XFS_IRECLAIM) ||
> > > > +	    xfs_is_internal_inode(ip)) {
> > > >  		fserror_report_metadata(ip->i_mount->m_super, -EFSCORRUPTED,
> > > >  				GFP_NOFS);
> > > >  		return;
> > > > diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> > > > index dbaab4ae709f9c..f13e55b75d66c4 100644
> > > > --- a/fs/xfs/xfs_icache.c
> > > > +++ b/fs/xfs/xfs_icache.c
> > > > @@ -636,6 +636,14 @@ xfs_iget_cache_miss(
> > > >  	if (!ip)
> > > >  		return -ENOMEM;
> > > >  
> > > > +	/*
> > > > +	 * Set XFS_INEW as early as possible so that the health code won't pass
> > > > +	 * the inode to the fserror code if the ondisk inode cannot be loaded.
> > > > +	 * We're going to free the xfs_inode immediately if that happens, which
> > > > +	 * would lead to UAF problems.
> > > > +	 */
> > > > +	xfs_iflags_set(ip, XFS_INEW);
> > > > +
> > > >  	error = xfs_imap(pag, tp, ip->i_ino, &ip->i_imap, flags);
> > > >  	if (error)
> > > >  		goto out_destroy;
> > > > @@ -713,7 +721,6 @@ xfs_iget_cache_miss(
> > > >  	ip->i_udquot = NULL;
> > > >  	ip->i_gdquot = NULL;
> > > >  	ip->i_pdquot = NULL;
> > > > -	xfs_iflags_set(ip, XFS_INEW);
> > > >  
> > > >  	/* insert the new inode */
> > > >  	spin_lock(&pag->pag_ici_lock);
> > > > 
> > > > 
> > > 
> > 
>