xfs: fix finding a last resort AG in xfs_filestream_pick_ag
authorChristoph Hellwig <hch@lst.de>
Wed, 23 Oct 2024 13:37:22 +0000 (15:37 +0200)
committerCarlos Maiolino <cem@kernel.org>
Wed, 30 Oct 2024 10:27:18 +0000 (11:27 +0100)
When the main loop in xfs_filestream_pick_ag fails to find a suitable
AG it tries to just pick the online AG.  But the loop for that uses
args->pag as loop iterator while the later code expects pag to be
set.  Fix this by reusing the max_pag case for this last resort, and
also add a check for impossible case of no AG just to make sure that
the uninitialized pag doesn't even escape in theory.

Reported-by: syzbot+4125a3c514e3436a02e6@syzkaller.appspotmail.com
Signed-off-by: Christoph Hellwig <hch@lst.de>
Tested-by: syzbot+4125a3c514e3436a02e6@syzkaller.appspotmail.com
Fixes: f8f1ed1ab3baba ("xfs: return a referenced perag from filestreams allocator")
Cc: <stable@vger.kernel.org> # v6.3
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
fs/xfs/xfs_filestream.c
fs/xfs/xfs_trace.h

index e3aaa055559781b3a77cf7acfb81ca6c54094700..88bd23ce74cde131bd7d9a8a919441ad2ae73b7e 100644 (file)
@@ -64,7 +64,7 @@ xfs_filestream_pick_ag(
        struct xfs_perag        *pag;
        struct xfs_perag        *max_pag = NULL;
        xfs_extlen_t            minlen = *longest;
-       xfs_extlen_t            free = 0, minfree, maxfree = 0;
+       xfs_extlen_t            minfree, maxfree = 0;
        xfs_agnumber_t          agno;
        bool                    first_pass = true;
        int                     err;
@@ -107,7 +107,6 @@ restart:
                             !(flags & XFS_PICK_USERDATA) ||
                             (flags & XFS_PICK_LOWSPACE))) {
                                /* Break out, retaining the reference on the AG. */
-                               free = pag->pagf_freeblks;
                                break;
                        }
                }
@@ -150,23 +149,25 @@ restart:
                 * grab.
                 */
                if (!max_pag) {
-                       for_each_perag_wrap(args->mp, 0, start_agno, args->pag)
+                       for_each_perag_wrap(args->mp, 0, start_agno, pag) {
+                               max_pag = pag;
                                break;
-                       atomic_inc(&args->pag->pagf_fstrms);
-                       *longest = 0;
-               } else {
-                       pag = max_pag;
-                       free = maxfree;
-                       atomic_inc(&pag->pagf_fstrms);
+                       }
+
+                       /* Bail if there are no AGs at all to select from. */
+                       if (!max_pag)
+                               return -ENOSPC;
                }
+
+               pag = max_pag;
+               atomic_inc(&pag->pagf_fstrms);
        } else if (max_pag) {
                xfs_perag_rele(max_pag);
        }
 
-       trace_xfs_filestream_pick(pag, pino, free);
+       trace_xfs_filestream_pick(pag, pino);
        args->pag = pag;
        return 0;
-
 }
 
 static struct xfs_inode *
index ee9f0b1f548dc1ef6de47db50e7740f67b20744a..fcb2bad4f76e4b5b71649b3dfcd484e8ca6c618f 100644 (file)
@@ -691,8 +691,8 @@ DEFINE_FILESTREAM_EVENT(xfs_filestream_lookup);
 DEFINE_FILESTREAM_EVENT(xfs_filestream_scan);
 
 TRACE_EVENT(xfs_filestream_pick,
-       TP_PROTO(struct xfs_perag *pag, xfs_ino_t ino, xfs_extlen_t free),
-       TP_ARGS(pag, ino, free),
+       TP_PROTO(struct xfs_perag *pag, xfs_ino_t ino),
+       TP_ARGS(pag, ino),
        TP_STRUCT__entry(
                __field(dev_t, dev)
                __field(xfs_ino_t, ino)
@@ -703,14 +703,9 @@ TRACE_EVENT(xfs_filestream_pick,
        TP_fast_assign(
                __entry->dev = pag->pag_mount->m_super->s_dev;
                __entry->ino = ino;
-               if (pag) {
-                       __entry->agno = pag->pag_agno;
-                       __entry->streams = atomic_read(&pag->pagf_fstrms);
-               } else {
-                       __entry->agno = NULLAGNUMBER;
-                       __entry->streams = 0;
-               }
-               __entry->free = free;
+               __entry->agno = pag->pag_agno;
+               __entry->streams = atomic_read(&pag->pagf_fstrms);
+               __entry->free = pag->pagf_freeblks;
        ),
        TP_printk("dev %d:%d ino 0x%llx agno 0x%x streams %d free %d",
                  MAJOR(__entry->dev), MINOR(__entry->dev),