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

[Pankaj Raghav <pankaj.raghav@linux.dev>]

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