tty: pty: use guard()s

Use guards in the pty code. This improves readability, makes error
handling easier, and marks locked portions of code explicit. All that
while being sure the lock is unlocked.

pty_set_pktmode() is handled specially -- the conditions are inverted
and return called if conditions unmet. This avoid double nested 'if's.
The variable is renamed to want_pktmode so it is not confused with the
current state of pktmode.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Link: https://patch.msgid.link/20251119100140.830761-2-jirislaby@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index 8bb1a01fef2a1e6a011b5d3b5c03ad24ade2825c..76188b8f3ba3ab217a7288fbf80f85d3b55baf8a 100644 (file)
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -57,9 +57,8 @@ static void pty_close(struct tty_struct *tty, struct file *filp)
        set_bit(TTY_IO_ERROR, &tty->flags);
        wake_up_interruptible(&tty->read_wait);
        wake_up_interruptible(&tty->write_wait);
-       spin_lock_irq(&tty->ctrl.lock);
-       tty->ctrl.packet = false;
-       spin_unlock_irq(&tty->ctrl.lock);
+       scoped_guard(spinlock_irq, &tty->ctrl.lock)
+               tty->ctrl.packet = false;
        /* Review - krefs on tty_link ?? */
        if (!tty->link)
                return;
@@ -70,10 +69,9 @@ static void pty_close(struct tty_struct *tty, struct file *filp)
                set_bit(TTY_OTHER_CLOSED, &tty->flags);
 #ifdef CONFIG_UNIX98_PTYS
                if (tty->driver == ptm_driver) {
-                       mutex_lock(&devpts_mutex);
+                       guard(mutex)(&devpts_mutex);
                        if (tty->link->driver_data)
                                devpts_pty_kill(tty->link->driver_data);
-                       mutex_unlock(&devpts_mutex);
                }
 #endif
                tty_vhangup(tty->link);
@@ -157,21 +155,23 @@ static int pty_get_lock(struct tty_struct *tty, int __user *arg)
 /* Set the packet mode on a pty */
 static int pty_set_pktmode(struct tty_struct *tty, int __user *arg)
 {
-       int pktmode;
+       int want_pktmode;
 
-       if (get_user(pktmode, arg))
+       if (get_user(want_pktmode, arg))
                return -EFAULT;
 
-       spin_lock_irq(&tty->ctrl.lock);
-       if (pktmode) {
-               if (!tty->ctrl.packet) {
-                       tty->link->ctrl.pktstatus = 0;
-                       smp_mb();
-                       tty->ctrl.packet = true;
-               }
-       } else
+       guard(spinlock_irq)(&tty->ctrl.lock);
+       if (!want_pktmode) {
                tty->ctrl.packet = false;
-       spin_unlock_irq(&tty->ctrl.lock);
+               return 0;
+       }
+
+       if (tty->ctrl.packet)
+               return 0;
+
+       tty->link->ctrl.pktstatus = 0;
+       smp_mb();
+       tty->ctrl.packet = true;
 
        return 0;
 }
@@ -210,10 +210,9 @@ static void pty_flush_buffer(struct tty_struct *tty)
 
        tty_buffer_flush(to, NULL);
        if (to->ctrl.packet) {
-               spin_lock_irq(&tty->ctrl.lock);
+               guard(spinlock_irq)(&tty->ctrl.lock);
                tty->ctrl.pktstatus |= TIOCPKT_FLUSHWRITE;
                wake_up_interruptible(&to->read_wait);
-               spin_unlock_irq(&tty->ctrl.lock);
        }
 }
 
@@ -252,17 +251,17 @@ static void pty_set_termios(struct tty_struct *tty,
                                STOP_CHAR(tty) == '\023' &&
                                START_CHAR(tty) == '\021');
                if ((old_flow != new_flow) || extproc) {
-                       spin_lock_irq(&tty->ctrl.lock);
-                       if (old_flow != new_flow) {
-                               tty->ctrl.pktstatus &= ~(TIOCPKT_DOSTOP | TIOCPKT_NOSTOP);
-                               if (new_flow)
-                                       tty->ctrl.pktstatus |= TIOCPKT_DOSTOP;
-                               else
-                                       tty->ctrl.pktstatus |= TIOCPKT_NOSTOP;
+                       scoped_guard(spinlock_irq, &tty->ctrl.lock) {
+                               if (old_flow != new_flow) {
+                                       tty->ctrl.pktstatus &= ~(TIOCPKT_DOSTOP | TIOCPKT_NOSTOP);
+                                       if (new_flow)
+                                               tty->ctrl.pktstatus |= TIOCPKT_DOSTOP;
+                                       else
+                                               tty->ctrl.pktstatus |= TIOCPKT_NOSTOP;
+                               }
+                               if (extproc)
+                                       tty->ctrl.pktstatus |= TIOCPKT_IOCTL;
                        }
-                       if (extproc)
-                               tty->ctrl.pktstatus |= TIOCPKT_IOCTL;
-                       spin_unlock_irq(&tty->ctrl.lock);
                        wake_up_interruptible(&tty->link->read_wait);
                }
        }
