| 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: <b72fbe46-f910-4181-8d5b-fdc66d46700a@oracle.com>
Date: Wed, 19 Feb 2025 10:38:39 -0600
From: Dave Kleikamp <dave.kleikamp@...cle.com>
To: Edward Adam Davis <eadavis@...com>
Cc: jfs-discussion@...ts.sourceforge.net, syzkaller-bugs@...glegroups.com,
linux-kernel@...r.kernel.org,
syzbot+355da3b3a74881008e8f@...kaller.appspotmail.com
Subject: Re: [Jfs-discussion] [PATCH V2] jfs: Prevent copying of nlink with
value 0 from disk inode
I'm catching up on some long-ignored emails and have a concern about
this patch.
On 12/24/24 2:17AM, Edward Adam Davis via Jfs-discussion wrote:
> syzbot report a deadlock in diFree. [1]
>
> When calling "ioctl$LOOP_SET_STATUS64", the offset value passed in is 4,
> which does not match the mounted loop device, causing the mapping of the
> mounted loop device to be invalidated.
>
> When creating the directory and creating the inode of iag in diReadSpecial(),
> read the page of fixed disk inode (AIT) in raw mode in read_metapage(), the
> metapage data it returns is corrupted, which causes the nlink value of 0 to be
> assigned to the iag inode when executing copy_from_dinode(), which ultimately
> causes a deadlock when entering diFree().
I'm not quite sure if it's always a problem to read an inode with
i_nlink = 0, so I think it may be safer to move this check from
copy_from_dinode to diReadSpecial.
Shaggy
>
> To avoid this, first check the nlink value of dinode before setting iag inode.
>
> [1]
> WARNING: possible recursive locking detected
> 6.12.0-rc7-syzkaller-00212-g4a5df3796467 #0 Not tainted
> --------------------------------------------
> syz-executor301/5309 is trying to acquire lock:
> ffff888044548920 (&(imap->im_aglock[index])){+.+.}-{3:3}, at: diFree+0x37c/0x2fb0 fs/jfs/jfs_imap.c:889
>
> but task is already holding lock:
> ffff888044548920 (&(imap->im_aglock[index])){+.+.}-{3:3}, at: diAlloc+0x1b6/0x1630
>
> other info that might help us debug this:
> Possible unsafe locking scenario:
>
> CPU0
> ----
> lock(&(imap->im_aglock[index]));
> lock(&(imap->im_aglock[index]));
>
> *** DEADLOCK ***
>
> May be due to missing lock nesting notation
>
> 5 locks held by syz-executor301/5309:
> #0: ffff8880422a4420 (sb_writers#9){.+.+}-{0:0}, at: mnt_want_write+0x3f/0x90 fs/namespace.c:515
> #1: ffff88804755b390 (&type->i_mutex_dir_key#6/1){+.+.}-{3:3}, at: inode_lock_nested include/linux/fs.h:850 [inline]
> #1: ffff88804755b390 (&type->i_mutex_dir_key#6/1){+.+.}-{3:3}, at: filename_create+0x260/0x540 fs/namei.c:4026
> #2: ffff888044548920 (&(imap->im_aglock[index])){+.+.}-{3:3}, at: diAlloc+0x1b6/0x1630
> #3: ffff888044548890 (&imap->im_freelock){+.+.}-{3:3}, at: diNewIAG fs/jfs/jfs_imap.c:2460 [inline]
> #3: ffff888044548890 (&imap->im_freelock){+.+.}-{3:3}, at: diAllocExt fs/jfs/jfs_imap.c:1905 [inline]
> #3: ffff888044548890 (&imap->im_freelock){+.+.}-{3:3}, at: diAllocAG+0x4b7/0x1e50 fs/jfs/jfs_imap.c:1669
> #4: ffff88804755a618 (&jfs_ip->rdwrlock/1){++++}-{3:3}, at: diNewIAG fs/jfs/jfs_imap.c:2477 [inline]
> #4: ffff88804755a618 (&jfs_ip->rdwrlock/1){++++}-{3:3}, at: diAllocExt fs/jfs/jfs_imap.c:1905 [inline]
> #4: ffff88804755a618 (&jfs_ip->rdwrlock/1){++++}-{3:3}, at: diAllocAG+0x869/0x1e50 fs/jfs/jfs_imap.c:1669
>
> stack backtrace:
> CPU: 0 UID: 0 PID: 5309 Comm: syz-executor301 Not tainted 6.12.0-rc7-syzkaller-00212-g4a5df3796467 #0
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2~bpo12+1 04/01/2014
> Call Trace:
> <TASK>
> __dump_stack lib/dump_stack.c:94 [inline]
> dump_stack_lvl+0x241/0x360 lib/dump_stack.c:120
> print_deadlock_bug+0x483/0x620 kernel/locking/lockdep.c:3037
> check_deadlock kernel/locking/lockdep.c:3089 [inline]
> validate_chain+0x15e2/0x5920 kernel/locking/lockdep.c:3891
> __lock_acquire+0x1384/0x2050 kernel/locking/lockdep.c:5202
> lock_acquire+0x1ed/0x550 kernel/locking/lockdep.c:5825
> __mutex_lock_common kernel/locking/mutex.c:608 [inline]
> __mutex_lock+0x136/0xd70 kernel/locking/mutex.c:752
> diFree+0x37c/0x2fb0 fs/jfs/jfs_imap.c:889
> jfs_evict_inode+0x32d/0x440 fs/jfs/inode.c:156
> evict+0x4e8/0x9b0 fs/inode.c:725
> diFreeSpecial fs/jfs/jfs_imap.c:552 [inline]
> duplicateIXtree+0x3c6/0x550 fs/jfs/jfs_imap.c:3022
> diNewIAG fs/jfs/jfs_imap.c:2597 [inline]
> diAllocExt fs/jfs/jfs_imap.c:1905 [inline]
> diAllocAG+0x17dc/0x1e50 fs/jfs/jfs_imap.c:1669
> diAlloc+0x1d2/0x1630 fs/jfs/jfs_imap.c:1590
> ialloc+0x8f/0x900 fs/jfs/jfs_inode.c:56
> jfs_mkdir+0x1c5/0xba0 fs/jfs/namei.c:225
> vfs_mkdir+0x2f9/0x4f0 fs/namei.c:4257
> do_mkdirat+0x264/0x3a0 fs/namei.c:4280
> __do_sys_mkdirat fs/namei.c:4295 [inline]
> __se_sys_mkdirat fs/namei.c:4293 [inline]
> __x64_sys_mkdirat+0x87/0xa0 fs/namei.c:4293
> 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
>
> Reported-by: syzbot+355da3b3a74881008e8f@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=355da3b3a74881008e8f
> Signed-off-by: Edward Adam Davis <eadavis@...com>
> ---
> V1 -> V2: if the nlink of disk inode is 0 return -EIO
>
> fs/jfs/jfs_imap.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
> index a360b24ed320..b3146e335782 100644
> --- a/fs/jfs/jfs_imap.c
> +++ b/fs/jfs/jfs_imap.c
> @@ -3035,6 +3035,7 @@ static int copy_from_dinode(struct dinode * dip, struct inode *ip)
> {
> struct jfs_inode_info *jfs_ip = JFS_IP(ip);
> struct jfs_sb_info *sbi = JFS_SBI(ip->i_sb);
> + u32 di_nlink;
>
> jfs_ip->fileset = le32_to_cpu(dip->di_fileset);
> jfs_ip->mode2 = le32_to_cpu(dip->di_mode);
> @@ -3053,7 +3054,12 @@ static int copy_from_dinode(struct dinode * dip, struct inode *ip)
> ip->i_mode |= 0001;
> }
> }
> - set_nlink(ip, le32_to_cpu(dip->di_nlink));
> +
> + di_nlink = le32_to_cpu(dip->di_nlink);
> + if (!di_nlink)
> + return -EIO;
> +
> + set_nlink(ip, di_nlink);
>
> jfs_ip->saved_uid = make_kuid(&init_user_ns, le32_to_cpu(dip->di_uid));
> if (!uid_valid(sbi->uid))
Powered by blists - more mailing lists