xfs: don't report half-built inodes to fserror

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>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Signed-off-by: Carlos Maiolino <cem@kernel.org>

diff --git a/fs/xfs/xfs_health.c b/fs/xfs/xfs_health.c
index 6475159eb9302c09610cac970a6f6c8219cdccd2..239b843e83d42aa2e6712a19957fc975bd5d3704 100644 (file)
--- 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 f2d4294efd371522b0bc853fb9368489f09cada1..a7a09e7eec815f1771a58caf5aa26537b8b4295f 100644 (file)
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -639,6 +639,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;
@@ -716,7 +724,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);