From: Leah Rumancik <leah.rumancik@gmail.com>
To: stable@vger.kernel.org
Cc: linux-xfs@vger.kernel.org, amir73il@gmail.com,
chandan.babu@oracle.com, fred@cloudflare.com,
Guo Xuenan <guoxuenan@huawei.com>,
"Darrick J . Wong" <djwong@kernel.org>,
Leah Rumancik <leah.rumancik@gmail.com>
Subject: [PATCH 6.1 16/24] xfs: wait iclog complete before tearing down AIL
Date: Wed, 1 May 2024 11:41:04 -0700 [thread overview]
Message-ID: <20240501184112.3799035-16-leah.rumancik@gmail.com> (raw)
In-Reply-To: <20240501184112.3799035-1-leah.rumancik@gmail.com>
From: Guo Xuenan <guoxuenan@huawei.com>
[ Upstream commit 1eb52a6a71981b80f9acbd915acd6a05a5037196 ]
Fix uaf in xfs_trans_ail_delete during xlog force shutdown.
In commit cd6f79d1fb32 ("xfs: run callbacks before waking waiters in
xlog_state_shutdown_callbacks") changed the order of running callbacks
and wait for iclog completion to avoid unmount path untimely destroy AIL.
But which seems not enough to ensue this, adding mdelay in
`xfs_buf_item_unpin` can prove that.
The reproduction is as follows. To ensure destroy AIL safely,
we should wait all xlog ioend workers done and sync the AIL.
==================================================================
BUG: KASAN: use-after-free in xfs_trans_ail_delete+0x240/0x2a0
Read of size 8 at addr ffff888023169400 by task kworker/1:1H/43
CPU: 1 PID: 43 Comm: kworker/1:1H Tainted: G W
6.1.0-rc1-00002-gc28266863c4a #137
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
1.13.0-1ubuntu1.1 04/01/2014
Workqueue: xfs-log/sda xlog_ioend_work
Call Trace:
<TASK>
dump_stack_lvl+0x4d/0x66
print_report+0x171/0x4a6
kasan_report+0xb3/0x130
xfs_trans_ail_delete+0x240/0x2a0
xfs_buf_item_done+0x7b/0xa0
xfs_buf_ioend+0x1e9/0x11f0
xfs_buf_item_unpin+0x4c8/0x860
xfs_trans_committed_bulk+0x4c2/0x7c0
xlog_cil_committed+0xab6/0xfb0
xlog_cil_process_committed+0x117/0x1e0
xlog_state_shutdown_callbacks+0x208/0x440
xlog_force_shutdown+0x1b3/0x3a0
xlog_ioend_work+0xef/0x1d0
process_one_work+0x6f9/0xf70
worker_thread+0x578/0xf30
kthread+0x28c/0x330
ret_from_fork+0x1f/0x30
</TASK>
Allocated by task 9606:
kasan_save_stack+0x1e/0x40
kasan_set_track+0x21/0x30
__kasan_kmalloc+0x7a/0x90
__kmalloc+0x59/0x140
kmem_alloc+0xb2/0x2f0
xfs_trans_ail_init+0x20/0x320
xfs_log_mount+0x37e/0x690
xfs_mountfs+0xe36/0x1b40
xfs_fs_fill_super+0xc5c/0x1a70
get_tree_bdev+0x3c5/0x6c0
vfs_get_tree+0x85/0x250
path_mount+0xec3/0x1830
do_mount+0xef/0x110
__x64_sys_mount+0x150/0x1f0
do_syscall_64+0x35/0x80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
Freed by task 9662:
kasan_save_stack+0x1e/0x40
kasan_set_track+0x21/0x30
kasan_save_free_info+0x2a/0x40
__kasan_slab_free+0x105/0x1a0
__kmem_cache_free+0x99/0x2d0
kvfree+0x3a/0x40
xfs_log_unmount+0x60/0xf0
xfs_unmountfs+0xf3/0x1d0
xfs_fs_put_super+0x78/0x300
generic_shutdown_super+0x151/0x400
kill_block_super+0x9a/0xe0
deactivate_locked_super+0x82/0xe0
deactivate_super+0x91/0xb0
cleanup_mnt+0x32a/0x4a0
task_work_run+0x15f/0x240
exit_to_user_mode_prepare+0x188/0x190
syscall_exit_to_user_mode+0x12/0x30
do_syscall_64+0x42/0x80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
The buggy address belongs to the object at ffff888023169400
which belongs to the cache kmalloc-128 of size 128
The buggy address is located 0 bytes inside of
128-byte region [ffff888023169400, ffff888023169480)
The buggy address belongs to the physical page:
page:ffffea00008c5a00 refcount:1 mapcount:0 mapping:0000000000000000
index:0xffff888023168f80 pfn:0x23168
head:ffffea00008c5a00 order:1 compound_mapcount:0 compound_pincount:0
flags: 0x1fffff80010200(slab|head|node=0|zone=1|lastcpupid=0x1fffff)
raw: 001fffff80010200 ffffea00006b3988 ffffea0000577a88 ffff88800f842ac0
raw: ffff888023168f80 0000000000150007 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff888023169300: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff888023169380: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>ffff888023169400: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff888023169480: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff888023169500: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
==================================================================
Disabling lock debugging due to kernel taint
Fixes: cd6f79d1fb32 ("xfs: run callbacks before waking waiters in xlog_state_shutdown_callbacks")
Signed-off-by: Guo Xuenan <guoxuenan@huawei.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Leah Rumancik <leah.rumancik@gmail.com>
Acked-by: Darrick J. Wong <djwong@kernel.org>
---
fs/xfs/xfs_log.c | 36 +++++++++++++++++++++++++-----------
1 file changed, 25 insertions(+), 11 deletions(-)
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index f02a0dd522b3..60b19f6d7077 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -886,6 +886,23 @@ xlog_force_iclog(
return xlog_state_release_iclog(iclog->ic_log, iclog, NULL);
}
+/*
+ * Cycle all the iclogbuf locks to make sure all log IO completion
+ * is done before we tear down these buffers.
+ */
+static void
+xlog_wait_iclog_completion(struct xlog *log)
+{
+ int i;
+ struct xlog_in_core *iclog = log->l_iclog;
+
+ for (i = 0; i < log->l_iclog_bufs; i++) {
+ down(&iclog->ic_sema);
+ up(&iclog->ic_sema);
+ iclog = iclog->ic_next;
+ }
+}
+
/*
* Wait for the iclog and all prior iclogs to be written disk as required by the
* log force state machine. Waiting on ic_force_wait ensures iclog completions
@@ -1111,6 +1128,14 @@ xfs_log_unmount(
{
xfs_log_clean(mp);
+ /*
+ * If shutdown has come from iclog IO context, the log
+ * cleaning will have been skipped and so we need to wait
+ * for the iclog to complete shutdown processing before we
+ * tear anything down.
+ */
+ xlog_wait_iclog_completion(mp->m_log);
+
xfs_buftarg_drain(mp->m_ddev_targp);
xfs_trans_ail_destroy(mp);
@@ -2113,17 +2138,6 @@ xlog_dealloc_log(
xlog_in_core_t *iclog, *next_iclog;
int i;
- /*
- * Cycle all the iclogbuf locks to make sure all log IO completion
- * is done before we tear down these buffers.
- */
- iclog = log->l_iclog;
- for (i = 0; i < log->l_iclog_bufs; i++) {
- down(&iclog->ic_sema);
- up(&iclog->ic_sema);
- iclog = iclog->ic_next;
- }
-
/*
* Destroy the CIL after waiting for iclog IO completion because an
* iclog EIO error will try to shut down the log, which accesses the
--
2.45.0.rc1.225.g2a3ae87e7f-goog
next prev parent reply other threads:[~2024-05-01 18:41 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-01 18:40 [PATCH 6.1 01/24] xfs: write page faults in iomap are not buffered writes Leah Rumancik
2024-05-01 18:40 ` [PATCH 6.1 02/24] xfs: punching delalloc extents on write failure is racy Leah Rumancik
2024-05-01 18:40 ` [PATCH 6.1 03/24] xfs: use byte ranges for write cleanup ranges Leah Rumancik
2024-05-01 18:40 ` [PATCH 6.1 04/24] xfs,iomap: move delalloc punching to iomap Leah Rumancik
2024-05-01 18:40 ` [PATCH 6.1 05/24] iomap: buffered write failure should not truncate the page cache Leah Rumancik
2024-05-01 18:40 ` [PATCH 6.1 06/24] xfs: xfs_bmap_punch_delalloc_range() should take a byte range Leah Rumancik
2024-05-01 18:40 ` [PATCH 6.1 07/24] iomap: write iomap validity checks Leah Rumancik
2024-05-01 18:40 ` [PATCH 6.1 08/24] xfs: use iomap_valid method to detect stale cached iomaps Leah Rumancik
2024-05-01 18:40 ` [PATCH 6.1 09/24] xfs: drop write error injection is unfixable, remove it Leah Rumancik
2024-05-01 18:40 ` [PATCH 6.1 10/24] xfs: fix off-by-one-block in xfs_discard_folio() Leah Rumancik
2024-05-01 18:40 ` [PATCH 6.1 11/24] xfs: fix incorrect error-out in xfs_remove Leah Rumancik
2024-05-01 18:41 ` [PATCH 6.1 12/24] xfs: fix sb write verify for lazysbcount Leah Rumancik
2024-05-01 18:41 ` [PATCH 6.1 13/24] xfs: fix incorrect i_nlink caused by inode racing Leah Rumancik
2024-05-01 18:41 ` [PATCH 6.1 14/24] xfs: invalidate block device page cache during unmount Leah Rumancik
2024-05-01 18:41 ` [PATCH 6.1 15/24] xfs: attach dquots to inode before reading data/cow fork mappings Leah Rumancik
2024-05-01 18:41 ` Leah Rumancik [this message]
2024-05-01 18:41 ` [PATCH 6.1 17/24] xfs: fix super block buf log item UAF during force shutdown Leah Rumancik
2024-05-01 18:41 ` [PATCH 6.1 18/24] xfs: hoist refcount record merge predicates Leah Rumancik
2024-05-01 18:41 ` [PATCH 6.1 19/24] xfs: estimate post-merge refcounts correctly Leah Rumancik
2024-05-01 18:41 ` [PATCH 6.1 20/24] xfs: invalidate xfs_bufs when allocating cow extents Leah Rumancik
2024-05-01 18:41 ` [PATCH 6.1 21/24] xfs: allow inode inactivation during a ro mount log recovery Leah Rumancik
2024-05-01 18:41 ` [PATCH 6.1 22/24] xfs: fix log recovery when unknown rocompat bits are set Leah Rumancik
2024-05-01 18:41 ` [PATCH 6.1 23/24] xfs: get root inode correctly at bulkstat Leah Rumancik
2024-05-01 18:41 ` [PATCH 6.1 24/24] xfs: short circuit xfs_growfs_data_private() if delta is zero Leah Rumancik
2024-05-04 9:16 ` [PATCH 6.1 01/24] xfs: write page faults in iomap are not buffered writes Greg KH
2024-05-04 18:17 ` Amir Goldstein
2024-05-06 17:52 ` Leah Rumancik
2024-05-22 14:11 ` Greg KH
2024-05-22 21:55 ` Leah Rumancik
2024-05-23 7:08 ` Greg KH
2024-05-23 11:06 ` Greg KH
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240501184112.3799035-16-leah.rumancik@gmail.com \
--to=leah.rumancik@gmail.com \
--cc=amir73il@gmail.com \
--cc=chandan.babu@oracle.com \
--cc=djwong@kernel.org \
--cc=fred@cloudflare.com \
--cc=guoxuenan@huawei.com \
--cc=linux-xfs@vger.kernel.org \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).