Re: [PATCH 0/2] SIGWINCH problem with terminal apps still alive

[Adam TlaÅka]

Welcome,

generally speaking terminal resize could be made from vc or other
terminal device module code, from terminal ioctl on tty master
or tty slave. As we read the code we will see that the problem is to
use a proper mutex. Also we should use a one variable in which we set
terminal sizes. So in case of tty master and slave we have tty and
real_tty structures. TIOCSWINSZ and TIOCGWINSZ ioctls could be
called on tty and real_tty at the same time. To avoid race condition
we should use a mutex. But this must be the same mutex in all cases
- tty or real_tty. For the best we should also use the same
winsize variable. Proposed previously code change only simplifies
the code and eliminates SIGWINCH signal race but an app could read
terminal sizes at any time so this patch not closes all race
possibilities. So I propose a patch in which we have some code
movements and always use real_tty->termios_mutex and also
real_tty->winsize. In no pty case tty == real_tty so it works properly
as before and in pty situaction we use the same mutex and variable so
it removes all race conditions according to access to winsize now.

Signed-off-by: Adam Tla/lka <atlka@xxxxxxxxx>

Regards

-- 
Adam TlaÅka       mailto:atlka@xxxxxxxxx    ^v^ ^v^ ^v^
System  & Network Administration Group       - - - ~~~~~~
Computer Center, GdaÅsk University of Technology, Poland
--- drivers/char/tty_io_orig.c	2008-10-10 05:37:30.000000000 +0200
+++ drivers/char/tty_io.c	2008-10-12 13:37:11.000000000 +0200
@@ -2524,27 +2524,26 @@ int tty_do_resize(struct tty_struct *tty
 
 	/* For a PTY we need to lock the tty side */
 	mutex_lock(&real_tty->termios_mutex);
-	if (!memcmp(ws, &tty->winsize, sizeof(*ws)))
-		goto done;
-	/* Get the PID values and reference them so we can
-	   avoid holding the tty ctrl lock while sending signals */
-	spin_lock_irqsave(&tty->ctrl_lock, flags);
-	pgrp = get_pid(tty->pgrp);
-	rpgrp = get_pid(real_tty->pgrp);
-	spin_unlock_irqrestore(&tty->ctrl_lock, flags);
+	flags = memcmp(ws, &real_tty->winsize, sizeof(*ws));
+	real_tty->winsize = *ws;
+	mutex_unlock(&real_tty->termios_mutex);
+	if (flags){
+		/* Get the PID values and reference them so we can
+		 avoid holding the tty ctrl lock while sending signals */
+		spin_lock_irqsave(&tty->ctrl_lock, flags);
+		pgrp = get_pid(tty->pgrp);
+		rpgrp = get_pid(real_tty->pgrp);
+		spin_unlock_irqrestore(&tty->ctrl_lock, flags);
 
-	if (pgrp)
-		kill_pgrp(pgrp, SIGWINCH, 1);
-	if (rpgrp != pgrp && rpgrp)
-		kill_pgrp(rpgrp, SIGWINCH, 1);
+		if (pgrp)
+			kill_pgrp(pgrp, SIGWINCH, 1);
+		if (rpgrp != pgrp && rpgrp)
+			kill_pgrp(rpgrp, SIGWINCH, 1);
 
-	put_pid(pgrp);
-	put_pid(rpgrp);
+		put_pid(pgrp);
+		put_pid(rpgrp);
+	}
 
-	tty->winsize = *ws;
-	real_tty->winsize = *ws;
-done:
-	mutex_unlock(&real_tty->termios_mutex);
 	return 0;
 }
 
@@ -2996,7 +2995,7 @@ long tty_ioctl(struct file *file, unsign
 	case TIOCSTI:
 		return tiocsti(tty, p);
 	case TIOCGWINSZ:
-		return tiocgwinsz(tty, p);
+		return tiocgwinsz(real_tty, p);
 	case TIOCSWINSZ:
 		return tiocswinsz(tty, real_tty, p);
 	case TIOCCONS: