| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20241017085519.524401-1-lizhi.xu@windriver.com>
Date: Thu, 17 Oct 2024 16:55:19 +0800
From: Lizhi Xu <lizhi.xu@...driver.com>
To: <syzbot+ca440b457d21568f8021@...kaller.appspotmail.com>
CC: <jlbec@...lplan.org>, <joseph.qi@...ux.alibaba.com>,
<linux-kernel@...r.kernel.org>, <mark@...heh.com>,
<ocfs2-devel@...ts.linux.dev>, <syzkaller-bugs@...glegroups.com>
Subject: [PATCH] ocfs2: Fix possible deadlock in ocfs2_fiemap
Syzbot reported a possible circular locking dependency.
WARNING: possible circular locking dependency detected
6.12.0-rc2-syzkaller-00205-g1d227fcc7222 #0 Not tainted
------------------------------------------------------
syz-executor161/5226 is trying to acquire lock:
ffff88807e907398 (&mm->mmap_lock){++++}-{3:3}, at: __might_fault+0xaa/0x120 mm/memory.c:6700
but task is already holding lock:
ffff8880720d0660 (&oi->ip_alloc_sem){++++}-{3:3}, at: ocfs2_fiemap+0x377/0xf80 fs/ocfs2/extent_map.c:755
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #2 (&oi->ip_alloc_sem){++++}-{3:3}:
lock_acquire+0x1ed/0x550 kernel/locking/lockdep.c:5825
down_write+0x99/0x220 kernel/locking/rwsem.c:1577
ocfs2_page_mkwrite+0x346/0xed0 fs/ocfs2/mmap.c:142
do_page_mkwrite+0x198/0x480 mm/memory.c:3162
do_shared_fault mm/memory.c:5358 [inline]
do_fault mm/memory.c:5420 [inline]
do_pte_missing mm/memory.c:3965 [inline]
handle_pte_fault+0x11fa/0x6800 mm/memory.c:5751
__handle_mm_fault mm/memory.c:5894 [inline]
handle_mm_fault+0x1053/0x1ad0 mm/memory.c:6062
do_user_addr_fault arch/x86/mm/fault.c:1389 [inline]
handle_page_fault arch/x86/mm/fault.c:1481 [inline]
exc_page_fault+0x2b9/0x8c0 arch/x86/mm/fault.c:1539
asm_exc_page_fault+0x26/0x30 arch/x86/include/asm/idtentry.h:623
-> #1 (sb_pagefaults){.+.+}-{0:0}:
lock_acquire+0x1ed/0x550 kernel/locking/lockdep.c:5825
percpu_down_read include/linux/percpu-rwsem.h:51 [inline]
__sb_start_write include/linux/fs.h:1716 [inline]
sb_start_pagefault include/linux/fs.h:1881 [inline]
ocfs2_page_mkwrite+0x222/0xed0 fs/ocfs2/mmap.c:122
do_page_mkwrite+0x198/0x480 mm/memory.c:3162
do_shared_fault mm/memory.c:5358 [inline]
do_fault mm/memory.c:5420 [inline]
do_pte_missing mm/memory.c:3965 [inline]
handle_pte_fault+0x11fa/0x6800 mm/memory.c:5751
__handle_mm_fault mm/memory.c:5894 [inline]
handle_mm_fault+0x1053/0x1ad0 mm/memory.c:6062
do_user_addr_fault arch/x86/mm/fault.c:1389 [inline]
handle_page_fault arch/x86/mm/fault.c:1481 [inline]
exc_page_fault+0x2b9/0x8c0 arch/x86/mm/fault.c:1539
asm_exc_page_fault+0x26/0x30 arch/x86/include/asm/idtentry.h:623
-> #0 (&mm->mmap_lock){++++}-{3:3}:
check_prev_add kernel/locking/lockdep.c:3161 [inline]
check_prevs_add kernel/locking/lockdep.c:3280 [inline]
validate_chain+0x18ef/0x5920 kernel/locking/lockdep.c:3904
__lock_acquire+0x1384/0x2050 kernel/locking/lockdep.c:5202
lock_acquire+0x1ed/0x550 kernel/locking/lockdep.c:5825
__might_fault+0xc6/0x120 mm/memory.c:6700
_inline_copy_to_user include/linux/uaccess.h:183 [inline]
_copy_to_user+0x2a/0xb0 lib/usercopy.c:26
copy_to_user include/linux/uaccess.h:216 [inline]
fiemap_fill_next_extent+0x235/0x410 fs/ioctl.c:145
ocfs2_fiemap+0x9f1/0xf80 fs/ocfs2/extent_map.c:796
ioctl_fiemap fs/ioctl.c:220 [inline]
do_vfs_ioctl+0x1bf8/0x2e40 fs/ioctl.c:841
__do_sys_ioctl fs/ioctl.c:905 [inline]
__se_sys_ioctl+0x81/0x170 fs/ioctl.c:893
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x77/0x7f
other info that might help us debug this:
Chain exists of:
&mm->mmap_lock --> sb_pagefaults --> &oi->ip_alloc_sem
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
rlock(&oi->ip_alloc_sem);
lock(sb_pagefaults);
lock(&oi->ip_alloc_sem);
rlock(&mm->mmap_lock);
*** DEADLOCK ***
Fix it by reordering locks sb_pagefaults and ip_alloc_sem, because
fiemap_fill_next_extent() does not need to be in the lock ip_alloc_sem,
cancel and retrieve ip_alloc_sem before and after the execution of it.
Reported-and-tested-by: syzbot+ca440b457d21568f8021@...kaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=ca440b457d21568f8021
Signed-off-by: Lizhi Xu <lizhi.xu@...driver.com>
---
fs/ocfs2/extent_map.c | 2 ++
fs/ocfs2/mmap.c | 14 +++++++-------
2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
index f7672472fa82..6d5ffa803b31 100644
--- a/fs/ocfs2/extent_map.c
+++ b/fs/ocfs2/extent_map.c
@@ -793,8 +793,10 @@ int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
phys_bytes = le64_to_cpu(rec.e_blkno) << osb->sb->s_blocksize_bits;
virt_bytes = (u64)le32_to_cpu(rec.e_cpos) << osb->s_clustersize_bits;
+ up_read(&OCFS2_I(inode)->ip_alloc_sem);
ret = fiemap_fill_next_extent(fieinfo, virt_bytes, phys_bytes,
len_bytes, fe_flags);
+ down_read(&OCFS2_I(inode)->ip_alloc_sem);
if (ret)
break;
diff --git a/fs/ocfs2/mmap.c b/fs/ocfs2/mmap.c
index 6ef4cb045ccd..f7863f7fb4a1 100644
--- a/fs/ocfs2/mmap.c
+++ b/fs/ocfs2/mmap.c
@@ -119,9 +119,6 @@ static vm_fault_t ocfs2_page_mkwrite(struct vm_fault *vmf)
int err;
vm_fault_t ret;
- sb_start_pagefault(inode->i_sb);
- ocfs2_block_signals(&oldset);
-
/*
* The cluster locks taken will block a truncate from another
* node. Taking the data lock will also ensure that we don't
@@ -131,7 +128,7 @@ static vm_fault_t ocfs2_page_mkwrite(struct vm_fault *vmf)
if (err < 0) {
mlog_errno(err);
ret = vmf_error(err);
- goto out;
+ return ret;
}
/*
@@ -141,16 +138,19 @@ static vm_fault_t ocfs2_page_mkwrite(struct vm_fault *vmf)
*/
down_write(&OCFS2_I(inode)->ip_alloc_sem);
+ sb_start_pagefault(inode->i_sb);
+ ocfs2_block_signals(&oldset);
+
ret = __ocfs2_page_mkwrite(vmf->vma->vm_file, di_bh, page);
+ ocfs2_unblock_signals(&oldset);
+ sb_end_pagefault(inode->i_sb);
+
up_write(&OCFS2_I(inode)->ip_alloc_sem);
brelse(di_bh);
ocfs2_inode_unlock(inode, 1);
-out:
- ocfs2_unblock_signals(&oldset);
- sb_end_pagefault(inode->i_sb);
return ret;
}
--
2.43.0
Powered by blists - more mailing lists