Patch "fcntl: fix potential deadlocks for &fown_struct.lock" has been added to the 5.14-stable tree

[Sasha Levin <sashal@xxxxxxxxxx>]

This is a note to let you know that I've just added the patch titled

    fcntl: fix potential deadlocks for &fown_struct.lock

to the 5.14-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     fcntl-fix-potential-deadlocks-for-fown_struct.lock.patch
and it can be found in the queue-5.14 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.



commit eba8d8a05973ae91ae47204eef6168c101eddb80
Author: Desmond Cheong Zhi Xi <desmondcheongzx@xxxxxxxxx>
Date:   Fri Jul 2 17:18:30 2021 +0800

    fcntl: fix potential deadlocks for &fown_struct.lock
    
    [ Upstream commit f671a691e299f58835d4660d642582bf0e8f6fda ]
    
    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@xxxxxxxxxxxxxxxxxxxxxxxxx
    Signed-off-by: Desmond Cheong Zhi Xi <desmondcheongzx@xxxxxxxxx>
    Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
    Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>

diff --git a/fs/fcntl.c b/fs/fcntl.c
index f946bec8f1f1..932ec1e9f5bf 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -150,7 +150,8 @@ void f_delown(struct file *filp)
 pid_t f_getown(struct file *filp)
 {
 	pid_t pid = 0;
-	read_lock(&filp->f_owner.lock);
+
+	read_lock_irq(&filp->f_owner.lock);
 	rcu_read_lock();
 	if (pid_task(filp->f_owner.pid, filp->f_owner.pid_type)) {
 		pid = pid_vnr(filp->f_owner.pid);
@@ -158,7 +159,7 @@ pid_t f_getown(struct file *filp)
 			pid = -pid;
 	}
 	rcu_read_unlock();
-	read_unlock(&filp->f_owner.lock);
+	read_unlock_irq(&filp->f_owner.lock);
 	return pid;
 }
 
@@ -208,7 +209,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);
 	rcu_read_lock();
 	if (pid_task(filp->f_owner.pid, filp->f_owner.pid_type))
 		owner.pid = pid_vnr(filp->f_owner.pid);
@@ -231,7 +232,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));
@@ -249,10 +250,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]);