From: Desmond Cheong Zhi Xi desmondcheongzx@gmail.com
mainline inclusion from mainline-v5.15-rc1 commit f671a691e299f58835d4660d642582bf0e8f6fda category: bugfix bugzilla: 185969 https://gitee.com/openeuler/kernel/issues/I4DDEL
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h...
--------------------------------
Syzbot reports a potential deadlock in do_fcntl:
======================================================== WARNING: possible irq lock inversion dependency detected 5.12.0-syzkaller #0 Not tainted -------------------------------------------------------- syz-executor132/8391 just changed the state of lock: ffff888015967bf8 (&f->f_owner.lock){.+..}-{2:2}, at: f_getown_ex fs/fcntl.c:211 [inline] ffff888015967bf8 (&f->f_owner.lock){.+..}-{2:2}, at: do_fcntl+0x8b4/0x1200 fs/fcntl.c:395 but this lock was taken by another, HARDIRQ-safe lock in the past: (&dev->event_lock){-...}-{2:2}
and interrupts could create inverse lock ordering between them.
other info that might help us debug this: Chain exists of: &dev->event_lock --> &new->fa_lock --> &f->f_owner.lock
Possible interrupt unsafe locking scenario:
CPU0 CPU1 ---- ---- lock(&f->f_owner.lock); local_irq_disable(); lock(&dev->event_lock); lock(&new->fa_lock); <Interrupt> lock(&dev->event_lock);
*** DEADLOCK ***
This happens because there is a lock hierarchy of &dev->event_lock --> &new->fa_lock --> &f->f_owner.lock from the following call chain:
input_inject_event(): spin_lock_irqsave(&dev->event_lock,...); input_handle_event(): input_pass_values(): input_to_handler(): evdev_events(): evdev_pass_values(): spin_lock(&client->buffer_lock); __pass_event(): kill_fasync(): kill_fasync_rcu(): read_lock(&fa->fa_lock); send_sigio(): read_lock_irqsave(&fown->lock,...);
However, since &dev->event_lock is HARDIRQ-safe, interrupts have to be disabled while grabbing &f->f_owner.lock, otherwise we invert the lock hierarchy.
Hence, we replace calls to read_lock/read_unlock on &f->f_owner.lock, with read_lock_irq/read_unlock_irq.
Reported-and-tested-by: syzbot+e6d5398a02c516ce5e70@syzkaller.appspotmail.com Signed-off-by: Desmond Cheong Zhi Xi desmondcheongzx@gmail.com Signed-off-by: Jeff Layton jlayton@kernel.org
Conflict: fs/fcntl.c Signed-off-by: Zhang Wensheng zhangwensheng5@huawei.com Signed-off-by: Zheng Zengkai zhengzengkai@huawei.com --- fs/fcntl.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/fs/fcntl.c b/fs/fcntl.c index 3eebcbead8bf..3e05c8d4661b 100644 --- a/fs/fcntl.c +++ b/fs/fcntl.c @@ -149,11 +149,11 @@ void f_delown(struct file *filp) pid_t f_getown(struct file *filp) { pid_t pid; - read_lock(&filp->f_owner.lock); + read_lock_irq(&filp->f_owner.lock); pid = pid_vnr(filp->f_owner.pid); if (filp->f_owner.pid_type == PIDTYPE_PGID) pid = -pid; - read_unlock(&filp->f_owner.lock); + read_unlock_irq(&filp->f_owner.lock); return pid; }
@@ -203,7 +203,7 @@ static int f_getown_ex(struct file *filp, unsigned long arg) struct f_owner_ex owner; int ret = 0;
- read_lock(&filp->f_owner.lock); + read_lock_irq(&filp->f_owner.lock); owner.pid = pid_vnr(filp->f_owner.pid); switch (filp->f_owner.pid_type) { case PIDTYPE_PID: @@ -223,7 +223,7 @@ static int f_getown_ex(struct file *filp, unsigned long arg) ret = -EINVAL; break; } - read_unlock(&filp->f_owner.lock); + read_unlock_irq(&filp->f_owner.lock);
if (!ret) { ret = copy_to_user(owner_p, &owner, sizeof(owner)); @@ -241,10 +241,10 @@ static int f_getowner_uids(struct file *filp, unsigned long arg) uid_t src[2]; int err;
- read_lock(&filp->f_owner.lock); + read_lock_irq(&filp->f_owner.lock); src[0] = from_kuid(user_ns, filp->f_owner.uid); src[1] = from_kuid(user_ns, filp->f_owner.euid); - read_unlock(&filp->f_owner.lock); + read_unlock_irq(&filp->f_owner.lock);
err = put_user(src[0], &dst[0]); err |= put_user(src[1], &dst[1]);