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

diff --git a/bin/ed/ed.h b/bin/ed/ed.h
index 12d7e3d..49de288 100644 (file)
--- 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 fe0b939..379d466 100644 (file)
--- 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 <regex.h>
 #include <signal.h>
+#include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -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 4a690b0..2776a30 100644 (file)
--- 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 <sys/ioctl.h>
 #include <sys/stat.h>
 #include <sys/wait.h>
+#include <unistd.h>
 
 #include <ctype.h>
 #include <err.h>
@@ -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);
 }