tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

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>

authored by

Yuto Ohnuki and committed by
Carlos Maiolino 79ef34ec 4f24a767

+51 -11
+32 -4
fs/xfs/xfs_trace.h
··· 56 56 #include <linux/tracepoint.h> 57 57 58 58 struct xfs_agf; 59 + struct xfs_ail; 59 60 struct xfs_alloc_arg; 60 61 struct xfs_attr_list_context; 61 62 struct xfs_buf_log_item; ··· 1651 1650 DEFINE_EVENT(xfs_log_item_class, name, \ 1652 1651 TP_PROTO(struct xfs_log_item *lip), \ 1653 1652 TP_ARGS(lip)) 1654 - DEFINE_LOG_ITEM_EVENT(xfs_ail_push); 1655 - DEFINE_LOG_ITEM_EVENT(xfs_ail_pinned); 1656 - DEFINE_LOG_ITEM_EVENT(xfs_ail_locked); 1657 - DEFINE_LOG_ITEM_EVENT(xfs_ail_flushing); 1658 1653 DEFINE_LOG_ITEM_EVENT(xfs_cil_whiteout_mark); 1659 1654 DEFINE_LOG_ITEM_EVENT(xfs_cil_whiteout_skip); 1660 1655 DEFINE_LOG_ITEM_EVENT(xfs_cil_whiteout_unpin); 1661 1656 DEFINE_LOG_ITEM_EVENT(xlog_ail_insert_abort); 1662 1657 DEFINE_LOG_ITEM_EVENT(xfs_trans_free_abort); 1658 + 1659 + DECLARE_EVENT_CLASS(xfs_ail_push_class, 1660 + TP_PROTO(struct xfs_ail *ailp, uint type, unsigned long flags, xfs_lsn_t lsn), 1661 + TP_ARGS(ailp, type, flags, lsn), 1662 + TP_STRUCT__entry( 1663 + __field(dev_t, dev) 1664 + __field(uint, type) 1665 + __field(unsigned long, flags) 1666 + __field(xfs_lsn_t, lsn) 1667 + ), 1668 + TP_fast_assign( 1669 + __entry->dev = ailp->ail_log->l_mp->m_super->s_dev; 1670 + __entry->type = type; 1671 + __entry->flags = flags; 1672 + __entry->lsn = lsn; 1673 + ), 1674 + TP_printk("dev %d:%d lsn %d/%d type %s flags %s", 1675 + MAJOR(__entry->dev), MINOR(__entry->dev), 1676 + CYCLE_LSN(__entry->lsn), BLOCK_LSN(__entry->lsn), 1677 + __print_symbolic(__entry->type, XFS_LI_TYPE_DESC), 1678 + __print_flags(__entry->flags, "|", XFS_LI_FLAGS)) 1679 + ) 1680 + 1681 + #define DEFINE_AIL_PUSH_EVENT(name) \ 1682 + DEFINE_EVENT(xfs_ail_push_class, name, \ 1683 + TP_PROTO(struct xfs_ail *ailp, uint type, unsigned long flags, xfs_lsn_t lsn), \ 1684 + TP_ARGS(ailp, type, flags, lsn)) 1685 + DEFINE_AIL_PUSH_EVENT(xfs_ail_push); 1686 + DEFINE_AIL_PUSH_EVENT(xfs_ail_pinned); 1687 + DEFINE_AIL_PUSH_EVENT(xfs_ail_locked); 1688 + DEFINE_AIL_PUSH_EVENT(xfs_ail_flushing); 1663 1689 1664 1690 DECLARE_EVENT_CLASS(xfs_ail_class, 1665 1691 TP_PROTO(struct xfs_log_item *lip, xfs_lsn_t old_lsn, xfs_lsn_t new_lsn),
+19 -7
fs/xfs/xfs_trans_ail.c
··· 365 365 return XFS_ITEM_SUCCESS; 366 366 } 367 367 368 + /* 369 + * Push a single log item from the AIL. 370 + * 371 + * @lip may have been released and freed by the time this function returns, 372 + * so callers must not dereference the log item afterwards. 373 + */ 368 374 static inline uint 369 375 xfsaild_push_item( 370 376 struct xfs_ail *ailp, ··· 511 505 512 506 lsn = lip->li_lsn; 513 507 while ((XFS_LSN_CMP(lip->li_lsn, ailp->ail_target) <= 0)) { 514 - int lock_result; 508 + int lock_result; 509 + uint type = lip->li_type; 510 + unsigned long flags = lip->li_flags; 511 + xfs_lsn_t item_lsn = lip->li_lsn; 515 512 516 513 if (test_bit(XFS_LI_FLUSHING, &lip->li_flags)) 517 514 goto next_item; ··· 523 514 * Note that iop_push may unlock and reacquire the AIL lock. We 524 515 * rely on the AIL cursor implementation to be able to deal with 525 516 * the dropped lock. 517 + * 518 + * The log item may have been freed by the push, so it must not 519 + * be accessed or dereferenced below this line. 526 520 */ 527 521 lock_result = xfsaild_push_item(ailp, lip); 528 522 switch (lock_result) { 529 523 case XFS_ITEM_SUCCESS: 530 524 XFS_STATS_INC(mp, xs_push_ail_success); 531 - trace_xfs_ail_push(lip); 525 + trace_xfs_ail_push(ailp, type, flags, item_lsn); 532 526 533 - ailp->ail_last_pushed_lsn = lsn; 527 + ailp->ail_last_pushed_lsn = item_lsn; 534 528 break; 535 529 536 530 case XFS_ITEM_FLUSHING: ··· 549 537 * AIL is being flushed. 550 538 */ 551 539 XFS_STATS_INC(mp, xs_push_ail_flushing); 552 - trace_xfs_ail_flushing(lip); 540 + trace_xfs_ail_flushing(ailp, type, flags, item_lsn); 553 541 554 542 flushing++; 555 - ailp->ail_last_pushed_lsn = lsn; 543 + ailp->ail_last_pushed_lsn = item_lsn; 556 544 break; 557 545 558 546 case XFS_ITEM_PINNED: 559 547 XFS_STATS_INC(mp, xs_push_ail_pinned); 560 - trace_xfs_ail_pinned(lip); 548 + trace_xfs_ail_pinned(ailp, type, flags, item_lsn); 561 549 562 550 stuck++; 563 551 ailp->ail_log_flush++; 564 552 break; 565 553 case XFS_ITEM_LOCKED: 566 554 XFS_STATS_INC(mp, xs_push_ail_locked); 567 - trace_xfs_ail_locked(lip); 555 + trace_xfs_ail_locked(ailp, type, flags, item_lsn); 568 556 569 557 stuck++; 570 558 break;