[syzbot] [efi?] [fs?] possible deadlock in efivarfs_actor

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit: e056da87c780 Merge remote-tracking branch 'will/for-next/p..
git tree: git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-kernelci
console output: https://syzkaller.appspot.com/x/log.txt?x=14ce9c64580000
kernel config: https://syzkaller.appspot.com/x/.config?x=d6b7e15dc5b5e776
dashboard link: https://syzkaller.appspot.com/bug?extid=019072ad24ab1d948228
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
userspace arch: arm64
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=111ed7a0580000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=13b97c64580000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/3d8b1b7cc4c0/disk-e056da87.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/b84c04cff235/vmlinux-e056da87.xz
kernel image: https://storage.googleapis.com/syzbot-assets/2ae4d0525881/Image-e056da87.gz.xz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+019072...@syzkaller.appspotmail.com

efivarfs: resyncing variable state
============================================
WARNING: possible recursive locking detected
6.14.0-rc4-syzkaller-ge056da87c780 #0 Not tainted
--------------------------------------------
syz-executor772/6443 is trying to acquire lock:
ffff0000c6826558 (&sb->s_type->i_mutex_key#16){++++}-{4:4}, at: inode_lock include/linux/fs.h:877 [inline]
ffff0000c6826558 (&sb->s_type->i_mutex_key#16){++++}-{4:4}, at: efivarfs_actor+0x1b8/0x2b8 fs/efivarfs/super.c:422

but task is already holding lock:
ffff0000c6c7a558 (&sb->s_type->i_mutex_key#16){++++}-{4:4}, at: iterate_dir+0x3b4/0x5f4 fs/readdir.c:101

other info that might help us debug this:
Possible unsafe locking scenario:

CPU0
----
lock(&sb->s_type->i_mutex_key#16);
lock(&sb->s_type->i_mutex_key#16);

*** DEADLOCK ***

May be due to missing lock nesting notation

3 locks held by syz-executor772/6443:
#0: ffff80008fc57208 (system_transition_mutex){+.+.}-{4:4}, at: lock_system_sleep+0x68/0xc0 kernel/power/main.c:56
#1: ffff80008fc75d70 ((pm_chain_head).rwsem){++++}-{4:4}, at: blocking_notifier_call_chain+0x58/0xa0 kernel/notifier.c:379
#2: ffff0000c6c7a558 (&sb->s_type->i_mutex_key#16){++++}-{4:4}, at: iterate_dir+0x3b4/0x5f4 fs/readdir.c:101

stack backtrace:
CPU: 0 UID: 0 PID: 6443 Comm: syz-executor772 Not tainted 6.14.0-rc4-syzkaller-ge056da87c780 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 12/27/2024
Call trace:
show_stack+0x2c/0x3c arch/arm64/kernel/stacktrace.c:466 (C)
__dump_stack lib/dump_stack.c:94 [inline]
dump_stack_lvl+0xe4/0x150 lib/dump_stack.c:120
dump_stack+0x1c/0x28 lib/dump_stack.c:129
print_deadlock_bug+0x4e8/0x668 kernel/locking/lockdep.c:3039
check_deadlock kernel/locking/lockdep.c:3091 [inline]
validate_chain kernel/locking/lockdep.c:3893 [inline]
__lock_acquire+0x6240/0x7904 kernel/locking/lockdep.c:5228
lock_acquire+0x23c/0x724 kernel/locking/lockdep.c:5851
down_write+0x50/0xc0 kernel/locking/rwsem.c:1577
inode_lock include/linux/fs.h:877 [inline]
efivarfs_actor+0x1b8/0x2b8 fs/efivarfs/super.c:422
dir_emit include/linux/fs.h:3849 [inline]
dcache_readdir+0x2dc/0x4e8 fs/libfs.c:209
iterate_dir+0x46c/0x5f4 fs/readdir.c:108
efivarfs_pm_notify+0x2f4/0x350 fs/efivarfs/super.c:517
notifier_call_chain+0x1c4/0x550 kernel/notifier.c:85
blocking_notifier_call_chain+0x70/0xa0 kernel/notifier.c:380
pm_notifier_call_chain+0x2c/0x3c kernel/power/main.c:109
snapshot_release+0x128/0x1b8 kernel/power/user.c:125
__fput+0x340/0x760 fs/file_table.c:464
____fput+0x20/0x30 fs/file_table.c:492
task_work_run+0x230/0x2e0 kernel/task_work.c:227
resume_user_mode_work include/linux/resume_user_mode.h:50 [inline]
do_notify_resume+0x178/0x1f4 arch/arm64/kernel/entry-common.c:151
exit_to_user_mode_prepare arch/arm64/kernel/entry-common.c:169 [inline]
exit_to_user_mode arch/arm64/kernel/entry-common.c:178 [inline]
el0_svc+0xac/0x168 arch/arm64/kernel/entry-common.c:745
el0t_64_sync_handler+0x84/0x108 arch/arm64/kernel/entry-common.c:762
el0t_64_sync+0x198/0x19c arch/arm64/kernel/entry.S:600


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzk...@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup

[Hillf Danton]

On Sat, 08 Mar 2025 18:52:38 -0800

> syzbot found the following issue on:
>
> HEAD commit: e056da87c780 Merge remote-tracking branch 'will/for-next/p..
> git tree: git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-kernelci

> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=13b97c64580000

#syz test upstream master

--- x/fs/efivarfs/super.c
+++ y/fs/efivarfs/super.c
@@ -421,7 +421,7 @@ static bool efivarfs_actor(struct dir_co
if (err)
size = 0;

- inode_lock(inode);
+ inode_lock_nested(inode, I_MUTEX_CHILD);
i_size_write(inode, size);
inode_unlock(inode);

--

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-by: syzbot+019072...@syzkaller.appspotmail.com
Tested-by: syzbot+019072...@syzkaller.appspotmail.com

Tested on:

commit: 1110ce6a Merge tag 'mm-hotfixes-stable-2025-03-08-16-2..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=14541fa0580000
kernel config: https://syzkaller.appspot.com/x/.config?x=afb3000d0159783f

dashboard link: https://syzkaller.appspot.com/bug?extid=019072ad24ab1d948228
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
userspace arch: arm64

patch: https://syzkaller.appspot.com/x/patch.diff?x=103ad878580000

Note: testing is done by a robot and is best-effort only.

[Ard Biesheuvel]

(cc James)

[Ard Biesheuvel]

On Mon, 10 Mar 2025 at 17:50, James Bottomley
<James.B...@hansenpartnership.com> wrote:

> > ffff0000c6826558 (&sb->s_type->i_mutex_key#16):4}, at:

> > iterate_dir+0x3b4/0x5f4 fs/readdir.c:101
> >
> > other info that might help us debug this:
> > Possible unsafe locking scenario:
> >
> > CPU0
> > ----
> > lock(&sb->s_type->i_mutex_key#16);
> > lock(&sb->s_type->i_mutex_key#16);
> >
> > *** DEADLOCK ***
>

> I can't figure out how you got here. the shared lock in readdir.c is
> on the directory and the inode_lock in the actor is on the files within
> the directory. The only way to get those to be the same is if the
> actor gets called on the '.' element, which efivarfs_pm_notify is
> supposed to skip with the
>
> file->f_pos = 2; /* skip . and .. */
>
> line. Emitting the '.' and '..' in positions 0 and 1 is hard coded
> into libfs.c:dcache_readdir() unless you're also applying a patch that
> alters that behaviour?
>

The repro log also has

program crashed: BUG: unable to handle kernel paging request in
efivarfs_pm_notify

preceding the other log output regarding the locks, so the deadlock
might be a symptom of another problem.

[Ard Biesheuvel]

And one of the other logs has

[ 47.650966][ T6617] syz.2.9/6617 is trying to acquire lock:
[ 47.652339][ T6617] ffff0000d69f6558
(&sb->s_type->i_mutex_key#25){++++}-{4:4}, at:
efivarfs_actor+0x1b8/0x2b8
[ 47.654943][ T6617]
[ 47.654943][ T6617] but task is already holding lock:
[ 47.656931][ T6617] ffff0000f5b84558
(&sb->s_type->i_mutex_key#25){++++}-{4:4}, at: iterate_dir+0x3b4/0x5f4

where the locks have the same name but the address is different.

So there is something dodgy going on here, and I'm inclined to just ignore it.

[James Bottomley]

On Sat, 2025-03-08 at 18:52 -0800, syzbot wrote:

> Hello,
>
> syzbot found the following issue on:
>

> HEAD commit:    e056da87c780 Merge remote-tracking branch 'will/for-

> next/p..
> git tree:      
> git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-
> kernelci

> ffff0000c6826558 (&sb->s_type->i_mutex_key#16):4}, at:

> iterate_dir+0x3b4/0x5f4 fs/readdir.c:101
>
> other info that might help us debug this:
>  Possible unsafe locking scenario:
>
>        CPU0
>        ----
>   lock(&sb->s_type->i_mutex_key#16);
>   lock(&sb->s_type->i_mutex_key#16);
>
>  *** DEADLOCK ***

I can't figure out how you got here. the shared lock in readdir.c is
on the directory and the inode_lock in the actor is on the files within
the directory. The only way to get those to be the same is if the
actor gets called on the '.' element, which efivarfs_pm_notify is
supposed to skip with the

file->f_pos = 2; /* skip . and .. */

line. Emitting the '.' and '..' in positions 0 and 1 is hard coded
into libfs.c:dcache_readdir() unless you're also applying a patch that
alters that behaviour?

Regards,

James

[Al Viro]

On Mon, Mar 10, 2025 at 07:24:43PM +0100, Ard Biesheuvel wrote:

> And one of the other logs has
>
> [ 47.650966][ T6617] syz.2.9/6617 is trying to acquire lock:
> [ 47.652339][ T6617] ffff0000d69f6558
> (&sb->s_type->i_mutex_key#25){++++}-{4:4}, at:
> efivarfs_actor+0x1b8/0x2b8
> [ 47.654943][ T6617]
> [ 47.654943][ T6617] but task is already holding lock:
> [ 47.656931][ T6617] ffff0000f5b84558
> (&sb->s_type->i_mutex_key#25){++++}-{4:4}, at: iterate_dir+0x3b4/0x5f4
>
> where the locks have the same name but the address is different.
>
> So there is something dodgy going on here, and I'm inclined to just ignore it.

That one is a false positive - iterate_dir() locks parent, then
callback locks child, but without bothering to tell lockdep about
that. IOW, in actor you should use inode_lock_nested(inode, INODE_CHILD);
instead of inode_lock(inode).

[Al Viro]

On Mon, Mar 10, 2025 at 07:21:53PM +0100, Ard Biesheuvel wrote:

> The repro log also has
>
> program crashed: BUG: unable to handle kernel paging request in
> efivarfs_pm_notify
>
> preceding the other log output regarding the locks, so the deadlock
> might be a symptom of another problem.

This:
struct path path = { .mnt = NULL, .dentry = sfi->sb->s_root, };

_What_ .mnt = NULL? That's already a bug. There is no such thing
as mountless open file; how would the kernel know not to shut the
damn thing down right under you?