From 9c44822d56002b591c9947ff2a16660b2031d8ac Mon Sep 17 00:00:00 2001 From: Ingo Schwarze Date: Wed, 1 Sep 2021 14:28:16 +0000 Subject: As a first step towards safe signal handling, improve the h_int() and h_winch() signal handlers to make one single store to a sig_atomic_t variable. Note that the h_hup() and h_term() signal handlers are still unsafe after this commit because they also set the "killersig" (how fitting!) field in a global struct. Despite storing information in static global variables rather than in structs passed around as arguments, this patch does not cause a change in behaviour because there is always exactly one GS object, initialized using gs_init() called from the top of main(), and screen_init() stores a pointer to this one and only GS object in the .gp member of each and every SCR object. Talk about useless abstraction... Problem pointed out by deraadt@. Patch from Tim on tech@. OK deraadt@. --- usr.bin/vi/cl/cl.h | 13 +++++++------ usr.bin/vi/cl/cl_funcs.c | 4 ++-- usr.bin/vi/cl/cl_main.c | 24 +++++++++++++++--------- usr.bin/vi/cl/cl_read.c | 42 ++++++++++++++++++++---------------------- 4 files changed, 44 insertions(+), 39 deletions(-) (limited to 'usr.bin/vi') diff --git a/usr.bin/vi/cl/cl.h b/usr.bin/vi/cl/cl.h index 5e5cdcb614c..bfef0dc225a 100644 --- a/usr.bin/vi/cl/cl.h +++ b/usr.bin/vi/cl/cl.h @@ -1,4 +1,4 @@ -/* $OpenBSD: cl.h,v 1.10 2016/05/27 09:18:11 martijn Exp $ */ +/* $OpenBSD: cl.h,v 1.11 2021/09/01 14:28:15 schwarze Exp $ */ /*- * Copyright (c) 1993, 1994 @@ -11,6 +11,11 @@ * @(#)cl.h 10.19 (Berkeley) 9/24/96 */ +extern volatile sig_atomic_t cl_sighup; +extern volatile sig_atomic_t cl_sigint; +extern volatile sig_atomic_t cl_sigterm; +extern volatile sig_atomic_t cl_sigwinch; + typedef struct _cl_private { CHAR_T ibuf[256]; /* Input keys. */ @@ -45,11 +50,7 @@ typedef struct _cl_private { #define CL_RENAME_OK 0x0004 /* User wants the windows renamed. */ #define CL_SCR_EX_INIT 0x0008 /* Ex screen initialized. */ #define CL_SCR_VI_INIT 0x0010 /* Vi screen initialized. */ -#define CL_SIGHUP 0x0020 /* SIGHUP arrived. */ -#define CL_SIGINT 0x0040 /* SIGINT arrived. */ -#define CL_SIGTERM 0x0080 /* SIGTERM arrived. */ -#define CL_SIGWINCH 0x0100 /* SIGWINCH arrived. */ -#define CL_STDIN_TTY 0x0200 /* Talking to a terminal. */ +#define CL_STDIN_TTY 0x0020 /* Talking to a terminal. */ u_int32_t flags; } CL_PRIVATE; diff --git a/usr.bin/vi/cl/cl_funcs.c b/usr.bin/vi/cl/cl_funcs.c index 4daa75e4360..eeb6046358f 100644 --- a/usr.bin/vi/cl/cl_funcs.c +++ b/usr.bin/vi/cl/cl_funcs.c @@ -1,4 +1,4 @@ -/* $OpenBSD: cl_funcs.c,v 1.20 2016/05/27 09:18:11 martijn Exp $ */ +/* $OpenBSD: cl_funcs.c,v 1.21 2021/09/01 14:28:15 schwarze Exp $ */ /*- * Copyright (c) 1993, 1994 @@ -601,7 +601,7 @@ cl_suspend(SCR *sp, int *allowedp) if (cl_ssize(sp, 1, NULL, NULL, &changed)) return (1); if (changed) - F_SET(CLP(sp), CL_SIGWINCH); + cl_sigwinch = 1; return (0); } diff --git a/usr.bin/vi/cl/cl_main.c b/usr.bin/vi/cl/cl_main.c index 38a10209f9e..4cd624e72bf 100644 --- a/usr.bin/vi/cl/cl_main.c +++ b/usr.bin/vi/cl/cl_main.c @@ -1,4 +1,4 @@ -/* $OpenBSD: cl_main.c,v 1.33 2016/05/05 20:36:41 martijn Exp $ */ +/* $OpenBSD: cl_main.c,v 1.34 2021/09/01 14:28:15 schwarze Exp $ */ /*- * Copyright (c) 1993, 1994 @@ -33,6 +33,11 @@ GS *__global_list; /* GLOBAL: List of screens. */ +volatile sig_atomic_t cl_sighup; +volatile sig_atomic_t cl_sigint; +volatile sig_atomic_t cl_sigterm; +volatile sig_atomic_t cl_sigwinch; + static void cl_func_std(GS *); static CL_PRIVATE *cl_init(GS *); static GS *gs_init(void); @@ -217,16 +222,14 @@ h_hup(int signo) { GLOBAL_CLP; - F_SET(clp, CL_SIGHUP); + cl_sighup = 1; clp->killersig = SIGHUP; } static void h_int(int signo) { - GLOBAL_CLP; - - F_SET(clp, CL_SIGINT); + cl_sigint = 1; } static void @@ -234,16 +237,14 @@ h_term(int signo) { GLOBAL_CLP; - F_SET(clp, CL_SIGTERM); + cl_sigterm = 1; clp->killersig = SIGTERM; } static void h_winch(int signo) { - GLOBAL_CLP; - - F_SET(clp, CL_SIGWINCH); + cl_sigwinch = 1; } #undef GLOBAL_CLP @@ -260,6 +261,11 @@ sig_init(GS *gp, SCR *sp) clp = GCLP(gp); + cl_sighup = 0; + cl_sigint = 0; + cl_sigterm = 0; + cl_sigwinch = 0; + if (sp == NULL) { if (setsig(SIGHUP, &clp->oact[INDX_HUP], h_hup) || setsig(SIGINT, &clp->oact[INDX_INT], h_int) || diff --git a/usr.bin/vi/cl/cl_read.c b/usr.bin/vi/cl/cl_read.c index 3644684b65f..aa3e1ba0dd4 100644 --- a/usr.bin/vi/cl/cl_read.c +++ b/usr.bin/vi/cl/cl_read.c @@ -1,4 +1,4 @@ -/* $OpenBSD: cl_read.c,v 1.21 2016/05/27 09:18:11 martijn Exp $ */ +/* $OpenBSD: cl_read.c,v 1.22 2021/09/01 14:28:15 schwarze Exp $ */ /*- * Copyright (c) 1993, 1994 @@ -54,34 +54,32 @@ cl_event(SCR *sp, EVENT *evp, u_int32_t flags, int ms) * so that we just keep returning them until the editor dies. */ clp = CLP(sp); -retest: if (LF_ISSET(EC_INTERRUPT) || F_ISSET(clp, CL_SIGINT)) { - if (F_ISSET(clp, CL_SIGINT)) { - F_CLR(clp, CL_SIGINT); +retest: if (LF_ISSET(EC_INTERRUPT) || cl_sigint) { + if (cl_sigint) { + cl_sigint = 0; evp->e_event = E_INTERRUPT; } else evp->e_event = E_TIMEOUT; return (0); } - if (F_ISSET(clp, CL_SIGHUP | CL_SIGTERM | CL_SIGWINCH)) { - if (F_ISSET(clp, CL_SIGHUP)) { - evp->e_event = E_SIGHUP; - return (0); - } - if (F_ISSET(clp, CL_SIGTERM)) { - evp->e_event = E_SIGTERM; + if (cl_sighup) { + evp->e_event = E_SIGHUP; + return (0); + } + if (cl_sigterm) { + evp->e_event = E_SIGTERM; + return (0); + } + if (cl_sigwinch) { + cl_sigwinch = 0; + if (cl_ssize(sp, 1, &lines, &columns, &changed)) + return (1); + if (changed) { + (void)cl_resize(sp, lines, columns); + evp->e_event = E_WRESIZE; return (0); } - if (F_ISSET(clp, CL_SIGWINCH)) { - F_CLR(clp, CL_SIGWINCH); - if (cl_ssize(sp, 1, &lines, &columns, &changed)) - return (1); - if (changed) { - (void)cl_resize(sp, lines, columns); - evp->e_event = E_WRESIZE; - return (0); - } - /* No real change, ignore the signal. */ - } + /* No real change, ignore the signal. */ } /* Set timer. */ -- cgit v1.2.3