ext4: fix use-after-free in ext4 delayed I/O completion
Subsystems (3)
PATCH 1/1 ext4: fix use-after-free in ext4 delayed I/O completion
ext4_add_complete_io() queues the inode's i_rsv_conversion_work without holding an extra reference to the inode. If the inode is unlinked and evicted before the delayed work runs, the inode slab object gets freed. When ext4_do_flush_completed_IO() later accesses io_end->inode, it is accessing a freed/reallocated object -- a use-after-free detected by KASAN.
Take igrab(inode) when first queueing the work (only when list is empty to avoid duplicate references), and drop the reference in ext4_do_flush_completed_IO() after processing all io_ends.
A per-inode bit (i_rsv_need_iput) tracks whether the extra reference was taken.
This is a defense-in-depth fix complementary to the upstream fix c678bdc99875 ("ext4: fix inode use after free in ext4_end_io_rsv_work()") which adds consistency checks in ext4_io_end_defer_completion(). Both fixes address the same root race but from different angles.
--- fs/ext4/ext4.h | 2 ++ fs/ext4/page-io.c | 15 +++++++++++++-- fs/ext4/super.c | 1 + 3 files changed, 16 insertions(+), 2 deletions(-)
fs/ext4/ext4.h +2 -0
@@ -1179,6 +1179,8 @@ struct ext4_inode_info {/* Lock protecting lists below */ spinlock_t i_completed_io_lock; +/* Track if ext4_add_complete_io() took an extra inode reference. */ + unsigned short i_rsv_need_iput:1;/* * Completed IOs that need unwritten extents handling and have * transaction reserved
fs/ext4/page-io.c +13 -2
@@ -262,8 +262,12 @@ static void ext4_add_complete_io(ext4_io_end_t *io_end)spin_lock_irqsave (&ei->i_completed_io_lock, flags); wq = sbi->rsv_conversion_wq; -if (list_empty (&ei->i_rsv_conversion_list)) -queue_work (wq, &ei->i_rsv_conversion_work); +if (list_empty (&ei->i_rsv_conversion_list)) { +if (igrab (io_end->inode)) { + ei->i_rsv_need_iput =1 ; +queue_work (wq, &ei->i_rsv_conversion_work); + } + }list_add_tail (&io_end->list, &ei->i_rsv_conversion_list);spin_unlock_irqrestore (&ei->i_completed_io_lock, flags); }
@@ -291,6 +295,13 @@ static int ext4_do_flush_completed_IO(struct inode *inode,if (unlikely (!ret && err)) ret = err; } + +/* Release inode reference from ext4_add_complete_io. */ +if (ei->i_rsv_need_iput) { + ei->i_rsv_need_iput =0 ; +iput (&ei->vfs_inode); + } +return ret; }
fs/ext4/super.c +1 -0
@@ -1418,6 +1418,7 @@ static struct inode *ext4_alloc_inode(struct super_block *sb) ei->jinode =NULL ;INIT_LIST_HEAD (&ei->i_rsv_conversion_list);spin_lock_init (&ei->i_completed_io_lock); + ei->i_rsv_need_iput =0 ; ei->i_sync_tid =0 ; ei->i_datasync_tid =0 ;INIT_WORK (&ei->i_rsv_conversion_work, ext4_end_io_rsv_work);
Signature · 1 line
2.34.1
Quoted 12 lines · Longxing Li
ext4_add_complete_io()queues the inode'si_rsv_conversion_workwithout holding an extra reference to the inode. If the inode is unlinked and evicted before the delayed work runs, the inode slab object gets freed. Whenext4_do_flush_completed_IO()later accessesio_end->inode, it is accessing a freed/reallocated object -- a use-after-free detected by KASAN.Take igrab(inode) when first queueing the work (only when list is empty to avoid duplicate references), and drop the reference in
ext4_do_flush_completed_IO()after processing allio_ends.
A per-inode bit (i_rsv_need_iput) tracks whether the extra reference was taken.
Quoted 4 lines
This is a defense-in-depth fix complementary to the upstream fix
c678bdc99875("ext4: fix inode use after free inext4_end_io_rsv_work()") which adds consistency checks inext4_io_end_defer_completion(). Both fixes address the same root race but from different angles.
Um. You've cc'd everybody except the maintainer. And this is already fixed, but you want to fix it even more? I don't get it.
(also if this patch isn't AI slop, it's doing a Really Good Job of pretending to be AI slop. Why would you choose 'unsigned short' as the type?)
Quoted 70 lines
Fixes ce51afb8cc5e ("ext4: abort journal on data writeback failure if in data_err=abort mode")Reported-by Longxing Li <[email protected]>Signed-off-by Longxing Li <[email protected]>--- fs/ext4/ext4.h | 2 ++ fs/ext4/page-io.c | 15 +++++++++++++-- fs/ext4/super.c | 1 + 3 files changed, 16 insertions(+), 2 deletions(-)fs/ext4/ext4.h +2
-0@@ -1179,6 +1179,8 @@ struct ext4_inode_info {/* Lock protecting lists below */ spinlock_t i_completed_io_lock; +/* Track if ext4_add_complete_io() took an extra inode reference. */ + unsigned short i_rsv_need_iput:1;/* * Completed IOs that need unwritten extents handling and have * transaction reserved fs/ext4/page-io.c +13
-2@@ -262,8 +262,12 @@ static void ext4_add_complete_io(ext4_io_end_t *io_end)spin_lock_irqsave (&ei->i_completed_io_lock, flags); wq = sbi->rsv_conversion_wq; -if (list_empty (&ei->i_rsv_conversion_list)) -queue_work (wq, &ei->i_rsv_conversion_work); +if (list_empty (&ei->i_rsv_conversion_list)) { +if (igrab (io_end->inode)) { + ei->i_rsv_need_iput =1 ; +queue_work (wq, &ei->i_rsv_conversion_work); + } + }list_add_tail (&io_end->list, &ei->i_rsv_conversion_list);spin_unlock_irqrestore (&ei->i_completed_io_lock, flags); }@@ -291,6 +295,13 @@ static int ext4_do_flush_completed_IO(struct inode *inode,if (unlikely (!ret && err)) ret = err; } + +/* Release inode reference from ext4_add_complete_io. */ +if (ei->i_rsv_need_iput) { + ei->i_rsv_need_iput =0 ; +iput (&ei->vfs_inode); + } +return ret; }fs/ext4/super.c +1
-0@@ -1418,6 +1418,7 @@ static struct inode *ext4_alloc_inode(struct super_block *sb) ei->jinode =NULL ;INIT_LIST_HEAD (&ei->i_rsv_conversion_list);spin_lock_init (&ei->i_completed_io_lock); + ei->i_rsv_need_iput =0 ; ei->i_sync_tid =0 ; ei->i_datasync_tid =0 ;INIT_WORK (&ei->i_rsv_conversion_work, ext4_end_io_rsv_work);Signature · 1 line
2.34.1
Theodore Tso
maintainerQuoted 4 lines · Matthew Wilcox
(also if this patch isn't AI slop, it's doing a Really Good Job of pretending to be AI slop. Why would you choose 'unsigned short' as the type?)
It's AI slop that didn't even bother to run it by an AI patch quality review process such as Sashiko:
- [High] Skipping queue_work() when igrab() fails causes a permanent deadlock during inode eviction by stranding pages in writeback state.
- [High] A data race and logical flaw with the 1-bit i_rsv_need_iput flag causes permanent inode reference leaks during concurrent I/O completions.
Syzbot also reported the inconsistent igrab state issue. So it's AI slop that wasted resources both with Shashiko and Syzbot....
Longxing Li, whether or not this is AI slop, this patch will tend to cause me to distrust any future code submissions form you. I suggest that you test patches and look at the code much more carefully before sending, if you don't want to have a bad reputation as a patch submitter.
Best regards,
- Ted
syzbot ci has tested the following series
syzbot ci has tested the following series
[v1] ext4: fix use-after-free in ext4 delayed I/O completion
https://lore.kernel.org/all/[email protected]
* [PATCH] ext4: fix use-after-free in ext4 delayed I/O completion
and found the following issue:
inconsistent lock state in igrab
Full report is available here:
https://ci.syzbot.org/series/91c98fac-dc4f-4090-8c56-abbc36b7cd36
***
inconsistent lock state in igrab
tree: mm-new URL: https://kernel.googlesource.com/pub/scm/linux/kernel/git/akpm/mm.git base: 86f436567f2516a0083b210bedc933544826a2c3 arch: amd64 compiler: Debian clang version 22.1.6 (++20260514074242+fc4aad7b5db3-1~exp1~20260514074407.73), Debian LLD 22.1.6 config: https://ci.syzbot.org/builds/7d4608a1-deef-4f49-aff7-ee231c101f0a/config
================================
WARNING: inconsistent lock state
syzkaller #0 Not tainted
--------------------------------
inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
swapper/0/0 [HC0[0]:SC1[1]:HE0:SE0] takes:ffff888112ebb350 (&sb->s_type->i_lock_key#23){+.?.}-{3:3}, at: igrab+0x2d/0x1e0 {SOFTIRQ-ON-W} state was registered at:
lock_acquire+0x116/0x340 _raw_spin_lock+0x2e/0x40 iget_locked+0x397/0x6a0 __ext4_iget+0x242/0x4290 ext4_get_journal_inode+0x30/0x1c0 ext4_load_and_init_journal+0x231/0x23f0 ext4_fill_super+0x3a64/0x6560 get_tree_bdev_flags+0x430/0x4f0 vfs_get_tree+0x92/0x2a0 do_new_mount+0x325/0xac0 init_mount+0xc3/0x110 do_mount_root+0xf4/0x260 mount_root_generic+0x1a7/0x370 prepare_namespace+0x71/0xa0 kernel_init_freeable+0x36e/0x4b0 kernel_init+0x1d/0x1d0 ret_from_fork+0x5b0/0xb20 ret_from_fork_asm+0x1a/0x30 irq event stamp: 682389 hardirqs last enabled at (682388): [<ffffffff81862340>] handle_softirqs+0x1a0/0x850 hardirqs last disabled at (682389): [<ffffffff8b73ff0e>] _raw_spin_lock_irqsave+0x8e/0x100 softirqs last enabled at (682374): [<ffffffff81862c35>] __irq_exit_rcu+0xd5/0x1f0 softirqs last disabled at (682387): [<ffffffff81862c35>] __irq_exit_rcu+0xd5/0x1f0
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(&sb->s_type->i_lock_key#23);
<Interrupt>
lock(&sb->s_type->i_lock_key#23);*** DEADLOCK ***
1 lock held by swapper/0/0:
#0: ffff888112ebb880 (&ei->i_completed_io_lock){..-.}-{3:3}, at: ext4_put_io_end_defer+0x442/0x700
stack backtrace: CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Not tainted syzkaller #0 PREEMPT(full) Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014 Call Trace: <IRQ> dump_stack_lvl+0x1a9/0x280 print_usage_bug+0x28b/0x2e0 mark_lock_irq+0x410/0x420 mark_lock+0x115/0x190 __lock_acquire+0x65c/0x2cd0 lock_acquire+0x116/0x340 _raw_spin_lock+0x2e/0x40 igrab+0x2d/0x1e0 ext4_put_io_end_defer+0x4d0/0x700 blk_update_request+0x57b/0xe30 scsi_end_request+0x77/0x820 scsi_io_completion+0x133/0x360 blk_done_softirq+0x10a/0x160 handle_softirqs+0x275/0x850 __irq_exit_rcu+0xd5/0x1f0 irq_exit_rcu+0x9/0x30 common_interrupt+0xbb/0xe0 </IRQ> <TASK> asm_common_interrupt+0x26/0x40 RIP: 0010:pv_native_safe_halt+0xf/0x20 Code: 57 5e 02 c3 cc cc cc cc cc cc cc 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 f3 0f 1e fa eb 07 0f 00 2d d3 07 18 00 fb f4 <e9> cc f3 02 00 cc cc cc cc cc cc cc cc cc cc cc cc 90 90 90 90 90 RSP: 0018:ffffffff8e007d80 EFLAGS: 00000286 RAX: ec158cdc8cc84600 RBX: ffffffff8198983a RCX: ec158cdc8cc84600 RDX: 0000000000000001 RSI: ffffffff8da4eee4 RDI: ffffffff8be557c0 RBP: ffffffff8e007eb0 R08: ffff8881210336db R09: 1ffff110242066db R10: dffffc0000000000 R11: ffffed10242066dc R12: 1ffffffff1c12a78 R13: dffffc0000000000 R14: 0000000000000000 R15: 1ffffffff1c00fbc default_idle+0x9/0x20 default_idle_call+0x72/0xb0 do_idle+0x1ea/0x4e0 cpu_startup_entry+0x43/0x60 rest_init+0x2de/0x300 start_kernel+0x3a6/0x400 x86_64_start_reservations+0x24/0x30 x86_64_start_kernel+0x137/0x1b0 common_startup_64+0x13e/0x147 </TASK>
***
If these findings have caused you to resend the series or submit a separate fix, please add the following tag to your commit message:
---
This report is generated by a bot. It may contain errors. syzbot ci engineers can be reached at [email protected].
To test a patch for this bug, please reply with #syz test (should be on a separate line).
The patch should be attached to the email.
Note: arguments like custom git repos and branches are not supported.