Re: [PATCH] n_tty_read: check for hanging tty while waiting for input

[Peter Hurley <peter@hurleysoftware.com>]

On 02/18/2015 09:58 AM, Aristeu Rozanski wrote:
> Hi Peter,
> On Tue, Feb 17, 2015 at 05:35:10PM -0500, Peter Hurley wrote:
>> I realize that. But hanging up the tty that is /dev/console only affects
>> open descriptors that are not /dev/console.
>>
>> So readers using the /dev/ttyS0 file descriptor will see a hungup fops,
>> but readers using /dev/console will not, and /dev/ttyS0 will _not_
>> be closed or released because of the still-open descriptor on /dev/console.
> 
> I see.
> 
>> Ok, so the process sleeping on /dev/console read() should have received
>> SIGHUP, which would wake the process and cause it to exit the
>> n_tty_read() loop, thus dropping the ldisc reference it holds.
>> Did it ignore the signal or perhaps the signal is masked?
> 
> Not masked on the test case (attached). Sent sighup manually and it did
> receive it.
> 

The child is not receiving SIGHUP because /dev/ttyS0 was not set as the
controlling terminal by ioctl(TIOCSCTTY), which is failing (probably
with errno == EPERM). You need to check the return value and errno.

To set the controlling tty, the calling process must be a session leader;
ie., have called setsid() before ioctl(TIOCSCTTY). Check the return value
for that too.

FWIW, the idiom for starting a session leader is for the parent to
fork a child and exit and for the child to become the session leader with
setsid() and establish its controlling tty either with ioctl(TIOCSCTTY)
or simply opening the first tty.

The reason for this idiom is that setsid() will fail for an existing
group leader (because otherwise a group leader could abandon existing
members of its process group, leaving them without a group leader in
a different session).

I highly recommend Ch 34 of Michael Kerrisk's book, "The Linux Programming
Interface", especially if this is not a toy project.

Regards,
Peter Hurley

> #include <stdio.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <termios.h>
> #include <errno.h>
> #include <signal.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <sys/wait.h>
> #include <fcntl.h>
> #include <sys/ioctl.h>
> 
> static char *default_console = "/dev/console";
> static char *default_tty = "/dev/ttyS0";
> struct data {
> 	char *console;
> 	char *tty;
> };
> 
> static void *reader(void *d)
> {
> 	struct data *data = (struct data *)d;
> 	struct termios old_termio;
> 	char buff[512];
> 	int fd = -1, rc;
> 
> 	while (1) {
> 		if (fd == -1) {
> 			fd = open(data->console, O_RDWR);
> 			if (fd < 0)
> 				exit(1);
> 			if (tcgetattr(fd, &old_termio) == -1)
> 				exit(1);
> 			old_termio.c_lflag = ICANON;
> 			if (tcsetattr(fd, 0, &old_termio) == -1)
> 				exit(1);
> 		}
> 		rc = read(fd, buff, sizeof(buff));
> 		if (rc < 0 && errno == EAGAIN)
> 			continue;
> 		close(fd);
> 		fd = -1;
> 	}
> }
> 
> void launch(void *(*fn)(void *), struct data *data)
> {
> 	if (fork() == 0)
> 		fn(data);
> }
> 
> int main(int argc, char *argv[])
> {
> 	struct data data;
> 	int fd;
> 
> 	fd = open("/dev/ttyS0", O_RDWR);
> 	close(0);
> 	close(1);
> 	close(2);
> 	ioctl(fd, TIOCSCTTY, 1);
> 
> 	data.console = default_console;
> 	data.tty = default_tty;
> 
> 	launch(reader, &data);
> 
> 	waitpid(-1, NULL, 0);
> 
> 	return 0;
> }
>