@@ -286,9 +285,9 @@ static int pty_resize(struct tty_struct *tty,  struct winsize *ws)
        struct tty_struct *pty = tty->link;
 
        /* For a PTY we need to lock the tty side */
-       mutex_lock(&tty->winsize_mutex);
+       guard(mutex)(&tty->winsize_mutex);
        if (!memcmp(ws, &tty->winsize, sizeof(*ws)))
-               goto done;
+               return 0;
 
        /* Signal the foreground process group of both ptys */
        pgrp = tty_get_pgrp(tty);
@@ -304,8 +303,7 @@ static int pty_resize(struct tty_struct *tty,  struct winsize *ws)
 
        tty->winsize = *ws;
        pty->winsize = *ws;     /* Never used so will go away soon */
-done:
-       mutex_unlock(&tty->winsize_mutex);
+
        return 0;
 }
 
@@ -321,28 +319,26 @@ done:
  */
 static void pty_start(struct tty_struct *tty)
 {
-       unsigned long flags;
+       if (!tty->link || !tty->link->ctrl.packet)
+               return;
 
-       if (tty->link && tty->link->ctrl.packet) {
-               spin_lock_irqsave(&tty->ctrl.lock, flags);
+       scoped_guard(spinlock_irqsave, &tty->ctrl.lock) {
                tty->ctrl.pktstatus &= ~TIOCPKT_STOP;
                tty->ctrl.pktstatus |= TIOCPKT_START;
-               spin_unlock_irqrestore(&tty->ctrl.lock, flags);
-               wake_up_interruptible_poll(&tty->link->read_wait, EPOLLIN);
        }
+       wake_up_interruptible_poll(&tty->link->read_wait, EPOLLIN);
 }
 
 static void pty_stop(struct tty_struct *tty)
 {
-       unsigned long flags;
+       if (!tty->link || !tty->link->ctrl.packet)
+               return;
 
-       if (tty->link && tty->link->ctrl.packet) {
-               spin_lock_irqsave(&tty->ctrl.lock, flags);
+       scoped_guard(spinlock_irqsave, &tty->ctrl.lock) {
                tty->ctrl.pktstatus &= ~TIOCPKT_START;
                tty->ctrl.pktstatus |= TIOCPKT_STOP;
-               spin_unlock_irqrestore(&tty->ctrl.lock, flags);
-               wake_up_interruptible_poll(&tty->link->read_wait, EPOLLIN);
        }
+       wake_up_interruptible_poll(&tty->link->read_wait, EPOLLIN);
 }
 
 /**
@@ -705,15 +701,9 @@ static struct tty_struct *ptm_unix98_lookup(struct tty_driver *driver,
 static struct tty_struct *pts_unix98_lookup(struct tty_driver *driver,
                struct file *file, int idx)
 {
-       struct tty_struct *tty;
-
-       mutex_lock(&devpts_mutex);
-       tty = devpts_get_priv(file->f_path.dentry);
-       mutex_unlock(&devpts_mutex);
+       guard(mutex)(&devpts_mutex);
        /* Master must be open before slave */
-       if (!tty)
-               return ERR_PTR(-EIO);
-       return tty;
+       return devpts_get_priv(file->f_path.dentry) ? : ERR_PTR(-EIO);
 }
 
 static int pty_unix98_install(struct tty_driver *driver, struct tty_struct *tty)
@@ -811,20 +801,17 @@ static int ptmx_open(struct inode *inode, struct file *filp)
        }
 
        /* find a device that is not in use. */
-       mutex_lock(&devpts_mutex);
-       index = devpts_new_index(fsi);
-       mutex_unlock(&devpts_mutex);
+       scoped_guard(mutex, &devpts_mutex)
+               index = devpts_new_index(fsi);
 
        retval = index;
        if (index < 0)
                goto out_put_fsi;
 
 
-       mutex_lock(&tty_mutex);
-       tty = tty_init_dev(ptm_driver, index);
-       /* The tty returned here is locked so we can safely
-          drop the mutex */
-       mutex_unlock(&tty_mutex);
+       /* The tty returned here is locked so we can safely drop the mutex */
+       scoped_guard(mutex, &tty_mutex)
+               tty = tty_init_dev(ptm_driver, index);
 
        retval = PTR_ERR(tty);
        if (IS_ERR(tty))