From 2bde29b351c80904b1c32bd5386a056013b27f45 Mon Sep 17 00:00:00 2001 From: deraadt Date: Tue, 16 Jul 2024 05:01:10 +0000 Subject: [PATCH] Fix the SIGHUP signal race. ed's "event loop" operates a getchar(); check the hup flag before and after that call, when the buffer structures are stable for write_file() to work. Remove the hup handling from the SPL0() macro, because this is run in at least one place during structure instability. The SIGINT handler, which uses siglongjmp(), is also trusting the SPL1/SPL0 dance more than it should. ok millert --- bin/ed/ed.h | 6 ++---- bin/ed/io.c | 10 ++++++++-- bin/ed/main.c | 25 ++++++++----------------- 3 files changed, 18 insertions(+), 23 deletions(-) diff --git a/bin/ed/ed.h b/bin/ed/ed.h index 12d7e3d5455..49de28811be 100644 --- a/bin/ed/ed.h +++ b/bin/ed/ed.h @@ -1,4 +1,4 @@ -/* $OpenBSD: ed.h,v 1.22 2016/03/27 00:43:38 mmcc Exp $ */ +/* $OpenBSD: ed.h,v 1.23 2024/07/16 05:01:10 deraadt Exp $ */ /* $NetBSD: ed.h,v 1.23 1995/03/21 09:04:40 cgd Exp $ */ /* ed.h: type and constant definitions for the ed editor. */ @@ -88,8 +88,6 @@ typedef struct undo { #define SPL0() \ do { \ if (--mutex == 0) { \ - if (sighup) \ - handle_hup(SIGHUP); \ if (sigint) \ handle_int(SIGINT); \ } \ @@ -160,7 +158,7 @@ char *get_extended_line(int *, int); int get_line_node_addr(line_t *); char *get_sbuf_line(line_t *); int get_tty_line(void); -void handle_hup(int); +void handle_hup(void); void handle_int(int); int has_trailing_escape(char *, char *); void init_buffers(void); diff --git a/bin/ed/io.c b/bin/ed/io.c index fe0b9394d17..379d466df11 100644 --- a/bin/ed/io.c +++ b/bin/ed/io.c @@ -1,4 +1,4 @@ -/* $OpenBSD: io.c,v 1.25 2022/11/18 14:52:03 millert Exp $ */ +/* $OpenBSD: io.c,v 1.26 2024/07/16 05:01:10 deraadt Exp $ */ /* $NetBSD: io.c,v 1.2 1995/03/21 09:04:43 cgd Exp $ */ /* io.c: This file contains the i/o routines for the ed line editor */ @@ -30,6 +30,7 @@ #include #include +#include #include #include #include @@ -261,7 +262,9 @@ get_tty_line(void) int i = 0; int c; - for (;;) + for (;;) { + if (sighup) + handle_hup(); switch (c = getchar()) { default: oi = 0; @@ -274,6 +277,8 @@ get_tty_line(void) ibufp = ibuf; return i; case EOF: + if (sighup) + handle_hup(); if (ferror(stdin)) { perror("stdin"); seterrmsg("cannot read stdin"); @@ -291,6 +296,7 @@ get_tty_line(void) return i; } } + } } diff --git a/bin/ed/main.c b/bin/ed/main.c index 4a690b04de1..2776a30781f 100644 --- a/bin/ed/main.c +++ b/bin/ed/main.c @@ -1,4 +1,4 @@ -/* $OpenBSD: main.c,v 1.68 2022/11/18 14:52:03 millert Exp $ */ +/* $OpenBSD: main.c,v 1.69 2024/07/16 05:01:10 deraadt Exp $ */ /* $NetBSD: main.c,v 1.3 1995/03/21 09:04:44 cgd Exp $ */ /* main.c: This file contains the main control and user-interface routines @@ -44,6 +44,7 @@ #include #include #include +#include #include #include @@ -180,6 +181,7 @@ top: signal(SIGWINCH, handle_winch); } signal(SIGHUP, signal_hup); + siginterrupt(SIGHUP, 1); signal(SIGQUIT, SIG_IGN); signal(SIGINT, signal_int); if (sigsetjmp(env, 1)) { @@ -1327,45 +1329,34 @@ strip_escapes(char *s) void signal_hup(int signo) { - int save_errno = errno; - - if (mutex) - sighup = 1; - else - handle_hup(signo); - errno = save_errno; + sighup = 1; } void signal_int(int signo) { - int save_errno = errno; - if (mutex) sigint = 1; else - handle_int(signo); - errno = save_errno; + handle_int(signo); /* XXX quite unsafe */ } void -handle_hup(int signo) +handle_hup(void) { char hup[PATH_MAX]; - if (!sigactive) - quit(1); /* XXX signal race */ + signal(SIGHUP, SIG_IGN); sighup = 0; - /* XXX signal race */ if (addr_last && write_file("ed.hup", "w", 1, addr_last) < 0 && home != NULL && home[0] == '/') { if (strlcpy(hup, home, sizeof(hup)) < sizeof(hup) && strlcat(hup, "/ed.hup", sizeof(hup)) < sizeof(hup)) write_file(hup, "w", 1, addr_last); } - _exit(2); + exit(2); } -- 2.20.1