From 6e1b57ead90ec623e4375b6e87f83c64221556ec Mon Sep 17 00:00:00 2001 From: Ahmad Fatoum Date: Sun, 25 Aug 2019 19:48:46 +0200 Subject: [PATCH 1/3] microcom: initialize the struct sigaction before use sigaction(2) reads the sa_mask and sa_flags struct members as well, but we didn't initialize them so far. We probably didn't run into problems, because we allocate it when the stack is fresh and full of zeroes. Fix this by explicitly zeroing the struct and emptying the signal set. Signed-off-by: Ahmad Fatoum --- microcom.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/microcom.c b/microcom.c index 1218ec4..142772a 100644 --- a/microcom.c +++ b/microcom.c @@ -202,7 +202,7 @@ int listenonly = 0; int main(int argc, char *argv[]) { - struct sigaction sact; /* used to initialize the signal handler */ + struct sigaction sact = {0}; /* used to initialize the signal handler */ int opt, ret; char *hostport = NULL; int telnet = 0, can = 0; @@ -319,6 +319,7 @@ int main(int argc, char *argv[]) /* set the signal handler to restore the old * termios handler */ sact.sa_handler = µcom_exit; + sigemptyset(&sact.sa_mask); sigaction(SIGHUP, &sact, NULL); sigaction(SIGINT, &sact, NULL); sigaction(SIGPIPE, &sact, NULL); From 2898704863c9a1ea2fca302ca489f62a70b59aa8 Mon Sep 17 00:00:00 2001 From: Ahmad Fatoum Date: Sun, 25 Aug 2019 21:22:07 +0200 Subject: [PATCH 2/3] Don't call async-signal-unsafe free in signal handler We use malloc elsewhere in the code with signals unmasked, but use free in a signal handler. This could result in a dead lock when terminating microcom by signal. The duplication of exit(0) call is ok, as the call site in the signal handler will be replaced in a follow-up commit. Signed-off-by: Ahmad Fatoum --- can.c | 1 - commands.c | 2 ++ microcom.c | 3 ++- serial.c | 1 - telnet.c | 1 - 5 files changed, 4 insertions(+), 4 deletions(-) diff --git a/can.c b/can.c index 1de2a12..62fccf2 100644 --- a/can.c +++ b/can.c @@ -69,7 +69,6 @@ static void can_exit(struct ios_ops *ios) shutdown(ios->fd, SHUT_RDWR); close(ios->fd); pthread_join(can_thread, NULL); - free(ios); } static void *can_thread_fun(void *_data) diff --git a/commands.c b/commands.c index 6d6c0e9..6b62fa8 100644 --- a/commands.c +++ b/commands.c @@ -148,6 +148,8 @@ static int cmd_break(int argc, char *argv[]) static int cmd_quit(int argc, char *argv[]) { microcom_exit(0); + free(ios); + exit(0); return 0; } diff --git a/microcom.c b/microcom.c index 142772a..4dfb5a8 100644 --- a/microcom.c +++ b/microcom.c @@ -155,7 +155,8 @@ void microcom_exit(int signal) ios->exit(ios); tcsetattr(STDIN_FILENO, TCSANOW, &sots); - exit(0); + if (signal) + exit(0); } /******************************************************************** diff --git a/serial.c b/serial.c index 0778fe1..2bc39f2 100644 --- a/serial.c +++ b/serial.c @@ -136,7 +136,6 @@ static void serial_exit(struct ios_ops *ios) { tcsetattr(ios->fd, TCSANOW, &pots); close(ios->fd); - free(ios); serial_unlock(); } diff --git a/telnet.c b/telnet.c index fe30da7..f510888 100644 --- a/telnet.c +++ b/telnet.c @@ -76,7 +76,6 @@ static int telnet_send_break(struct ios_ops *ios) static void telnet_exit(struct ios_ops *ios) { close(ios->fd); - free(ios); } struct ios_ops *telnet_init(char *hostport) From 6b025f4caed0db5654d0e3c8c74b2fe94286ff03 Mon Sep 17 00:00:00 2001 From: Ahmad Fatoum Date: Sun, 25 Aug 2019 21:45:14 +0200 Subject: [PATCH 3/3] microcom: make microcom_exit async-signal-safer Both printf and exit aren't async signal safe, replace them with write and _Exit respectively. The access to ios and ios->exit and callees is still illegal, but at least we don't run risk reentering stdio and causing a deadlock anymore. As exit flushes stdio buffers as well, we now risk losing unflushed stdio output. This is acceptable as we aren't buffering the serial port output and the program is being terminated abnormally anyway. Signed-off-by: Ahmad Fatoum --- commands.c | 1 + microcom.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/commands.c b/commands.c index 6b62fa8..3fc6fa4 100644 --- a/commands.c +++ b/commands.c @@ -147,6 +147,7 @@ static int cmd_break(int argc, char *argv[]) static int cmd_quit(int argc, char *argv[]) { + fflush(NULL); microcom_exit(0); free(ios); exit(0); diff --git a/microcom.c b/microcom.c index 4dfb5a8..6376b07 100644 --- a/microcom.c +++ b/microcom.c @@ -150,13 +150,13 @@ int flag_to_baudrate(speed_t speed) void microcom_exit(int signal) { - printf("exiting\n"); + write(1, "exiting\n", 8); ios->exit(ios); tcsetattr(STDIN_FILENO, TCSANOW, &sots); if (signal) - exit(0); + _Exit(0); } /********************************************************************