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>

1 files changed, 45 insertions, 58 deletions

diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index 8bb1a01fef2a..76188b8f3ba3 100644
--- 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))