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 <[email protected]> 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/[email protected]?part=2
