Re: [PATCH v4 2/9] dax: use READ_ONCE() in dax_holder()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] dax: UAF and NULL deref in xfs_dax_notify_failure due to synchronous holder free
--

commit 27365c0f46f3cc2abb67ac0ebca3b78ca639e39f
Author: Miklos Szeredi <mszeredi@redhat.com>

dax: use READ_ONCE() in dax_holder()

This commit updates dax_holder() to use READ_ONCE() when returning the
holder_data. This is intended to allow dax_holder() to be utilized in RCU
context and ensure the holder cannot be freed by a racing fs_put_dax.

> diff --git a/drivers/dax/super.c b/drivers/dax/super.c
> index 25836f389bded..76e8d36546290 100644
> --- a/drivers/dax/super.c
> +++ b/drivers/dax/super.c
> @@ -642,7 +642,7 @@ EXPORT_SYMBOL_GPL(put_dax);
>   */
>  void *dax_holder(struct dax_device *dax_dev)
>  {
> -	return dax_dev->holder_data;
> +	return READ_ONCE(dax_dev->holder_data);

[Severity: High]
Does this change and the reliance on RCU context expose a NULL pointer
dereference and Use-After-Free risk for existing users like XFS?

When a hardware memory failure triggers dax_holder_notify_failure(), it
takes the SRCU read lock and calls xfs_dax_notify_failure(). This reads the
holder pointer and unconditionally dereferences it without checking for NULL:

fs/xfs/xfs_notify_failure.c:xfs_dax_notify_failure() {
	struct xfs_mount *mp = dax_holder(dax_dev);

	if (!(mp->m_super->s_flags & SB_BORN)) {
		...
}

Concurrently, if the filesystem is unmounted, xfs_free_buftarg() calls
fs_put_dax(), which clears the holder_data without waiting for active SRCU
or RCU readers to finish:

drivers/dax/super.c:fs_put_dax() {
	...
	WRITE_ONCE(dax_dev->holder_ops, NULL);
	prev = cmpxchg(&dax_dev->holder_data, holder, NULL);
	...
}

Immediately after, xfs_mount_free() synchronously frees the xfs_mount object
without waiting for an RCU or SRCU grace period:

fs/xfs/xfs_super.c:xfs_mount_free() {
	...
	debugfs_remove(mp->m_debugfs);
	kfree(mp->m_rtname);
	kfree(mp->m_logname);
#ifdef DEBUG
	kfree(mp->m_errortag);
#endif
	kfree(mp);
}

If xfs_dax_notify_failure() calls dax_holder() after it is cleared, mp is
NULL, causing an immediate crash when accessing mp->m_super. If it reads the
pointer before clearing, but kfree(mp) executes concurrently, it causes a
Use-After-Free.

An asynchronous memory failure during unmount could reliably trigger this.

The commit message mentions utilizing dax_holder() in RCU context to make
sure that the holder cannot be freed by a racing fs_put_dax. Does this
assume that filesystems defer freeing the holder? For XFS, the holder
appears to be synchronously freed without an RCU grace period.

>  }
>  EXPORT_SYMBOL_GPL(dax_holder);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008112004.1899560-1-mszeredi@redhat.com?part=2