linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* Re: [syzbot] possible deadlock in mnt_want_write (2)
       [not found] ` <c75858662b90592ae3f3ab57d4500381a28bafe5.camel@linux.ibm.com>
@ 2022-07-06 12:10   ` Hillf Danton
  2022-07-06 22:24     ` Mimi Zohar
  0 siblings, 1 reply; 2+ messages in thread
From: Hillf Danton @ 2022-07-06 12:10 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: syzbot, linux-fsdevel, linux-mm, linux-integrity, linux-kernel,
	miklos, syzkaller-bugs, syzbot, Amir Goldstein, Hillf Danton

On Tue, 05 Jul 2022 08:53:15 -0400 Mimi Zohar wrote:
> 
> Thank you for the reproducer.  This seems to be a similar false
> positive as was discussed:
> https://lore.kernel.org/linux-unionfs/000000000000c5b77105b4c3546e@google.com/
> 
> thanks,
> 

Hi Mimi

Please pick up the patch attached if it makes sense to you.

Hillf

> 
> On Sat, 2022-07-02 at 10:27 -0700, syzbot wrote:
> > syzbot has found a reproducer for the following issue on:
> > 
> > HEAD commit:    089866061428 Merge tag 'libnvdimm-fixes-5.19-rc5' of git:/..
> > git tree:       upstream
> > console output: https://syzkaller.appspot.com/x/log.txt?x=11dd91f0080000
> > kernel config:  https://syzkaller.appspot.com/x/.config?x=75c9ff14e1db87c0
> > dashboard link: https://syzkaller.appspot.com/bug?extid=b42fe626038981fb7bfa
> > compiler:       Debian clang version 13.0.1-++20220126092033+75e33f71c2da-1~exp1~20220126212112.63, GNU ld (GNU Binutils for Debian) 2.35.2
> > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=167bafc0080000
> > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=11aad3e0080000
> > 
> > IMPORTANT: if you fix the issue, please add the following tag to the commit:
> > Reported-by: syzbot+b42fe626038981fb7bfa@syzkaller.appspotmail.com
> > 
> > ======================================================
> > WARNING: possible circular locking dependency detected
> > 5.19.0-rc4-syzkaller-00187-g089866061428 #0 Not tainted
> > ------------------------------------------------------
> > syz-executor450/3829 is trying to acquire lock:
> > ffff88807e574460 (sb_writers#4){.+.+}-{0:0}, at: mnt_want_write+0x3b/0x80 fs/namespace.c:393
> > 
> > but task is already holding lock:
> > ffff888074de91a0 (&iint->mutex){+.+.}-{3:3}, at: process_measurement+0x7d2/0x1c10 security/integrity/ima/ima_main.c:260
> > 
> > which lock already depends on the new lock.
> > 
> > 
> > the existing dependency chain (in reverse order) is:
> > 
> > -> #1 (&iint->mutex){+.+.}-{3:3}:
> >        lock_acquire+0x1a7/0x400 kernel/locking/lockdep.c:5665
> >        __mutex_lock_common+0x1de/0x26c0 kernel/locking/mutex.c:603
> >        __mutex_lock kernel/locking/mutex.c:747 [inline]
> >        mutex_lock_nested+0x17/0x20 kernel/locking/mutex.c:799
> >        process_measurement+0x7d2/0x1c10 security/integrity/ima/ima_main.c:260
> >        ima_file_check+0xe7/0x160 security/integrity/ima/ima_main.c:517
> >        do_open fs/namei.c:3522 [inline]
> >        path_openat+0x2705/0x2ec0 fs/namei.c:3653
> >        do_filp_open+0x277/0x4f0 fs/namei.c:3680
> >        do_sys_openat2+0x13b/0x500 fs/open.c:1278
> >        do_sys_open fs/open.c:1294 [inline]
> >        __do_sys_open fs/open.c:1302 [inline]
> >        __se_sys_open fs/open.c:1298 [inline]
> >        __x64_sys_open+0x221/0x270 fs/open.c:1298
> >        do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> >        do_syscall_64+0x2b/0x70 arch/x86/entry/common.c:80
> >        entry_SYSCALL_64_after_hwframe+0x46/0xb0
> > 
> > -> #0 (sb_writers#4){.+.+}-{0:0}:
> >        check_prev_add kernel/locking/lockdep.c:3095 [inline]
> >        check_prevs_add kernel/locking/lockdep.c:3214 [inline]
> >        validate_chain+0x185c/0x65c0 kernel/locking/lockdep.c:3829
> >        __lock_acquire+0x129a/0x1f80 kernel/locking/lockdep.c:5053
> >        lock_acquire+0x1a7/0x400 kernel/locking/lockdep.c:5665
> >        percpu_down_read include/linux/percpu-rwsem.h:51 [inline]
> >        __sb_start_write include/linux/fs.h:1699 [inline]
> >        sb_start_write+0x4d/0x1a0 include/linux/fs.h:1774
> >        mnt_want_write+0x3b/0x80 fs/namespace.c:393
> >        ovl_maybe_copy_up+0x124/0x190 fs/overlayfs/copy_up.c:1078
> >        ovl_open+0x106/0x2a0 fs/overlayfs/file.c:152
> >        do_dentry_open+0x789/0x1040 fs/open.c:848
> >        vfs_open fs/open.c:981 [inline]
> >        dentry_open+0xc1/0x120 fs/open.c:997
> >        ima_calc_file_hash+0x157/0x1cb0 security/integrity/ima/ima_crypto.c:557
> >        ima_collect_measurement+0x3de/0x850 security/integrity/ima/ima_api.c:292
> >        process_measurement+0xf87/0x1c10 security/integrity/ima/ima_main.c:337
> >        ima_file_check+0xe7/0x160 security/integrity/ima/ima_main.c:517
> >        do_open fs/namei.c:3522 [inline]
> >        path_openat+0x2705/0x2ec0 fs/namei.c:3653
> >        do_filp_open+0x277/0x4f0 fs/namei.c:3680
> >        do_sys_openat2+0x13b/0x500 fs/open.c:1278
> >        do_sys_open fs/open.c:1294 [inline]
> >        __do_sys_open fs/open.c:1302 [inline]
> >        __se_sys_open fs/open.c:1298 [inline]
> >        __x64_sys_open+0x221/0x270 fs/open.c:1298
> >        do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> >        do_syscall_64+0x2b/0x70 arch/x86/entry/common.c:80
> >        entry_SYSCALL_64_after_hwframe+0x46/0xb0
> > 
> > other info that might help us debug this:
> > 
> >  Possible unsafe locking scenario:
> > 
> >        CPU0                    CPU1
> >        ----                    ----
> >   lock(&iint->mutex);
> >                                lock(sb_writers#4);
> >                                lock(&iint->mutex);
> >   lock(sb_writers#4);
> > 
> >  *** DEADLOCK ***
> > 
> > 1 lock held by syz-executor450/3829:
> >  #0: ffff888074de91a0 (&iint->mutex){+.+.}-{3:3}, at: process_measurement+0x7d2/0x1c10 security/integrity/ima/ima_main.c:260
> > 
> > stack backtrace:
> > CPU: 1 PID: 3829 Comm: syz-executor450 Not tainted 5.19.0-rc4-syzkaller-00187-g089866061428 #0
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 06/29/2022
> > Call Trace:
> >  <TASK>
> >  __dump_stack lib/dump_stack.c:88 [inline]
> >  dump_stack_lvl+0x1e3/0x2cb lib/dump_stack.c:106
> >  check_noncircular+0x2f7/0x3b0 kernel/locking/lockdep.c:2175
> >  check_prev_add kernel/locking/lockdep.c:3095 [inline]
> >  check_prevs_add kernel/locking/lockdep.c:3214 [inline]
> >  validate_chain+0x185c/0x65c0 kernel/locking/lockdep.c:3829
> >  __lock_acquire+0x129a/0x1f80 kernel/locking/lockdep.c:5053
> >  lock_acquire+0x1a7/0x400 kernel/locking/lockdep.c:5665
> >  percpu_down_read include/linux/percpu-rwsem.h:51 [inline]
> >  __sb_start_write include/linux/fs.h:1699 [inline]
> >  sb_start_write+0x4d/0x1a0 include/linux/fs.h:1774
> >  mnt_want_write+0x3b/0x80 fs/namespace.c:393
> >  ovl_maybe_copy_up+0x124/0x190 fs/overlayfs/copy_up.c:1078
> >  ovl_open+0x106/0x2a0 fs/overlayfs/file.c:152
> >  do_dentry_open+0x789/0x1040 fs/open.c:848
> >  vfs_open fs/open.c:981 [inline]
> >  dentry_open+0xc1/0x120 fs/open.c:997
> >  ima_calc_file_hash+0x157/0x1cb0 security/integrity/ima/ima_crypto.c:557
> >  ima_collect_measurement+0x3de/0x850 security/integrity/ima/ima_api.c:292
> >  process_measurement+0xf87/0x1c10 security/integrity/ima/ima_main.c:337
> >  ima_file_check+0xe7/0x160 security/integrity/ima/ima_main.c:517
> >  do_open fs/namei.c:3522 [inline]
> >  path_openat+0x2705/0x2ec0 fs/namei.c:3653
> >  do_filp_open+0x277/0x4f0 fs/namei.c:3680
> >  do_sys_openat2+0x13b/0x500 fs/open.c:1278
> >  do_sys_open fs/open.c:1294 [inline]
> >  __do_sys_open fs/open.c:1302 [inline]
> >  __se_sys_open fs/open.c:1298 [inline]
> >  __x64_sys_open+0x221/0x270 fs/open.c:1298
> >  do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> >  do_syscall_64+0x2b/0x70 arch/x86/entry/common.c:80
> >  entry_SYSCALL_64_after_hwframe+0x46/0xb0
> > RIP: 0033:0x7faf98402749
> > Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 31 16 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48
> > RSP: 002b:00007faf9838e2f8 EFLAGS: 00000246 ORIG_RAX: 0000000000000002
> > RAX: ffffffffffffffda RBX: 00007faf98491270 RCX: 00007faf98402749
> > RDX: 0000000000000000 RSI: 000000000000000b RDI: 00000000200000c0
> > RBP: 00007faf98458504 R08: 0000000000000000 R09: 0000000000000000
> > R10: 0000000000000000 R11: 0000000000000246 R12: 0030656c69662f2e
> > R13: 3d7269647265776f R14: 0079616c7265766f R15: 00007faf98491278
> >  </TASK>
> > 

--->8---
From: Hillf Danton <hdanton@sina.com>
Subject: [PATCH] integrity: lockdep annotate of iint->mutex

This fixes a reported lockdep splat

        CPU0                    CPU1
        ----                    ----
	lock(&iint->mutex);
				lock(sb_writers#4);
				lock(&iint->mutex);
	lock(sb_writers#4);
 
	*** DEADLOCK ***

using the method in 4eae06de482b annotating OVL_I(inode)->lock.

Links: https://lore.kernel.org/linux-unionfs/CAOQ4uxjk4XYuwz5HCmN-Ge=Ld=tM1f7ZxVrd5U1AC2Wisc9MTA@mail.gmail.com/
Reported-and-tested-by: syzbot <syzbot+b42fe626038981fb7bfa@syzkaller.appspotmail.com>
Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Hillf Danton <hdanton@sina.com>
---

--- a/security/integrity/iint.c 
+++ b/security/integrity/iint.c 
@@ -85,6 +85,17 @@ static void iint_free(struct integrity_i
 	kmem_cache_free(iint_cache, iint);
 }
 
+static void iint_annotate_mutex_key(struct integrity_iint_cache *iint, struct inode *inode)
+{
+#ifdef CONFIG_LOCKDEP
+	static struct lock_class_key iint_mutex_key[FILESYSTEM_MAX_STACK_DEPTH];
+
+	int depth = inode->i_sb->s_stack_depth;
+
+	lockdep_set_class(&iint->mutex, &iint_mutex_key[depth]);
+#endif
+}
+
 /**
  * integrity_inode_get - find or allocate an iint associated with an inode
  * @inode: pointer to the inode
@@ -114,6 +125,8 @@ struct integrity_iint_cache *integrity_i
 	if (!iint)
 		return NULL;
 
+	iint_annotate_mutex_key(iint, inode);
+
 	write_lock(&integrity_iint_lock);
 
 	p = &integrity_iint_tree.rb_node;
--


^ permalink raw reply	[flat|nested] 2+ messages in thread
* Re: [syzbot] possible deadlock in mnt_want_write (2)
  2022-07-06 12:10   ` [syzbot] possible deadlock in mnt_want_write (2) Hillf Danton
