From c5cd6b805addecb1d1fc5b2e95a185c3a25c01da Mon Sep 17 00:00:00 2001 From: Ahmad Fatoum Date: Sat, 4 May 2019 14:03:37 +0200 Subject: [PATCH 1/5] serial: atomically create lock files Previously, a different process could create the lock between both calls to open(2) with the result that two processes access the same serial port. Fix this by specifying only opening the lock file with O_CREAT | O_EXCL. Signed-off-by: Ahmad Fatoum --- serial.c | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/serial.c b/serial.c index 0778fe1..37a606c 100644 --- a/serial.c +++ b/serial.c @@ -22,6 +22,7 @@ #include #include +#include #include #include "microcom.h" @@ -177,26 +178,28 @@ struct ios_ops * serial_init(char *device) exit(1); } - fd = open(lockfile, O_RDONLY); - if (fd >= 0 && !opt_force) { - close(fd); - main_usage(3, "lockfile for port exists", device); - } +relock: + fd = open(lockfile, O_RDWR | O_CREAT | O_EXCL, 0444); + if (fd < 0) { + if (errno == EEXIST) { + if (opt_force) { + printf("lockfile for port exists, ignoring\n"); + serial_unlock(); + goto relock; + } + + main_usage(3, "lockfile for port exists", device); + } + + if (opt_force) { + printf("cannot create lockfile. ignoring\n"); + lockfile = NULL; + goto force; + } - if (fd >= 0 && opt_force) { - close(fd); - printf("lockfile for port exists, ignoring\n"); - serial_unlock(); + main_usage(3, "cannot create lockfile", device); } - fd = open(lockfile, O_RDWR | O_CREAT, 0444); - if (fd < 0 && opt_force) { - printf("cannot create lockfile. ignoring\n"); - lockfile = NULL; - goto force; - } - if (fd < 0) - main_usage(3, "cannot create lockfile", device); /* Kermit wants binary pid */ pid = getpid(); write(fd, &pid, sizeof(long)); From eb89a278cab7ac28a6192d458a21b47364937629 Mon Sep 17 00:00:00 2001 From: Ahmad Fatoum Date: Sat, 4 May 2019 14:03:37 +0200 Subject: [PATCH 2/5] serial: create lock files in HDB UUCP format While at it remove the now outdated comment about kermit compatibility. It no longer applies to ckermit [1]. [1]: https://github.com/pengutronix/microcom/pull/9#discussion_r217924856 Signed-off-by: Ahmad Fatoum --- serial.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/serial.c b/serial.c index 37a606c..97b5448 100644 --- a/serial.c +++ b/serial.c @@ -200,9 +200,8 @@ struct ios_ops * serial_init(char *device) main_usage(3, "cannot create lockfile", device); } - /* Kermit wants binary pid */ pid = getpid(); - write(fd, &pid, sizeof(long)); + dprintf(fd, "%10ld\n", (long)pid); close(fd); force: /* open the device */ From 6f15b324fbf17cd7d387654a479bf37226558137 Mon Sep 17 00:00:00 2001 From: Ahmad Fatoum Date: Sat, 4 May 2019 14:03:37 +0200 Subject: [PATCH 3/5] serial: delete stale lock files If microcom crashes, e.g. due to the buffer underflow described in #10, it may leave a stale lock file behind. Teach microcom detection of these stale lock files. Signed-off-by: Ahmad Fatoum --- serial.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/serial.c b/serial.c index 97b5448..3fb9a9b 100644 --- a/serial.c +++ b/serial.c @@ -182,12 +182,34 @@ struct ios_ops * serial_init(char *device) fd = open(lockfile, O_RDWR | O_CREAT | O_EXCL, 0444); if (fd < 0) { if (errno == EEXIST) { + char pidbuf[12]; + ssize_t nbytes = 0; if (opt_force) { printf("lockfile for port exists, ignoring\n"); serial_unlock(); goto relock; } + fd = open(lockfile, O_RDONLY); + if (fd < 0) + main_usage(3, "lockfile for port can't be opened", device); + + do { + ret = read(fd, &pidbuf[nbytes], sizeof(pidbuf) - nbytes - 1); + nbytes += ret; + } while (ret > 0 && nbytes < sizeof (pidbuf) - 1); + + if (ret >= 0) { + pidbuf[nbytes] = '\0'; + ret = sscanf(pidbuf, "%10ld\n", &pid); + + if (ret == 1 && kill(pid, 0) < 0 && errno == ESRCH) { + printf("lockfile contains stale pid, ignoring\n"); + serial_unlock(); + goto relock; + } + } + main_usage(3, "lockfile for port exists", device); } From 082d2ff6036fa0538775134a5326bed64e8520e5 Mon Sep 17 00:00:00 2001 From: Ahmad Fatoum Date: Fri, 23 Aug 2019 22:44:22 +0200 Subject: [PATCH 4/5] Set O_CLOEXEC on newly opened file descriptors In preparation for piping the log into a file, mark existing file descriptors so they are closed for exec'ing child processes. Signed-off-by: Ahmad Fatoum --- commands_fsl_imx.c | 2 +- mux.c | 2 +- parser.c | 2 +- serial.c | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/commands_fsl_imx.c b/commands_fsl_imx.c index 3d64601..ec67369 100644 --- a/commands_fsl_imx.c +++ b/commands_fsl_imx.c @@ -384,7 +384,7 @@ static int upload_file(uint32_t address, char *name, unsigned char type) }; struct stat stat; - upfd = open(name, O_RDONLY); + upfd = open(name, O_RDONLY | O_CLOEXEC); if (upfd < 0) { perror("open"); return 1; diff --git a/mux.c b/mux.c index 5dab65d..581cce8 100644 --- a/mux.c +++ b/mux.c @@ -378,7 +378,7 @@ int logfile_open(const char *path) { int fd; - fd = open(path, O_CREAT | O_TRUNC | O_WRONLY, 0644); + fd = open(path, O_CREAT | O_TRUNC | O_WRONLY | O_CLOEXEC, 0644); if (fd < 0) { fprintf(stderr, "Cannot open logfile '%s': %s\n", path, strerror(errno)); return fd; diff --git a/parser.c b/parser.c index a9c4b2e..4dd8ebc 100644 --- a/parser.c +++ b/parser.c @@ -189,7 +189,7 @@ int do_commandline(void) int do_script(char *script) { - int fd = open(script, O_RDONLY); + int fd = open(script, O_RDONLY | O_CLOEXEC); int stdin = dup(1); int ret; diff --git a/serial.c b/serial.c index 3fb9a9b..c750749 100644 --- a/serial.c +++ b/serial.c @@ -179,7 +179,7 @@ struct ios_ops * serial_init(char *device) } relock: - fd = open(lockfile, O_RDWR | O_CREAT | O_EXCL, 0444); + fd = open(lockfile, O_RDWR | O_CREAT | O_EXCL | O_CLOEXEC, 0444); if (fd < 0) { if (errno == EEXIST) { char pidbuf[12]; @@ -227,7 +227,7 @@ struct ios_ops * serial_init(char *device) close(fd); force: /* open the device */ - fd = open(device, O_RDWR | O_NONBLOCK); + fd = open(device, O_RDWR | O_NONBLOCK | O_CLOEXEC); ops->fd = fd; if (fd < 0) { From e5f482b9e685e4701c003f3c47159baa59b560ba Mon Sep 17 00:00:00 2001 From: Ahmad Fatoum Date: Fri, 23 Aug 2019 22:00:56 +0200 Subject: [PATCH 5/5] commands: allow specifying a process to log to While microcom's stdout can be piped to another processes, it makes input somewhat of a hassle. Address this by teaching the --logfile option and the log command that a filename starting with a pipe character refers to an executable to start and pipe the input into. The child process shares stderr, but has stdout closed. Signed-off-by: Ahmad Fatoum --- commands.c | 4 +-- microcom.1 | 7 +++++ microcom.c | 11 ++++++-- microcom.h | 8 ++++-- mux.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++--- parser.c | 7 +++-- 6 files changed, 99 insertions(+), 13 deletions(-) diff --git a/commands.c b/commands.c index 6d6c0e9..eab57d0 100644 --- a/commands.c +++ b/commands.c @@ -184,7 +184,7 @@ static int cmd_log(int argc, char *argv[]) if (argc < 2) return MICROCOM_CMD_USAGE; - ret = logfile_open(argv[1]); + ret = log_open(argv); return ret; } @@ -239,7 +239,7 @@ static struct cmd cmds[] = { }, { .name = "log", .fn = cmd_log, - .info = "log to file", + .info = "log to file or |executable", .help = "log ", }, { .name = "#", diff --git a/microcom.1 b/microcom.1 index 04eac82..9dd68b8 100644 --- a/microcom.1 +++ b/microcom.1 @@ -14,6 +14,8 @@ microcom \- A minimalistic terminal program .IR host : port ] .RB [\| \-s .IR speed \|] +.RB [ -l +.IR logfile ] .br .B microcom -c .IB interface : rx_id : tx_id @@ -65,6 +67,11 @@ work in telnet (rfc2217) mode. .BI \-c\ interface\fB:\fIrx_id\fB:\fItx_id\fR,\ \fI \-\-can= interface\fB:\fIrx_id\fB:\fItx_id work in CAN mode (default: \fBcan0:200:200\fR) .TP +.BI \-l\ logfile \fR,\ \fB\-\-log= logfile +log output to \fIlogfile\fR. If \fIlogfile\fR starts with a pipe `\fB|\fR' character, +it's instead piped to an executable with that name in \fBPATH\fR. The executable receives +the microcom output over \fBstdin\fR with \fBstdout\fR closed and \fBstderr\fR connected to microcom's \fBstderr\fR. +.TP .BR -h ", " \-\-help Show help. diff --git a/microcom.c b/microcom.c index 1218ec4..c267f00 100644 --- a/microcom.c +++ b/microcom.c @@ -182,7 +182,7 @@ void main_usage(int exitcode, char *str, char *dev) " default: (%s:%x:%x)\n" " -f, --force ignore existing lock file\n" " -d, --debug output debugging info\n" - " -l, --logfile= log output to \n" + " -l, --logfile= log output to or <|executable>\n" " -o, --listenonly Do not modify local terminal, do not send input\n" " from stdin\n" " -a, --answerback= specify the answerback string sent as response to\n" @@ -297,7 +297,12 @@ int main(int argc, char *argv[]) exit(1); if (logfile) { - ret = logfile_open(logfile); + char *args[MAXARGS + 1]; + ret = parse_line(logfile, NULL, args); + if (ret < 0) + exit(1); + + ret = log_open(args); if (ret < 0) exit(1); } @@ -324,6 +329,8 @@ int main(int argc, char *argv[]) sigaction(SIGPIPE, &sact, NULL); sigaction(SIGTERM, &sact, NULL); sigaction(SIGQUIT, &sact, NULL); + sact.sa_handler = SIG_IGN; + sigaction(SIGCHLD, &sact, NULL); } /* run the main program loop */ diff --git a/microcom.h b/microcom.h index e8ed544..3625184 100644 --- a/microcom.h +++ b/microcom.h @@ -82,8 +82,12 @@ struct cmd { char *help; }; -int logfile_open(const char *path); -void logfile_close(void); +#define MAXARGS 64 + +int parse_line(char *_line, int *argc, char *argv[]); + +int log_open(char *argv[]); +void log_close(void); int register_command(struct cmd *cmd); #define MICROCOM_CMD_START 100 diff --git a/mux.c b/mux.c index 581cce8..f41e9fa 100644 --- a/mux.c +++ b/mux.c @@ -21,6 +21,7 @@ #include #include #include +#include #define BUFSIZE 1024 @@ -366,7 +367,7 @@ static void cook_buf(struct ios_ops *ios, unsigned char *buf, int num) } /* while - end of processing all the charactes in the buffer */ } -void logfile_close(void) +void log_close(void) { if (logfd >= 0) close(logfd); @@ -374,7 +375,54 @@ void logfile_close(void) logfd = -1; } -int logfile_open(const char *path) +static int logproc_open(const char *path, char *argv[]) +{ + int pipefd[2]; + int ret; + + ret = pipe(pipefd); + if (ret) { + fprintf(stderr, "Cannot open log pipe: %s\n", strerror(errno)); + return -1; + } + + ret = fcntl(pipefd[0], F_SETFD, FD_CLOEXEC); + if (!ret) + ret = fcntl(pipefd[1], F_SETFD, FD_CLOEXEC); + if (ret) { + fprintf(stderr, "Cannot set FD_CLOEXEC on log pipe: %s\n", + strerror(errno)); + return -1; + } + + /* flush now, so we don't clone unflushed output buffers */ + fflush(NULL); + + ret = fork(); + if (ret == 0) { + close(0); + close(1); + dup(pipefd[0]); + + execvp(path, argv); + fprintf(stderr, "Cannot exec log process (%s): %s\n", + path, strerror(errno)); + _Exit(1); + } else { + close(pipefd[0]); + + if (ret < 0) { + close(pipefd[1]); + fprintf(stderr, "Cannot fork log process: %s\n", + strerror(errno)); + return -1; + } + } + + return pipefd[1]; +} + +static int logfile_open(const char *path) { int fd; @@ -385,7 +433,28 @@ int logfile_open(const char *path) } if (logfd >= 0) - logfile_close(); + log_close(); + + logfd = fd; + + return 0; +} + +int log_open(char *argv[]) +{ + const char *path = argv[0]; + int fd; + + if (*path == '|') + fd = logproc_open(++path, argv); + else + fd = logfile_open(path); + + if (fd < 0) + return fd; + + if (logfd >= 0) + log_close(); logfd = fd; diff --git a/parser.c b/parser.c index 4dd8ebc..593d6d7 100644 --- a/parser.c +++ b/parser.c @@ -11,9 +11,7 @@ #include #include "microcom.h" -#define MAXARGS 64 - -static int parse_line(char *_line, int *argc, char *argv[]) +int parse_line(char *_line, int *argc, char *argv[]) { char *line = _line; int nargs = 0; @@ -64,7 +62,8 @@ static int parse_line(char *_line, int *argc, char *argv[]) printf("Too many args (max. %d)\n", MAXARGS); out: argv[nargs] = NULL; - *argc = nargs; + if (argc) + *argc = nargs; return line - _line + 1; }