linux-kernel - Re: [PATCH v2] erofs: fix lockdep false positives on initializing erofs_pseudo

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Fri, 8 Mar 2024 09:04:44 +0800
From: Baokun Li <libaokun1@...wei.com>
To: Gao Xiang <hsiangkao@...ux.alibaba.com>, <linux-erofs@...ts.ozlabs.org>
CC: <xiang@...nel.org>, <chao@...nel.org>, <huyue2@...lpad.com>,
	<jefflexu@...ux.alibaba.com>, <viro@...iv.linux.org.uk>,
	<brauner@...nel.org>, <linux-kernel@...r.kernel.org>, <yangerkun@...wei.com>,
	<houtao1@...wei.com>, <yukuai3@...wei.com>, <chengzhihao1@...wei.com>, Baokun
 Li <libaokun1@...wei.com>
Subject: Re: [PATCH v2] erofs: fix lockdep false positives on initializing
 erofs_pseudo_mnt

On 2024/3/7 22:18, Gao Xiang wrote:
>
>
> On 2024/3/7 18:10, Baokun Li wrote:
>> Lockdep reported the following issue when mounting erofs with a 
>> domain_id:
>>
>> ============================================
>> WARNING: possible recursive locking detected
>> 6.8.0-rc7-xfstests #521 Not tainted
>> --------------------------------------------
>> mount/396 is trying to acquire lock:
>> ffff907a8aaaa0e0 (&type->s_umount_key#50/1){+.+.}-{3:3},
>>                         at: alloc_super+0xe3/0x3d0
>>
>> but task is already holding lock:
>> ffff907a8aaa90e0 (&type->s_umount_key#50/1){+.+.}-{3:3},
>>                         at: alloc_super+0xe3/0x3d0
>>
>> other info that might help us debug this:
>>   Possible unsafe locking scenario:
>>
>>         CPU0
>>         ----
>>    lock(&type->s_umount_key#50/1);
>>    lock(&type->s_umount_key#50/1);
>>
>>   *** DEADLOCK ***
>>
>>   May be due to missing lock nesting notation
>>
>> 2 locks held by mount/396:
>>   #0: ffff907a8aaa90e0 (&type->s_umount_key#50/1){+.+.}-{3:3},
>>             at: alloc_super+0xe3/0x3d0
>>   #1: ffffffffc00e6f28 (erofs_domain_list_lock){+.+.}-{3:3},
>>             at: erofs_fscache_register_fs+0x3d/0x270 [erofs]
>>
>> stack backtrace:
>> CPU: 1 PID: 396 Comm: mount Not tainted 6.8.0-rc7-xfstests #521
>> Call Trace:
>>   <TASK>
>>   dump_stack_lvl+0x64/0xb0
>>   validate_chain+0x5c4/0xa00
>>   __lock_acquire+0x6a9/0xd50
>>   lock_acquire+0xcd/0x2b0
>>   down_write_nested+0x45/0xd0
>>   alloc_super+0xe3/0x3d0
>>   sget_fc+0x62/0x2f0
>>   vfs_get_super+0x21/0x90
>>   vfs_get_tree+0x2c/0xf0
>>   fc_mount+0x12/0x40
>>   vfs_kern_mount.part.0+0x75/0x90
>>   kern_mount+0x24/0x40
>>   erofs_fscache_register_fs+0x1ef/0x270 [erofs]
>>   erofs_fc_fill_super+0x213/0x380 [erofs]
>>
>> This is because the file_system_type of both erofs and the pseudo-mount
>> point of domain_id is erofs_fs_type, so two successive calls to
>> alloc_super() are considered to be using the same lock and trigger the
>> warning above.
>>
>> Therefore add a nodev file_system_type called erofs_anon_fs_type in
>> fscache.c to silence this complaint. Because kern_mount() takes a
>> pointer to struct file_system_type, not its (string) name. So we don't
>> need to call register_filesystem(). In addition, call init_pseudo() in
>> erofs_anon_init_fs_context() as suggested by Al Viro, so that we can
>> remove erofs_fc_fill_pseudo_super(), erofs_fc_anon_get_tree(), and
>> erofs_anon_context_ops.
>>
>> Signed-off-by: Baokun Li <libaokun1@...wei.com>
>
> I will add
>
> Suggested-by: Al Viro <viro@...iv.linux.org.uk>
>
> when applying..
Okay, thanks for adding it.
>
> Also since it's a false positive and too close to the
> final so let's keep this patch in -next for a while and
> then aim for v6.9-rc1 instead.
>
> Thanks,
> Gao Xiang
Fine! Thanks!
-- 
With Best Regards,
Baokun Li
.