[PATCHSET 1/2] xfs: bug fixes for 7.0

[PATCHSET 1/2] xfs: bug fixes for 7.0

[Darrick J. Wong]

Hi all,

Bug fixes for 7.0.

If you're going to start using this code, I strongly recommend pulling
from my git trees, which are linked below.

With a bit of luck, this should all go splendidly.
Comments and questions are, as always, welcome.

--D

kernel git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=xfs-fixes-7.0
---
Commits in this patchset:
 * xfs: fix copy-paste error in previous fix
 * xfs: fix xfs_group release bug in xfs_verify_report_losses
 * xfs: fix xfs_group release bug in xfs_verify_report_losses
 * xfs: fix potential pointer access race in xfs_healthmon_get
 * xfs: don't report metadata inodes to fserror
 * xfs: don't report half-built inodes to fserror
---
 fs/xfs/xfs_mount.h          |    2 +-
 fs/xfs/scrub/dir_repair.c   |    2 +-
 fs/xfs/xfs_health.c         |   20 ++++++++++++++++++--
 fs/xfs/xfs_healthmon.c      |   10 ++++++----
 fs/xfs/xfs_icache.c         |    9 ++++++++-
 fs/xfs/xfs_notify_failure.c |    4 ++--
 fs/xfs/xfs_verify_media.c   |    4 ++--
 7 files changed, 38 insertions(+), 13 deletions(-)

[PATCH 1/6] xfs: fix copy-paste error in previous fix

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Chris Mason noticed that there is a copy-paste error in a recent change
to xrep_dir_teardown that nulls out pointers after freeing the
resources.

Fixes: ba408d299a3bb3c ("xfs: only call xf{array,blob}_destroy if we have a valid pointer")
Link: https://lore.kernel.org/linux-xfs/20260205194211.2307232-1-clm@meta.com/
Reported-by: Chris Mason <clm@meta.com>
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/scrub/dir_repair.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/dir_repair.c b/fs/xfs/scrub/dir_repair.c
index f105e49f654bd1..6166d9dee90f13 100644
--- a/fs/xfs/scrub/dir_repair.c
+++ b/fs/xfs/scrub/dir_repair.c
@@ -177,7 +177,7 @@ xrep_dir_teardown(
 	rd->dir_names = NULL;
 	if (rd->dir_entries)
 		xfarray_destroy(rd->dir_entries);
-	rd->dir_names = NULL;
+	rd->dir_entries = NULL;
 }
 
 /* Set up for a directory repair. */

Re: [PATCH 1/6] xfs: fix copy-paste error in previous fix

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 1/6] xfs: fix copy-paste error in previous fix

[Carlos Maiolino]

On Wed, Feb 18, 2026 at 10:00:58PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Chris Mason noticed that there is a copy-paste error in a recent change
> to xrep_dir_teardown that nulls out pointers after freeing the
> resources.
> 
> Fixes: ba408d299a3bb3c ("xfs: only call xf{array,blob}_destroy if we have a valid pointer")
> Link: https://lore.kernel.org/linux-xfs/20260205194211.2307232-1-clm@meta.com/
> Reported-by: Chris Mason <clm@meta.com>
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> ---
>  fs/xfs/scrub/dir_repair.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 
> diff --git a/fs/xfs/scrub/dir_repair.c b/fs/xfs/scrub/dir_repair.c
> index f105e49f654bd1..6166d9dee90f13 100644
> --- a/fs/xfs/scrub/dir_repair.c
> +++ b/fs/xfs/scrub/dir_repair.c
> @@ -177,7 +177,7 @@ xrep_dir_teardown(
>  	rd->dir_names = NULL;
>  	if (rd->dir_entries)
>  		xfarray_destroy(rd->dir_entries);
> -	rd->dir_names = NULL;
> +	rd->dir_entries = NULL;
>  }
>  
>  /* Set up for a directory repair. */
> 

[PATCH 2/6] xfs: fix xfs_group release bug in xfs_verify_report_losses

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Chris Mason reports that his AI tools noticed that we were using
xfs_perag_put and xfs_group_put to release the group reference returned
by xfs_group_next_range.  However, the iterator function returns an
object with an active refcount, which means that we must use the correct
function to release the active refcount, which is _rele.

