[v3] jfs: Prevent copying of nlink with value 0 from disk inode

[PATCH V3] jfs: Prevent copying of nlink with value 0 from disk inode

Posted by Edward Adam Davis 10 months ago

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().

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@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=355da3b3a74881008e8f
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
V1 -> V2: if the nlink of disk inode is 0 return -EIO
V2 -> V3: move the checking to diReadSpecial

---
 fs/jfs/jfs_imap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
index 0cedaccb7218..25bb3485da3b 100644
--- a/fs/jfs/jfs_imap.c
+++ b/fs/jfs/jfs_imap.c
@@ -460,7 +460,7 @@ struct inode *diReadSpecial(struct super_block *sb, ino_t inum, int secondary)
 	dp += inum % 8;		/* 8 inodes per 4K page */
 
 	/* copy on-disk inode to in-memory inode */
-	if ((copy_from_dinode(dp, ip)) != 0) {
+	if (!le32_to_cpu(dp->di_nlink) || (copy_from_dinode(dp, ip)) != 0) {
 		/* handle bad return by returning NULL for ip */
 		set_nlink(ip, 1);	/* Don't want iput() deleting it */
 		iput(ip);
-- 
2.43.0

Re: [PATCH V3] jfs: Prevent copying of nlink with value 0 from disk inode

Posted by Dave Kleikamp 10 months ago

On 2/20/25 5:13AM, Edward Adam Davis 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().
> 
> 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

I'm taking this patch, but making a change. It's a little cleaner to check ip->i_nlink after calling copy_from_dinode.

> 
> Reported-by: syzbot+355da3b3a74881008e8f@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=355da3b3a74881008e8f
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
> V1 -> V2: if the nlink of disk inode is 0 return -EIO
> V2 -> V3: move the checking to diReadSpecial
> 
> ---
>   fs/jfs/jfs_imap.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
> index 0cedaccb7218..25bb3485da3b 100644
> --- a/fs/jfs/jfs_imap.c
> +++ b/fs/jfs/jfs_imap.c
> @@ -460,7 +460,7 @@ struct inode *diReadSpecial(struct super_block *sb, ino_t inum, int secondary)
>   	dp += inum % 8;		/* 8 inodes per 4K page */
>   
>   	/* copy on-disk inode to in-memory inode */
> -	if ((copy_from_dinode(dp, ip)) != 0) {
> +	if (!le32_to_cpu(dp->di_nlink) || (copy_from_dinode(dp, ip)) != 0) {
>   		/* handle bad return by returning NULL for ip */
>   		set_nlink(ip, 1);	/* Don't want iput() deleting it */
>   		iput(ip);

My change:

diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
index 298445f6d3d4..ecb8e05b8b84 100644
--- a/fs/jfs/jfs_imap.c
+++ b/fs/jfs/jfs_imap.c
@@ -456,7 +456,7 @@ struct inode *diReadSpecial(struct super_block *sb, ino_t inum, int secondary)
  	dp += inum % 8;		/* 8 inodes per 4K page */
  
  	/* copy on-disk inode to in-memory inode */
-	if ((copy_from_dinode(dp, ip)) != 0) {
+	if ((copy_from_dinode(dp, ip) != 0) || (ip->i_nlink == 0)) {
  		/* handle bad return by returning NULL for ip */
  		set_nlink(ip, 1);	/* Don't want iput() deleting it */
  		iput(ip);

Re: [PATCH V3] jfs: Prevent copying of nlink with value 0 from disk inode

Posted by Edward Adam Davis 10 months ago

On Thu, 20 Feb 2025 10:15:04 -0600, Dave Kleikamp 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().
> > 
> > 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
> 
> I'm taking this patch, but making a change. It's a little cleaner to check ip->i_nlink after calling copy_from_dinode.
> 
> > 
> > Reported-by: syzbot+355da3b3a74881008e8f@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=355da3b3a74881008e8f
> > Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> > ---
> > V1 -> V2: if the nlink of disk inode is 0 return -EIO
> > V2 -> V3: move the checking to diReadSpecial
> > 
> > ---
> >   fs/jfs/jfs_imap.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
> > index 0cedaccb7218..25bb3485da3b 100644
> > --- a/fs/jfs/jfs_imap.c
> > +++ b/fs/jfs/jfs_imap.c
> > @@ -460,7 +460,7 @@ struct inode *diReadSpecial(struct super_block *sb, ino_t inum, int secondary)
> >   	dp += inum % 8;		/* 8 inodes per 4K page */
> >   
> >   	/* copy on-disk inode to in-memory inode */
> > -	if ((copy_from_dinode(dp, ip)) != 0) {
> > +	if (!le32_to_cpu(dp->di_nlink) || (copy_from_dinode(dp, ip)) != 0) {
> >   		/* handle bad return by returning NULL for ip */
> >   		set_nlink(ip, 1);	/* Don't want iput() deleting it */
> >   		iput(ip);
> 
> My change:
> 
> diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
> index 298445f6d3d4..ecb8e05b8b84 100644
> --- a/fs/jfs/jfs_imap.c
> +++ b/fs/jfs/jfs_imap.c
> @@ -456,7 +456,7 @@ struct inode *diReadSpecial(struct super_block *sb, ino_t inum, int secondary)
>   	dp += inum % 8;		/* 8 inodes per 4K page */
>   
>   	/* copy on-disk inode to in-memory inode */
> -	if ((copy_from_dinode(dp, ip)) != 0) {
> +	if ((copy_from_dinode(dp, ip) != 0) || (ip->i_nlink == 0)) {
This is incorrect. The purpose of adding this check is to prevent copy_from_dinode()
from using dip->i_nlink with a value of 0 to assign to ip.
>   		/* handle bad return by returning NULL for ip */
>   		set_nlink(ip, 1);	/* Don't want iput() deleting it */
>   		iput(ip);
BR,
Edward

Re: [PATCH V3] jfs: Prevent copying of nlink with value 0 from disk inode

Posted by Dave Kleikamp 10 months ago

On 2/20/25 5:22PM, Edward Adam Davis wrote:
> On Thu, 20 Feb 2025 10:15:04 -0600, Dave Kleikamp 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().
>>>
>>> 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
>>
>> I'm taking this patch, but making a change. It's a little cleaner to check ip->i_nlink after calling copy_from_dinode.
>>
>>>
>>> Reported-by: syzbot+355da3b3a74881008e8f@syzkaller.appspotmail.com
>>> Closes: https://syzkaller.appspot.com/bug?extid=355da3b3a74881008e8f
>>> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
>>> ---
>>> V1 -> V2: if the nlink of disk inode is 0 return -EIO
>>> V2 -> V3: move the checking to diReadSpecial
>>>
>>> ---
>>>    fs/jfs/jfs_imap.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
>>> index 0cedaccb7218..25bb3485da3b 100644
>>> --- a/fs/jfs/jfs_imap.c
>>> +++ b/fs/jfs/jfs_imap.c
>>> @@ -460,7 +460,7 @@ struct inode *diReadSpecial(struct super_block *sb, ino_t inum, int secondary)
>>>    	dp += inum % 8;		/* 8 inodes per 4K page */
>>>    
>>>    	/* copy on-disk inode to in-memory inode */
>>> -	if ((copy_from_dinode(dp, ip)) != 0) {
>>> +	if (!le32_to_cpu(dp->di_nlink) || (copy_from_dinode(dp, ip)) != 0) {
>>>    		/* handle bad return by returning NULL for ip */
>>>    		set_nlink(ip, 1);	/* Don't want iput() deleting it */
>>>    		iput(ip);
>>
>> My change:
>>
>> diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
>> index 298445f6d3d4..ecb8e05b8b84 100644
>> --- a/fs/jfs/jfs_imap.c
>> +++ b/fs/jfs/jfs_imap.c
>> @@ -456,7 +456,7 @@ struct inode *diReadSpecial(struct super_block *sb, ino_t inum, int secondary)
>>    	dp += inum % 8;		/* 8 inodes per 4K page */
>>    
>>    	/* copy on-disk inode to in-memory inode */
>> -	if ((copy_from_dinode(dp, ip)) != 0) {
>> +	if ((copy_from_dinode(dp, ip) != 0) || (ip->i_nlink == 0)) {
> This is incorrect. The purpose of adding this check is to prevent copy_from_dinode()
> from using dip->i_nlink with a value of 0 to assign to ip.
>>    		/* handle bad return by returning NULL for ip */
>>    		set_nlink(ip, 1);	/* Don't want iput() deleting it */

It will get set  to 1 right here ^^^

>>    		iput(ip);
> BR,
> Edward
>

Re: [PATCH V3] jfs: Prevent copying of nlink with value 0 from disk inode

Posted by Edward Adam Davis 9 months, 4 weeks ago

On Thu, 20 Feb 2025 17:28:49 -0600, Dave Kleikamp wrote:
>On 2/20/25 5:22PM, Edward Adam Davis wrote:
>> On Thu, 20 Feb 2025 10:15:04 -0600, Dave Kleikamp 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().
>>>>
>>>> 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
>>>
>>> I'm taking this patch, but making a change. It's a little cleaner to check ip->i_nlink after calling copy_from_dinode.
>>>
>>>>
>>>> Reported-by: syzbot+355da3b3a74881008e8f@syzkaller.appspotmail.com
>>>> Closes: https://syzkaller.appspot.com/bug?extid=355da3b3a74881008e8f
>>>> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
>>>> ---
>>>> V1 -> V2: if the nlink of disk inode is 0 return -EIO
>>>> V2 -> V3: move the checking to diReadSpecial
>>>>
>>>> ---
>>>>    fs/jfs/jfs_imap.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
>>>> index 0cedaccb7218..25bb3485da3b 100644
>>>> --- a/fs/jfs/jfs_imap.c
>>>> +++ b/fs/jfs/jfs_imap.c
>>>> @@ -460,7 +460,7 @@ struct inode *diReadSpecial(struct super_block *sb, ino_t inum, int secondary)
>>>>    	dp += inum % 8;		/* 8 inodes per 4K page */
>>>>    
>>>>    	/* copy on-disk inode to in-memory inode */
>>>> -	if ((copy_from_dinode(dp, ip)) != 0) {
>>>> +	if (!le32_to_cpu(dp->di_nlink) || (copy_from_dinode(dp, ip)) != 0) {
>>>>    		/* handle bad return by returning NULL for ip */
>>>>    		set_nlink(ip, 1);	/* Don't want iput() deleting it */
>>>>    		iput(ip);
>>>
>>> My change:
>>>
>>> diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
>>> index 298445f6d3d4..ecb8e05b8b84 100644
>>> --- a/fs/jfs/jfs_imap.c
>>> +++ b/fs/jfs/jfs_imap.c
>>> @@ -456,7 +456,7 @@ struct inode *diReadSpecial(struct super_block *sb, ino_t inum, int secondary)
>>>    	dp += inum % 8;		/* 8 inodes per 4K page */
>>>    
>>>    	/* copy on-disk inode to in-memory inode */
>>> -	if ((copy_from_dinode(dp, ip)) != 0) {
>>> +	if ((copy_from_dinode(dp, ip) != 0) || (ip->i_nlink == 0)) {
>> This is incorrect. The purpose of adding this check is to prevent copy_from_dinode()
>> from using dip->i_nlink with a value of 0 to assign to ip.
>>>    		/* handle bad return by returning NULL for ip */
>>>    		set_nlink(ip, 1);	/* Don't want iput() deleting it */
>
>It will get set  to 1 right here ^^^
Things that can be determined by "di_nlink" before executing copy_from_dinode(),
Why let the CPU run copy_from_dinode() for an extra time before checking?
Isn't this a waste of CPU?

BR,
Edward

Re: [PATCH V3] jfs: Prevent copying of nlink with value 0 from disk inode

Posted by Dave Kleikamp 9 months, 3 weeks ago

On 2/21/25 6:16PM, Edward Adam Davis wrote:
> On Thu, 20 Feb 2025 17:28:49 -0600, Dave Kleikamp wrote:
>> On 2/20/25 5:22PM, Edward Adam Davis wrote:
>>> On Thu, 20 Feb 2025 10:15:04 -0600, Dave Kleikamp 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().
>>>>>
>>>>> 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
>>>>
>>>> I'm taking this patch, but making a change. It's a little cleaner to check ip->i_nlink after calling copy_from_dinode.
>>>>
>>>>>
>>>>> Reported-by: syzbot+355da3b3a74881008e8f@syzkaller.appspotmail.com
>>>>> Closes: https://syzkaller.appspot.com/bug?extid=355da3b3a74881008e8f
>>>>> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
>>>>> ---
>>>>> V1 -> V2: if the nlink of disk inode is 0 return -EIO
>>>>> V2 -> V3: move the checking to diReadSpecial
>>>>>
>>>>> ---
>>>>>     fs/jfs/jfs_imap.c | 2 +-
>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
>>>>> index 0cedaccb7218..25bb3485da3b 100644
>>>>> --- a/fs/jfs/jfs_imap.c
>>>>> +++ b/fs/jfs/jfs_imap.c
>>>>> @@ -460,7 +460,7 @@ struct inode *diReadSpecial(struct super_block *sb, ino_t inum, int secondary)
>>>>>     	dp += inum % 8;		/* 8 inodes per 4K page */
>>>>>     
>>>>>     	/* copy on-disk inode to in-memory inode */
>>>>> -	if ((copy_from_dinode(dp, ip)) != 0) {
>>>>> +	if (!le32_to_cpu(dp->di_nlink) || (copy_from_dinode(dp, ip)) != 0) {
>>>>>     		/* handle bad return by returning NULL for ip */
>>>>>     		set_nlink(ip, 1);	/* Don't want iput() deleting it */
>>>>>     		iput(ip);
>>>>
>>>> My change:
>>>>
>>>> diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
>>>> index 298445f6d3d4..ecb8e05b8b84 100644
>>>> --- a/fs/jfs/jfs_imap.c
>>>> +++ b/fs/jfs/jfs_imap.c
>>>> @@ -456,7 +456,7 @@ struct inode *diReadSpecial(struct super_block *sb, ino_t inum, int secondary)
>>>>     	dp += inum % 8;		/* 8 inodes per 4K page */
>>>>     
>>>>     	/* copy on-disk inode to in-memory inode */
>>>> -	if ((copy_from_dinode(dp, ip)) != 0) {
>>>> +	if ((copy_from_dinode(dp, ip) != 0) || (ip->i_nlink == 0)) {
>>> This is incorrect. The purpose of adding this check is to prevent copy_from_dinode()
>>> from using dip->i_nlink with a value of 0 to assign to ip.
>>>>     		/* handle bad return by returning NULL for ip */
>>>>     		set_nlink(ip, 1);	/* Don't want iput() deleting it */
>>
>> It will get set  to 1 right here ^^^
> Things that can be determined by "di_nlink" before executing copy_from_dinode(),
> Why let the CPU run copy_from_dinode() for an extra time before checking?
> Isn't this a waste of CPU?

It's an exceptional case. It's very, very unlikely to fail, so the extra 
cpu cycles that are executed in the common case are not a concern.

> 
> BR,
> Edward
>