@ 2022-07-06 22:24     ` Mimi Zohar
  0 siblings, 0 replies; 2+ messages in thread
From: Mimi Zohar @ 2022-07-06 22:24 UTC (permalink / raw)
  To: Hillf Danton
  Cc: syzbot, linux-fsdevel, linux-mm, linux-integrity, linux-kernel,
	miklos, syzkaller-bugs, syzbot, Amir Goldstein

Hi Hillf,g

On Wed, 2022-07-06 at 20:10 +0800, Hillf Danton wrote:
> On Tue, 05 Jul 2022 08:53:15 -0400 Mimi Zohar wrote:
> > 
> > Thank you for the reproducer.  This seems to be a similar false
> > positive as was discussed:
> > https://lore.kernel.org/linux-unionfs/000000000000c5b77105b4c3546e@google.com/
> > 
> > thanks,
> > 
> 
> Hi Mimi
> 
> Please pick up the patch attached if it makes sense to you.

The patch itself looks good, but missing from the patch description is
an indication that the lockdep warning is a false positive.  Perhaps
add a "Suggested-by" line crediting Amir.   I'd appreciate your posting
the patch on the mailing list.

thanks!

Mimi

> From: Hillf Danton <hdanton@sina.com>
> Subject: [PATCH] integrity: lockdep annotate of iint->mutex
> 
> This fixes a reported lockdep splat
> 
>         CPU0                    CPU1
>         ----                    ----
> 	lock(&iint->mutex);
> 				lock(sb_writers#4);
> 				lock(&iint->mutex);
> 	lock(sb_writers#4);
>  
> 	*** DEADLOCK ***
> 
> using the method in 4eae06de482b annotating OVL_I(inode)->lock.
> 
> Links: https://lore.kernel.org/linux-unionfs/CAOQ4uxjk4XYuwz5HCmN-Ge=Ld=tM1f7ZxVrd5U1AC2Wisc9MTA@mail.gmail.com/
> Reported-and-tested-by: syzbot <syzbot+b42fe626038981fb7bfa@syzkaller.appspotmail.com>
> Cc: Mimi Zohar <zohar@linux.ibm.com>
> Cc: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Hillf Danton <hdanton@sina.com>
> ---
> 
> --- a/security/integrity/iint.c 
> +++ b/security/integrity/iint.c 
> @@ -85,6 +85,17 @@ static void iint_free(struct integrity_i
>  	kmem_cache_free(iint_cache, iint);
>  }
>  
> +static void iint_annotate_mutex_key(struct integrity_iint_cache *iint, struct inode *inode)
> +{
> +#ifdef CONFIG_LOCKDEP
> +	static struct lock_class_key iint_mutex_key[FILESYSTEM_MAX_STACK_DEPTH];
> +
> +	int depth = inode->i_sb->s_stack_depth;
> +
> +	lockdep_set_class(&iint->mutex, &iint_mutex_key[depth]);
> +#endif
> +}
> +
>  /**
>   * integrity_inode_get - find or allocate an iint associated with an inode
>   * @inode: pointer to the inode
> @@ -114,6 +125,8 @@ struct integrity_iint_cache *integrity_i
>  	if (!iint)
>  		return NULL;
>  
> +	iint_annotate_mutex_key(iint, inode);
> +
>  	write_lock(&integrity_iint_lock);
>  
>  	p = &integrity_iint_tree.rb_node;
> --




^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-07-06 22:24 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <000000000000466f0d05e2d5d1d1@google.com>
     [not found] ` <c75858662b90592ae3f3ab57d4500381a28bafe5.camel@linux.ibm.com>
2022-07-06 12:10   ` [syzbot] possible deadlock in mnt_want_write (2) Hillf Danton
2022-07-06 22:24     ` Mimi Zohar

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 NNTP newsgroup(s).