Fixes: b8accfd65d31f ("xfs: add media verification ioctl")
Reported-by: Chris Mason <clm@meta.com>
Link: https://lore.kernel.org/linux-xfs/20260206030527.2506821-1-clm@meta.com/
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/xfs_verify_media.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


diff --git a/fs/xfs/xfs_verify_media.c b/fs/xfs/xfs_verify_media.c
index 069cd371619dc2..8bbd4ec567f8a1 100644
--- a/fs/xfs/xfs_verify_media.c
+++ b/fs/xfs/xfs_verify_media.c
@@ -122,7 +122,7 @@ xfs_verify_report_losses(
 
 			error = xfs_alloc_read_agf(pag, tp, 0, &agf_bp);
 			if (error) {
-				xfs_perag_put(pag);
+				xfs_perag_rele(pag);
 				break;
 			}
 
@@ -158,7 +158,7 @@ xfs_verify_report_losses(
 		if (rtg)
 			xfs_rtgroup_unlock(rtg, XFS_RTGLOCK_RMAP);
 		if (error) {
-			xfs_group_put(xg);
+			xfs_group_rele(xg);
 			break;
 		}
 	}

Re: [PATCH 2/6] xfs: fix xfs_group release bug in xfs_verify_report_losses

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 2/6] xfs: fix xfs_group release bug in xfs_verify_report_losses

[Carlos Maiolino]

