xfs: avoid dereferencing log items after push callbacks

After xfsaild_push_item() calls iop_push(), the log item may have been
freed if the AIL lock was dropped during the push. Background inode
reclaim or the dquot shrinker can free the log item while the AIL lock
is not held, and the tracepoints in the switch statement dereference
the log item after iop_push() returns.

Fix this by capturing the log item type, flags, and LSN before calling
xfsaild_push_item(), and introducing a new xfs_ail_push_class trace
event class that takes these pre-captured values and the ailp pointer
instead of the log item pointer.

Reported-by: syzbot+652af2b3c5569c4ab63c@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=652af2b3c5569c4ab63c
Fixes: 90c60e164012 ("xfs: xfs_iflush() is no longer necessary")
Cc: stable@vger.kernel.org # v5.9
Signed-off-by: Yuto Ohnuki <ytohnuki@amazon.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Carlos Maiolino <cem@kernel.org>

diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 813e5a9f57eb7ae8c64cfb17d85f281a167cff75..0e994b3f768f91ace036543cf9c30bb78ae9c7ad 100644 (file)
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -56,6 +56,7 @@
 #include <linux/tracepoint.h>
 
 struct xfs_agf;
+struct xfs_ail;
 struct xfs_alloc_arg;
 struct xfs_attr_list_context;
 struct xfs_buf_log_item;
@@ -1650,16 +1651,43 @@ TRACE_EVENT(xfs_log_force,
 DEFINE_EVENT(xfs_log_item_class, name, \
        TP_PROTO(struct xfs_log_item *lip), \
        TP_ARGS(lip))
-DEFINE_LOG_ITEM_EVENT(xfs_ail_push);
-DEFINE_LOG_ITEM_EVENT(xfs_ail_pinned);
-DEFINE_LOG_ITEM_EVENT(xfs_ail_locked);
-DEFINE_LOG_ITEM_EVENT(xfs_ail_flushing);
 DEFINE_LOG_ITEM_EVENT(xfs_cil_whiteout_mark);
 DEFINE_LOG_ITEM_EVENT(xfs_cil_whiteout_skip);
 DEFINE_LOG_ITEM_EVENT(xfs_cil_whiteout_unpin);
 DEFINE_LOG_ITEM_EVENT(xlog_ail_insert_abort);
 DEFINE_LOG_ITEM_EVENT(xfs_trans_free_abort);
 
+DECLARE_EVENT_CLASS(xfs_ail_push_class,
+       TP_PROTO(struct xfs_ail *ailp, uint type, unsigned long flags, xfs_lsn_t lsn),
+       TP_ARGS(ailp, type, flags, lsn),
+       TP_STRUCT__entry(
+               __field(dev_t, dev)
+               __field(uint, type)
+               __field(unsigned long, flags)
+               __field(xfs_lsn_t, lsn)
+       ),
+       TP_fast_assign(
+               __entry->dev = ailp->ail_log->l_mp->m_super->s_dev;
+               __entry->type = type;
+               __entry->flags = flags;
+               __entry->lsn = lsn;
+       ),
+       TP_printk("dev %d:%d lsn %d/%d type %s flags %s",
+                 MAJOR(__entry->dev), MINOR(__entry->dev),
+                 CYCLE_LSN(__entry->lsn), BLOCK_LSN(__entry->lsn),
+                 __print_symbolic(__entry->type, XFS_LI_TYPE_DESC),
+                 __print_flags(__entry->flags, "|", XFS_LI_FLAGS))
+)
+
+#define DEFINE_AIL_PUSH_EVENT(name) \
+DEFINE_EVENT(xfs_ail_push_class, name, \
+       TP_PROTO(struct xfs_ail *ailp, uint type, unsigned long flags, xfs_lsn_t lsn), \
+       TP_ARGS(ailp, type, flags, lsn))
+DEFINE_AIL_PUSH_EVENT(xfs_ail_push);
+DEFINE_AIL_PUSH_EVENT(xfs_ail_pinned);
+DEFINE_AIL_PUSH_EVENT(xfs_ail_locked);
+DEFINE_AIL_PUSH_EVENT(xfs_ail_flushing);
+
 DECLARE_EVENT_CLASS(xfs_ail_class,
        TP_PROTO(struct xfs_log_item *lip, xfs_lsn_t old_lsn, xfs_lsn_t new_lsn),
        TP_ARGS(lip, old_lsn, new_lsn),

diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
index 923729af42067c9dc2c726321e68264bd1fd16d9..63266d31b51430dff7dd2f2b01784f14dd5bfb35 100644 (file)
--- a/fs/xfs/xfs_trans_ail.c
+++ b/fs/xfs/xfs_trans_ail.c
@@ -365,6 +365,12 @@ xfsaild_resubmit_item(
        return XFS_ITEM_SUCCESS;
 }
 
+/*
+ * Push a single log item from the AIL.
+ *
+ * @lip may have been released and freed by the time this function returns,
+ * so callers must not dereference the log item afterwards.
+ */
 static inline uint
 xfsaild_push_item(
        struct xfs_ail          *ailp,
@@ -505,7 +511,10 @@ xfsaild_push(
 
        lsn = lip->li_lsn;
        while ((XFS_LSN_CMP(lip->li_lsn, ailp->ail_target) <= 0)) {
-               int     lock_result;
+               int             lock_result;
+               uint            type = lip->li_type;
+               unsigned long   flags = lip->li_flags;
+               xfs_lsn_t       item_lsn = lip->li_lsn;
 
                if (test_bit(XFS_LI_FLUSHING, &lip->li_flags))
                        goto next_item;
@@ -514,14 +523,17 @@ xfsaild_push(
                 * Note that iop_push may unlock and reacquire the AIL lock.  We
                 * rely on the AIL cursor implementation to be able to deal with
                 * the dropped lock.
+                *
+                * The log item may have been freed by the push, so it must not
+                * be accessed or dereferenced below this line.
                 */
                lock_result = xfsaild_push_item(ailp, lip);
                switch (lock_result) {
                case XFS_ITEM_SUCCESS:
                        XFS_STATS_INC(mp, xs_push_ail_success);
-                       trace_xfs_ail_push(lip);
+                       trace_xfs_ail_push(ailp, type, flags, item_lsn);
 
-                       ailp->ail_last_pushed_lsn = lsn;
+                       ailp->ail_last_pushed_lsn = item_lsn;
                        break;
 
                case XFS_ITEM_FLUSHING:
@@ -537,22 +549,22 @@ xfsaild_push(
                         * AIL is being flushed.
                         */
                        XFS_STATS_INC(mp, xs_push_ail_flushing);
-                       trace_xfs_ail_flushing(lip);
+                       trace_xfs_ail_flushing(ailp, type, flags, item_lsn);
 
                        flushing++;
-                       ailp->ail_last_pushed_lsn = lsn;
+                       ailp->ail_last_pushed_lsn = item_lsn;
                        break;
 
                case XFS_ITEM_PINNED:
                        XFS_STATS_INC(mp, xs_push_ail_pinned);
-                       trace_xfs_ail_pinned(lip);
+                       trace_xfs_ail_pinned(ailp, type, flags, item_lsn);
 
                        stuck++;
                        ailp->ail_log_flush++;
                        break;
                case XFS_ITEM_LOCKED:
                        XFS_STATS_INC(mp, xs_push_ail_locked);
-                       trace_xfs_ail_locked(lip);
+                       trace_xfs_ail_locked(ailp, type, flags, item_lsn);
 
                        stuck++;
                        break;