On Wed, Feb 18, 2026 at 10:01:14PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Chris Mason reports that his AI tools noticed that we were using
> xfs_perag_put and xfs_group_put to release the group reference returned
> by xfs_group_next_range.  However, the iterator function returns an
> object with an active refcount, which means that we must use the correct
> function to release the active refcount, which is _rele.

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> 
> Fixes: b8accfd65d31f ("xfs: add media verification ioctl")
> Reported-by: Chris Mason <clm@meta.com>
> Link: https://lore.kernel.org/linux-xfs/20260206030527.2506821-1-clm@meta.com/
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> ---
>  fs/xfs/xfs_verify_media.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_verify_media.c b/fs/xfs/xfs_verify_media.c
> index 069cd371619dc2..8bbd4ec567f8a1 100644
> --- a/fs/xfs/xfs_verify_media.c
> +++ b/fs/xfs/xfs_verify_media.c
> @@ -122,7 +122,7 @@ xfs_verify_report_losses(
>  
>  			error = xfs_alloc_read_agf(pag, tp, 0, &agf_bp);
>  			if (error) {
> -				xfs_perag_put(pag);
> +				xfs_perag_rele(pag);
>  				break;
>  			}
>  
> @@ -158,7 +158,7 @@ xfs_verify_report_losses(
>  		if (rtg)
>  			xfs_rtgroup_unlock(rtg, XFS_RTGLOCK_RMAP);
>  		if (error) {
> -			xfs_group_put(xg);
> +			xfs_group_rele(xg);
>  			break;
>  		}
>  	}
> 
> 

[PATCH 3/6] xfs: fix xfs_group release bug in xfs_verify_report_losses

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Chris Mason reports that his AI tools noticed that we were using
xfs_perag_put and xfs_group_put to release the group reference returned
by xfs_group_next_range.  However, the iterator function returns an
object with an active refcount, which means that we must use the correct
function to release the active refcount, which is _rele.

Cc: <stable@vger.kernel.org> # v6.0
Fixes: 6f643c57d57c56 ("xfs: implement ->notify_failure() for XFS")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/xfs_notify_failure.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


diff --git a/fs/xfs/xfs_notify_failure.c b/fs/xfs/xfs_notify_failure.c
index 6be19fa1ebe262..64c8afb935c261 100644
--- a/fs/xfs/xfs_notify_failure.c
+++ b/fs/xfs/xfs_notify_failure.c
@@ -304,7 +304,7 @@ xfs_dax_notify_dev_failure(
 
 			error = xfs_alloc_read_agf(pag, tp, 0, &agf_bp);
 			if (error) {
-				xfs_perag_put(pag);
+				xfs_perag_rele(pag);
 				break;
 			}
 
@@ -340,7 +340,7 @@ xfs_dax_notify_dev_failure(
 		if (rtg)
 			xfs_rtgroup_unlock(rtg, XFS_RTGLOCK_RMAP);
 		if (error) {
-			xfs_group_put(xg);
+			xfs_group_rele(xg);
 			break;
 		}
 	}

Re: [PATCH 3/6] xfs: fix xfs_group release bug in xfs_verify_report_losses

[Christoph Hellwig]

On Wed, Feb 18, 2026 at 10:01:30PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Chris Mason reports that his AI tools noticed that we were using
> xfs_perag_put and xfs_group_put to release the group reference returned
> by xfs_group_next_range.  However, the iterator function returns an
> object with an active refcount, which means that we must use the correct
> function to release the active refcount, which is _rele.

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 3/6] xfs: fix xfs_group release bug in xfs_verify_report_losses

[Carlos Maiolino]

On Wed, Feb 18, 2026 at 10:01:30PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Chris Mason reports that his AI tools noticed that we were using
> xfs_perag_put and xfs_group_put to release the group reference returned
> by xfs_group_next_range.  However, the iterator function returns an
> object with an active refcount, which means that we must use the correct
> function to release the active refcount, which is _rele.

The subject looks a copy/paste from the previous one, ditto for the
description.

The description matches the patch, but the subject doesn't.

If you're going to send me a PR with this series, please fix it. If I'm
pulling this series straight from the list, I'll fix it here.

Other than the description problems, the patch looks fine:

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> 
> Cc: <stable@vger.kernel.org> # v6.0
> Fixes: 6f643c57d57c56 ("xfs: implement ->notify_failure() for XFS")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> ---
>  fs/xfs/xfs_notify_failure.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_notify_failure.c b/fs/xfs/xfs_notify_failure.c
> index 6be19fa1ebe262..64c8afb935c261 100644
> --- a/fs/xfs/xfs_notify_failure.c
> +++ b/fs/xfs/xfs_notify_failure.c
> @@ -304,7 +304,7 @@ xfs_dax_notify_dev_failure(
>  
>  			error = xfs_alloc_read_agf(pag, tp, 0, &agf_bp);
>  			if (error) {
> -				xfs_perag_put(pag);
> +				xfs_perag_rele(pag);
>  				break;
>  			}
>  
> @@ -340,7 +340,7 @@ xfs_dax_notify_dev_failure(
>  		if (rtg)
>  			xfs_rtgroup_unlock(rtg, XFS_RTGLOCK_RMAP);
>  		if (error) {
> -			xfs_group_put(xg);
> +			xfs_group_rele(xg);
>  			break;
>  		}
>  	}
> 
> 

Re: [PATCH 3/6] xfs: fix xfs_group release bug in xfs_verify_report_losses

[Darrick J. Wong]

On Thu, Feb 19, 2026 at 02:02:06PM +0100, Carlos Maiolino wrote:
> On Wed, Feb 18, 2026 at 10:01:30PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Chris Mason reports that his AI tools noticed that we were using
> > xfs_perag_put and xfs_group_put to release the group reference returned
> > by xfs_group_next_range.  However, the iterator function returns an
> > object with an active refcount, which means that we must use the correct
> > function to release the active refcount, which is _rele.
> 
> The subject looks a copy/paste from the previous one, ditto for the
> description.
> 
> The description matches the patch, but the subject doesn't.

Sorry about that.  Yes, the subject should have referenced
xfs_dax_notify_dev_failure.

> If you're going to send me a PR with this series, please fix it. If I'm
> pulling this series straight from the list, I'll fix it here.
> 
> Other than the description problems, the patch looks fine:
> 
> Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

I'll fix it before I send you a PR.

--D

> > 
> > Cc: <stable@vger.kernel.org> # v6.0
> > Fixes: 6f643c57d57c56 ("xfs: implement ->notify_failure() for XFS")
> > Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> > ---
> >  fs/xfs/xfs_notify_failure.c |    4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > 
> > diff --git a/fs/xfs/xfs_notify_failure.c b/fs/xfs/xfs_notify_failure.c
> > index 6be19fa1ebe262..64c8afb935c261 100644
> > --- a/fs/xfs/xfs_notify_failure.c
> > +++ b/fs/xfs/xfs_notify_failure.c
> > @@ -304,7 +304,7 @@ xfs_dax_notify_dev_failure(
> >  
> >  			error = xfs_alloc_read_agf(pag, tp, 0, &agf_bp);
> >  			if (error) {
> > -				xfs_perag_put(pag);
> > +				xfs_perag_rele(pag);
> >  				break;
> >  			}
> >  
> > @@ -340,7 +340,7 @@ xfs_dax_notify_dev_failure(
> >  		if (rtg)
> >  			xfs_rtgroup_unlock(rtg, XFS_RTGLOCK_RMAP);
> >  		if (error) {
> > -			xfs_group_put(xg);
> > +			xfs_group_rele(xg);
> >  			break;
> >  		}
> >  	}
> > 
> > 
> 

[PATCH 4/6] xfs: fix potential pointer access race in xfs_healthmon_get

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Pankaj Raghav asks about this code in xfs_healthmon_get:

  hm = mp->m_healthmon;
  if (hm && !refcount_inc_not_zero(&hm->ref))
    hm = NULL;
  rcu_read_unlock();
  return hm;

(slightly edited to compress a mailing list thread)

"Nit: Should we do a READ_ONCE(mp->m_healthmon) here to avoid any
compiler tricks that can result in an undefined behaviour? I am not sure
if I am being paranoid here.

"So this is my understanding: RCU guarantees that we get a valid object
(actual data of m_healthmon) but does not guarantee the compiler will
not reread the pointer between checking if hm is !NULL and accessing the
pointer as we are doing it lockless.

"So just a barrier() call in rcu_read_lock is enough to make sure this
doesn't happen and probably adding a READ_ONCE() is not needed?"

After some initial confusion I concluded that he's correct.  The
compiler could very well eliminate the hm variable in favor of walking
the pointers twice, turning the code into:

  if (mp->m_healthmon && !refcount_inc_not_zero(&mp->m_healthmon->ref))

If this happens, then xfs_healthmon_detach can sneak in between the
two sides of the && expression and set mp->m_healthmon to NULL, and
thereby cause a null pointer dereference crash.  Fix this by using the
rcu pointer assignment and dereference functions, which ensure that the
proper reordering barriers are in place.

Practically speaking, gcc seems to allocate an actual variable for hm
and only reads mp->m_healthmon once (as intended), but we ought to be
more explicit about requiring this.

Reported-by: Pankaj Raghav <pankaj.raghav@linux.dev>
Fixes: a48373e7d35a89f6f ("xfs: start creating infrastructure for health monitoring")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/xfs_mount.h     |    2 +-
 fs/xfs/xfs_healthmon.c |   10 ++++++----
 2 files changed, 7 insertions(+), 5 deletions(-)


diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index 61c71128d171cb..ddd4028be8d6ba 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -345,7 +345,7 @@ typedef struct xfs_mount {
 	struct xfs_hooks	m_dir_update_hooks;
 
 	/* Private data referring to a health monitor object. */
-	struct xfs_healthmon	*m_healthmon;
+	struct xfs_healthmon __rcu	*m_healthmon;
 } xfs_mount_t;
 
 #define M_IGEO(mp)		(&(mp)->m_ino_geo)
diff --git a/fs/xfs/xfs_healthmon.c b/fs/xfs/xfs_healthmon.c
index ca7352dcd182fb..4b5283fc74335c 100644
--- a/fs/xfs/xfs_healthmon.c
+++ b/fs/xfs/xfs_healthmon.c
@@ -69,7 +69,7 @@ xfs_healthmon_get(
 	struct xfs_healthmon		*hm;
 
 	rcu_read_lock();
-	hm = mp->m_healthmon;
+	hm = rcu_dereference(mp->m_healthmon);
 	if (hm && !refcount_inc_not_zero(&hm->ref))
 		hm = NULL;
 	rcu_read_unlock();
@@ -110,13 +110,13 @@ xfs_healthmon_attach(
 	struct xfs_healthmon	*hm)
 {
 	spin_lock(&xfs_healthmon_lock);
-	if (mp->m_healthmon != NULL) {
+	if (rcu_access_pointer(mp->m_healthmon) != NULL) {
 		spin_unlock(&xfs_healthmon_lock);
 		return -EEXIST;
 	}
 
 	refcount_inc(&hm->ref);
-	mp->m_healthmon = hm;
+	rcu_assign_pointer(mp->m_healthmon, hm);
 	hm->mount_cookie = (uintptr_t)mp->m_super;
 	spin_unlock(&xfs_healthmon_lock);
 
@@ -134,7 +134,9 @@ xfs_healthmon_detach(
 		return;
 	}
 
-	XFS_M((struct super_block *)hm->mount_cookie)->m_healthmon = NULL;
+	rcu_assign_pointer(XFS_M(
+			(struct super_block *)hm->mount_cookie)->m_healthmon,
+			NULL);
 	hm->mount_cookie = DETACHED_MOUNT_COOKIE;
 	spin_unlock(&xfs_healthmon_lock);
 

Re: [PATCH 4/6] xfs: fix potential pointer access race in xfs_healthmon_get

[Christoph Hellwig]

On Wed, Feb 18, 2026 at 10:01:45PM -0800, Darrick J. Wong wrote:
>  
> -	XFS_M((struct super_block *)hm->mount_cookie)->m_healthmon = NULL;
> +	rcu_assign_pointer(XFS_M(
> +			(struct super_block *)hm->mount_cookie)->m_healthmon,
> +			NULL);

Just a nitpick, but factoring the cookie to sb thing into a helper
or at least separate assignment would really clean this up.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 4/6] xfs: fix potential pointer access race in xfs_healthmon_get

[Carlos Maiolino]

On Wed, Feb 18, 2026 at 10:43:47PM -0800, Christoph Hellwig wrote:
> On Wed, Feb 18, 2026 at 10:01:45PM -0800, Darrick J. Wong wrote:
> >  
> > -	XFS_M((struct super_block *)hm->mount_cookie)->m_healthmon = NULL;
> > +	rcu_assign_pointer(XFS_M(
> > +			(struct super_block *)hm->mount_cookie)->m_healthmon,
> > +			NULL);
> 
> Just a nitpick, but factoring the cookie to sb thing into a helper
> or at least separate assignment would really clean this up.

Or perhaps just factor out a struct super_block *sb from it to untangle
it a bit.

I'm fine with any of the approaches though.

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

Re: [PATCH 4/6] xfs: fix potential pointer access race in xfs_healthmon_get

[Darrick J. Wong]

On Wed, Feb 18, 2026 at 10:43:47PM -0800, Christoph Hellwig wrote:
> On Wed, Feb 18, 2026 at 10:01:45PM -0800, Darrick J. Wong wrote:
> >  
> > -	XFS_M((struct super_block *)hm->mount_cookie)->m_healthmon = NULL;
> > +	rcu_assign_pointer(XFS_M(
> > +			(struct super_block *)hm->mount_cookie)->m_healthmon,
> > +			NULL);
> 
> Just a nitpick, but factoring the cookie to sb thing into a helper
> or at least separate assignment would really clean this up.

Ok, I'll do that.

	mp = XFS_M((struct super_block *)hm->mount_cookie);
	rcu_assign_pointer(mp->m_healthmon, NULL);

> Otherwise looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Thanks for the reviews!

--D

Re: [PATCH 4/6] xfs: fix potential pointer access race in xfs_healthmon_get

[Pankaj Raghav]

On 2/19/2026 7:01 AM, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Pankaj Raghav asks about this code in xfs_healthmon_get:
> 
>    hm = mp->m_healthmon;
>    if (hm && !refcount_inc_not_zero(&hm->ref))
>      hm = NULL;
>    rcu_read_unlock();
>    return hm;
> 
> (slightly edited to compress a mailing list thread)
> 
> "Nit: Should we do a READ_ONCE(mp->m_healthmon) here to avoid any
> compiler tricks that can result in an undefined behaviour? I am not sure
> if I am being paranoid here.
> 
> "So this is my understanding: RCU guarantees that we get a valid object
> (actual data of m_healthmon) but does not guarantee the compiler will
> not reread the pointer between checking if hm is !NULL and accessing the
> pointer as we are doing it lockless.
> 
> "So just a barrier() call in rcu_read_lock is enough to make sure this
> doesn't happen and probably adding a READ_ONCE() is not needed?"
> 
> After some initial confusion I concluded that he's correct.  The
> compiler could very well eliminate the hm variable in favor of walking
> the pointers twice, turning the code into:
> 
>    if (mp->m_healthmon && !refcount_inc_not_zero(&mp->m_healthmon->ref))
> 
> If this happens, then xfs_healthmon_detach can sneak in between the
> two sides of the && expression and set mp->m_healthmon to NULL, and
> thereby cause a null pointer dereference crash.  Fix this by using the
> rcu pointer assignment and dereference functions, which ensure that the
> proper reordering barriers are in place.
> 
> Practically speaking, gcc seems to allocate an actual variable for hm
> and only reads mp->m_healthmon once (as intended), but we ought to be
> more explicit about requiring this.
> 
> Reported-by: Pankaj Raghav <pankaj.raghav@linux.dev>
> Fixes: a48373e7d35a89f6f ("xfs: start creating infrastructure for health monitoring")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> ---

Looks good,

Reviewed-by: Pankaj Raghav <p.raghav@samsung.com>

--
Pankaj

[PATCH 5/6] xfs: don't report metadata inodes to fserror

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Internal metadata inodes are not exposed to userspace programs, so it
makes no sense to pass them to the fserror functions (aka fsnotify).
Instead, report metadata file problems as general filesystem corruption.

Fixes: 5eb4cb18e445d0 ("xfs: convey metadata health events to the health monitor")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/xfs_health.c |   16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)


diff --git a/fs/xfs/xfs_health.c b/fs/xfs/xfs_health.c
index 169123772cb39b..6475159eb9302c 100644
--- a/fs/xfs/xfs_health.c
+++ b/fs/xfs/xfs_health.c
@@ -314,6 +314,18 @@ xfs_rgno_mark_sick(
 	xfs_rtgroup_put(rtg);
 }
 
+static inline void xfs_inode_report_fserror(struct xfs_inode *ip)
+{
+	/* Report metadata inodes as general filesystem corruption */
+	if (xfs_is_internal_inode(ip)) {
+		fserror_report_metadata(ip->i_mount->m_super, -EFSCORRUPTED,
+				GFP_NOFS);
+		return;
+	}
+
+	fserror_report_file_metadata(VFS_I(ip), -EFSCORRUPTED, GFP_NOFS);
+}
+
 /* Mark the unhealthy parts of an inode. */
 void
 xfs_inode_mark_sick(
@@ -339,7 +351,7 @@ xfs_inode_mark_sick(
 	inode_state_clear(VFS_I(ip), I_DONTCACHE);
 	spin_unlock(&VFS_I(ip)->i_lock);
 
-	fserror_report_file_metadata(VFS_I(ip), -EFSCORRUPTED, GFP_NOFS);
+	xfs_inode_report_fserror(ip);
 	if (mask)
 		xfs_healthmon_report_inode(ip, XFS_HEALTHMON_SICK, old_mask,
 				mask);
@@ -371,7 +383,7 @@ xfs_inode_mark_corrupt(
 	inode_state_clear(VFS_I(ip), I_DONTCACHE);
 	spin_unlock(&VFS_I(ip)->i_lock);
 
-	fserror_report_file_metadata(VFS_I(ip), -EFSCORRUPTED, GFP_NOFS);
+	xfs_inode_report_fserror(ip);
 	if (mask)
 		xfs_healthmon_report_inode(ip, XFS_HEALTHMON_CORRUPT, old_mask,
 				mask);

Re: [PATCH 5/6] xfs: don't report metadata inodes to fserror

[Christoph Hellwig]

On Wed, Feb 18, 2026 at 10:02:01PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Internal metadata inodes are not exposed to userspace programs, so it
> makes no sense to pass them to the fserror functions (aka fsnotify).
> Instead, report metadata file problems as general filesystem corruption.
> 
> Fixes: 5eb4cb18e445d0 ("xfs: convey metadata health events to the health monitor")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 5/6] xfs: don't report metadata inodes to fserror

[Carlos Maiolino]

On Wed, Feb 18, 2026 at 10:02:01PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Internal metadata inodes are not exposed to userspace programs, so it
> makes no sense to pass them to the fserror functions (aka fsnotify).
> Instead, report metadata file problems as general filesystem corruption.
> 
> Fixes: 5eb4cb18e445d0 ("xfs: convey metadata health events to the health monitor")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> ---
>  fs/xfs/xfs_health.c |   16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_health.c b/fs/xfs/xfs_health.c
> index 169123772cb39b..6475159eb9302c 100644
> --- a/fs/xfs/xfs_health.c
> +++ b/fs/xfs/xfs_health.c
> @@ -314,6 +314,18 @@ xfs_rgno_mark_sick(
>  	xfs_rtgroup_put(rtg);
>  }
>  
> +static inline void xfs_inode_report_fserror(struct xfs_inode *ip)
> +{
> +	/* Report metadata inodes as general filesystem corruption */
> +	if (xfs_is_internal_inode(ip)) {
> +		fserror_report_metadata(ip->i_mount->m_super, -EFSCORRUPTED,
> +				GFP_NOFS);
> +		return;
> +	}
> +
> +	fserror_report_file_metadata(VFS_I(ip), -EFSCORRUPTED, GFP_NOFS);
> +}
> +
>  /* Mark the unhealthy parts of an inode. */
>  void
>  xfs_inode_mark_sick(
> @@ -339,7 +351,7 @@ xfs_inode_mark_sick(
>  	inode_state_clear(VFS_I(ip), I_DONTCACHE);
>  	spin_unlock(&VFS_I(ip)->i_lock);
>  
> -	fserror_report_file_metadata(VFS_I(ip), -EFSCORRUPTED, GFP_NOFS);
> +	xfs_inode_report_fserror(ip);
>  	if (mask)
>  		xfs_healthmon_report_inode(ip, XFS_HEALTHMON_SICK, old_mask,
>  				mask);
> @@ -371,7 +383,7 @@ xfs_inode_mark_corrupt(
>  	inode_state_clear(VFS_I(ip), I_DONTCACHE);
>  	spin_unlock(&VFS_I(ip)->i_lock);
>  
> -	fserror_report_file_metadata(VFS_I(ip), -EFSCORRUPTED, GFP_NOFS);
> +	xfs_inode_report_fserror(ip);
>  	if (mask)
>  		xfs_healthmon_report_inode(ip, XFS_HEALTHMON_CORRUPT, old_mask,
>  				mask);
> 

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

[Darrick J. Wong]

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>
---
 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);

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

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

[Carlos Maiolino]

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?

Otherwise pointing this as a fix to a problem we can't be sure has
actually been fixed, sounds misleading at best.

For the fix itself though:

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>


> ---
>  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);
> 
> 

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

[Darrick J. Wong]

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.

> 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.

> 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);
> > 
> > 
> 

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

[Carlos Maiolino]

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.

> 
> > 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);
> > > 
> > > 
> > 
> 

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

[Darrick J. Wong]

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);
> > > > 
> > > > 
> > > 
> > 
> 

[PATCHSET v2 1/2] xfs: bug fixes for 7.0

[Darrick J. Wong]

Hi all,

Bug fixes for 7.0.

v2: clean a couple of things up, add rvbs

If you're going to start using this code, I strongly recommend pulling
from my git trees, which are linked below.

With a bit of luck, this should all go splendidly.
Comments and questions are, as always, welcome.

--D

kernel git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=xfs-fixes-7.0
---
Commits in this patchset:
 * xfs: fix copy-paste error in previous fix
 * xfs: fix xfs_group release bug in xfs_verify_report_losses
 * xfs: fix xfs_group release bug in xfs_dax_notify_dev_failure
 * xfs: fix potential pointer access race in xfs_healthmon_get
 * xfs: don't report metadata inodes to fserror
 * xfs: don't report half-built inodes to fserror
---
 fs/xfs/xfs_mount.h          |    2 +-
 fs/xfs/scrub/dir_repair.c   |    2 +-
 fs/xfs/xfs_health.c         |   20 ++++++++++++++++++--
 fs/xfs/xfs_healthmon.c      |   11 +++++++----
 fs/xfs/xfs_icache.c         |    9 ++++++++-
 fs/xfs/xfs_notify_failure.c |    4 ++--
 fs/xfs/xfs_verify_media.c   |    4 ++--
 7 files changed, 39 insertions(+), 13 deletions(-)

[PATCH 4/6] xfs: fix potential pointer access race in xfs_healthmon_get

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Pankaj Raghav asks about this code in xfs_healthmon_get:

  hm = mp->m_healthmon;
  if (hm && !refcount_inc_not_zero(&hm->ref))
    hm = NULL;
  rcu_read_unlock();
  return hm;

(slightly edited to compress a mailing list thread)

"Nit: Should we do a READ_ONCE(mp->m_healthmon) here to avoid any
compiler tricks that can result in an undefined behaviour? I am not sure
if I am being paranoid here.

"So this is my understanding: RCU guarantees that we get a valid object
(actual data of m_healthmon) but does not guarantee the compiler will
not reread the pointer between checking if hm is !NULL and accessing the
pointer as we are doing it lockless.

"So just a barrier() call in rcu_read_lock is enough to make sure this
doesn't happen and probably adding a READ_ONCE() is not needed?"

After some initial confusion I concluded that he's correct.  The
compiler could very well eliminate the hm variable in favor of walking
the pointers twice, turning the code into:

  if (mp->m_healthmon && !refcount_inc_not_zero(&mp->m_healthmon->ref))

If this happens, then xfs_healthmon_detach can sneak in between the
two sides of the && expression and set mp->m_healthmon to NULL, and
thereby cause a null pointer dereference crash.  Fix this by using the
rcu pointer assignment and dereference functions, which ensure that the
proper reordering barriers are in place.

Practically speaking, gcc seems to allocate an actual variable for hm
and only reads mp->m_healthmon once (as intended), but we ought to be
more explicit about requiring this.

Reported-by: Pankaj Raghav <pankaj.raghav@linux.dev>
Fixes: a48373e7d35a89f6f ("xfs: start creating infrastructure for health monitoring")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Reviewed-by: Pankaj Raghav <p.raghav@samsung.com>
---
 fs/xfs/xfs_mount.h     |    2 +-
 fs/xfs/xfs_healthmon.c |   11 +++++++----
 2 files changed, 8 insertions(+), 5 deletions(-)


diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index 61c71128d171cb..ddd4028be8d6ba 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -345,7 +345,7 @@ typedef struct xfs_mount {
 	struct xfs_hooks	m_dir_update_hooks;
 
 	/* Private data referring to a health monitor object. */
-	struct xfs_healthmon	*m_healthmon;
+	struct xfs_healthmon __rcu	*m_healthmon;
 } xfs_mount_t;
 
 #define M_IGEO(mp)		(&(mp)->m_ino_geo)
diff --git a/fs/xfs/xfs_healthmon.c b/fs/xfs/xfs_healthmon.c
index ca7352dcd182fb..1494408f66a0df 100644
--- a/fs/xfs/xfs_healthmon.c
+++ b/fs/xfs/xfs_healthmon.c
@@ -69,7 +69,7 @@ xfs_healthmon_get(
 	struct xfs_healthmon		*hm;
 
 	rcu_read_lock();
-	hm = mp->m_healthmon;
+	hm = rcu_dereference(mp->m_healthmon);
 	if (hm && !refcount_inc_not_zero(&hm->ref))
 		hm = NULL;
 	rcu_read_unlock();
@@ -110,13 +110,13 @@ xfs_healthmon_attach(
 	struct xfs_healthmon	*hm)
 {
 	spin_lock(&xfs_healthmon_lock);
-	if (mp->m_healthmon != NULL) {
+	if (rcu_access_pointer(mp->m_healthmon) != NULL) {
 		spin_unlock(&xfs_healthmon_lock);
 		return -EEXIST;
 	}
 
 	refcount_inc(&hm->ref);
-	mp->m_healthmon = hm;
+	rcu_assign_pointer(mp->m_healthmon, hm);
 	hm->mount_cookie = (uintptr_t)mp->m_super;
 	spin_unlock(&xfs_healthmon_lock);
 
@@ -128,13 +128,16 @@ STATIC void
 xfs_healthmon_detach(
 	struct xfs_healthmon	*hm)
 {
+	struct xfs_mount	*mp;
+
 	spin_lock(&xfs_healthmon_lock);
 	if (hm->mount_cookie == DETACHED_MOUNT_COOKIE) {
 		spin_unlock(&xfs_healthmon_lock);
 		return;
 	}
 
-	XFS_M((struct super_block *)hm->mount_cookie)->m_healthmon = NULL;
+	mp = XFS_M((struct super_block *)hm->mount_cookie);
+	rcu_assign_pointer(mp->m_healthmon, NULL);
 	hm->mount_cookie = DETACHED_MOUNT_COOKIE;
 	spin_unlock(&xfs_healthmon_lock);