Make the signal handler safe: block signals when updating data-structures

that are walked by routines called from the signal handler and use
dprintf() instead fprintf() in ar_close().

ok millert@

5 files changed, 66 insertions, 47 deletions

diff --git a/bin/pax/ar_io.c b/bin/pax/ar_io.c
index ead73525720..e4bf39b5903 100644
--- a/bin/pax/ar_io.c
+++ b/bin/pax/ar_io.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ar_io.c,v 1.45 2014/05/21 04:17:56 guenther Exp $	*/
+/*	$OpenBSD: ar_io.c,v 1.46 2014/05/23 19:47:49 guenther Exp $	*/
 /*	$NetBSD: ar_io.c,v 1.5 1996/03/26 23:54:13 mrg Exp $	*/
 
 /*-
@@ -289,11 +289,12 @@ ar_open(const char *name)
 }
 
 /*
- * ar_close()
+ * ar_close(int int_sig)
  *	closes archive device, increments volume number, and prints i/o summary
+ *	If in_sig is set we're in a signal handler and can't flush stdio.
  */
 void
-ar_close(void)
+ar_close(int in_sig)
 {
 	int status;
 
@@ -301,6 +302,8 @@ ar_close(void)
 		did_io = io_ok = flcnt = 0;
 		return;
 	}
+	if (!in_sig)
+		fflush(listf);
 
 	/*
 	 * Close archive file. This may take a LONG while on tapes (we may be
@@ -309,12 +312,9 @@ ar_close(void)
 	 * broken).
 	 */
 	if (vflag && (artyp == ISTAPE)) {
-		if (vfpart)
-			(void)putc('\n', listf);
-		(void)fprintf(listf,
-			"%s: Waiting for tape drive close to complete...",
-			argv0);
-		(void)fflush(listf);
+		(void)dprintf(listfd,
+		    "%s%s: Waiting for tape drive close to complete...",
+		    vfpart ? "\n" : "", argv0);
 	}
 
 	/*
@@ -347,9 +347,8 @@ ar_close(void)
 
 
 	if (vflag && (artyp == ISTAPE)) {
-		(void)fputs("done.\n", listf);
+		(void)write(listfd, "done.\n", sizeof("done.\n")-1);
 		vfpart = 0;
-		(void)fflush(listf);
 	}
 	arfd = -1;
 
@@ -375,7 +374,7 @@ ar_close(void)
 	 * Print out a summary of I/O for this archive volume.
 	 */
 	if (vfpart) {
-		(void)putc('\n', listf);
+		(void)write(listfd, "\n", 1);
 		vfpart = 0;
 	}
 
@@ -385,22 +384,21 @@ ar_close(void)
 	 * could have written anything yet.
 	 */
 	if (frmt == NULL) {
-		(void)fprintf(listf, "%s: unknown format, %llu bytes skipped.\n",
-		    argv0, rdcnt);
-		(void)fflush(listf);
+		(void)dprintf(listfd,
+		    "%s: unknown format, %llu bytes skipped.\n", argv0, rdcnt);
 		flcnt = 0;
 		return;
 	}
 
 	if (strcmp(NM_TAR, argv0) != 0)
-		(void)fprintf(listf,
-		    "%s: %s vol %d, %lu files, %llu bytes read, %llu bytes written.\n",
+		(void)dprintf(listfd, "%s: %s vol %d, %lu files,"
+		    " %llu bytes read, %llu bytes written.\n",
 		    argv0, frmt->name, arvol-1, flcnt, rdcnt, wrcnt);
 #ifndef NOCPIO
 	else if (strcmp(NM_CPIO, argv0) == 0)
-		(void)fprintf(listf, "%llu blocks\n", (rdcnt ? rdcnt : wrcnt) / 5120);
+		(void)dprintf(listfd, "%llu blocks\n",
+		    (rdcnt ? rdcnt : wrcnt) / 5120);
 #endif /* !NOCPIO */
-	(void)fflush(listf);
 	flcnt = 0;
 }
 
@@ -1110,7 +1108,7 @@ ar_next(void)
 	 */
 	if (sigprocmask(SIG_BLOCK, &s_mask, &o_mask) < 0)
 		syswarn(0, errno, "Unable to set signal mask");
-	ar_close();
+	ar_close(0);
 	if (sigprocmask(SIG_SETMASK, &o_mask, NULL) < 0)
 		syswarn(0, errno, "Unable to restore signal mask");
 
diff --git a/bin/pax/ar_subs.c b/bin/pax/ar_subs.c
index bbefbe94398..48493bd17ab 100644
--- a/bin/pax/ar_subs.c
+++ b/bin/pax/ar_subs.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ar_subs.c,v 1.38 2014/02/05 20:35:42 halex Exp $	*/
+/*	$OpenBSD: ar_subs.c,v 1.39 2014/05/23 19:47:49 guenther Exp $	*/
 /*	$NetBSD: ar_subs.c,v 1.5 1995/03/21 09:07:06 cgd Exp $	*/
 
 /*-
@@ -144,7 +144,7 @@ list(void)
 	 */
 	(void)(*frmt->end_rd)();
 	(void)sigprocmask(SIG_BLOCK, &s_mask, NULL);
-	ar_close();
+	ar_close(0);
 	pat_chk();
 }
 
@@ -359,8 +359,8 @@ popd:
 	 */
 	(void)(*frmt->end_rd)();
 	(void)sigprocmask(SIG_BLOCK, &s_mask, NULL);
-	ar_close();
-	proc_dir();
+	ar_close(0);
+	proc_dir(0);
 	pat_chk();
 }
 
@@ -554,9 +554,9 @@ trailer:
 		wr_fin();
 	}
 	(void)sigprocmask(SIG_BLOCK, &s_mask, NULL);
-	ar_close();
+	ar_close(0);
 	if (tflag)
-		proc_dir();
+		proc_dir(0);
 	ftree_chk();
 }
 
@@ -968,8 +968,8 @@ copy(void)
 	 * multiple entry into the cleanup code.
 	 */
 	(void)sigprocmask(SIG_BLOCK, &s_mask, NULL);
-	ar_close();
-	proc_dir();
+	ar_close(0);
+	proc_dir(0);
 	ftree_chk();
 }
 
diff --git a/bin/pax/extern.h b/bin/pax/extern.h
index a8ef317e7bd..0fe50d61ce6 100644
--- a/bin/pax/extern.h
+++ b/bin/pax/extern.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: extern.h,v 1.40 2014/01/19 10:22:57 guenther Exp $	*/
+/*	$OpenBSD: extern.h,v 1.41 2014/05/23 19:47:49 guenther Exp $	*/
 /*	$NetBSD: extern.h,v 1.5 1996/03/26 23:54:16 mrg Exp $	*/
 
 /*-
@@ -47,7 +47,7 @@ extern const char *arcname;
 extern const char *gzip_program;
 extern int force_one_volume;
 int ar_open(const char *);
-void ar_close(void);
+void ar_close(int _in_sig);
 void ar_drain(void);
 int ar_set_wr(void);
 int ar_app_ok(void);
@@ -236,6 +236,7 @@ extern int docrc;
 extern char *dirptr;
 extern char *argv0;
 extern FILE *listf;
+extern int listfd;
 extern char *tempfile;
 extern char *tempbase;
 extern int havechd;
@@ -272,7 +273,7 @@ void add_atdir(char *, dev_t, ino_t, time_t, time_t);
 int get_atdir(dev_t, ino_t, time_t *, time_t *);
 int dir_start(void);
 void add_dir(char *, struct stat *, int);
-void proc_dir(void);
+void proc_dir(int _in_sig);
 u_int st_hash(char *, int, int);
 
 /*
diff --git a/bin/pax/pax.c b/bin/pax/pax.c
index 511c9ca7c9f..abb6ecb493f 100644
--- a/bin/pax/pax.c
+++ b/bin/pax/pax.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pax.c,v 1.35 2014/01/09 03:12:25 guenther Exp $	*/
+/*	$OpenBSD: pax.c,v 1.36 2014/05/23 19:47:49 guenther Exp $	*/
 /*	$NetBSD: pax.c,v 1.5 1996/03/26 23:54:20 mrg Exp $	*/
 
 /*-
@@ -92,6 +92,7 @@ char	*dirptr;		/* destination dir in a copy */
 char	*argv0;			/* root of argv[0] */
 sigset_t s_mask;		/* signal mask for cleanup critical sect */
 FILE	*listf = stderr;	/* file pointer to print file list to */
+int	listfd = STDERR_FILENO;	/* fd matching listf, for sighandler output */
 char	*tempfile;		/* tempfile to use for mkstemp(3) */
 char	*tempbase;		/* basename of tempfile to use for mkstemp(3) */
 
@@ -297,24 +298,22 @@ sig_cleanup(int which_sig)
 
 	/*
 	 * restore modes and times for any dirs we may have created
-	 * or any dirs we may have read. Set vflag and vfpart so the user
-	 * will clearly see the message on a line by itself.
+	 * or any dirs we may have read.
 	 */
-	vflag = vfpart = 1;
 
 	/* paxwarn() uses stdio; fake it as well as we can */
 	if (which_sig == SIGXCPU)
-		strlcpy(errbuf, "CPU time limit reached, cleaning up.\n",
+		strlcpy(errbuf, "\nCPU time limit reached, cleaning up.\n",
 		    sizeof errbuf);
 	else
-		strlcpy(errbuf, "Signal caught, cleaning up.\n",
+		strlcpy(errbuf, "\nSignal caught, cleaning up.\n",
 		    sizeof errbuf);
 	(void) write(STDERR_FILENO, errbuf, strlen(errbuf));
 
-	ar_close();			/* XXX signal race */
-	proc_dir();			/* XXX signal race */
+	ar_close(1);
+	proc_dir(1);
 	if (tflag)
-		atdir_end();		/* XXX signal race */
+		atdir_end();
 	_exit(1);
 }
 
@@ -378,6 +377,10 @@ gen_init(void)
 		paxwarn(1, "Unable to set up signal mask");
 		return(-1);
 	}
+
+	/* snag the fd to be used from the signal handler */
+	listfd = fileno(listf);
+
 	memset(&n_hand, 0, sizeof n_hand);
 	n_hand.sa_mask = s_mask;
 	n_hand.sa_flags = 0;
diff --git a/bin/pax/tables.c b/bin/pax/tables.c
index 5fc78e41665..3377fdcda7e 100644
--- a/bin/pax/tables.c
+++ b/bin/pax/tables.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: tables.c,v 1.31 2014/05/07 14:56:57 tedu Exp $	*/
+/*	$OpenBSD: tables.c,v 1.32 2014/05/23 19:47:49 guenther Exp $	*/
 /*	$NetBSD: tables.c,v 1.4 1995/03/21 09:07:45 cgd Exp $	*/
 
 /*-
@@ -954,6 +954,7 @@ void
 add_atdir(char *fname, dev_t dev, ino_t ino, time_t mtime, time_t atime)
 {
 	ATDIR *pt;
+	sigset_t allsigs, savedsigs;
 	u_int indx;
 
 	if (atab == NULL)
@@ -984,7 +985,9 @@ add_atdir(char *fname, dev_t dev, ino_t ino, time_t mtime, time_t atime)
 	/*
 	 * add it to the front of the hash chain
 	 */
-	if ((pt = (ATDIR *)malloc(sizeof(ATDIR))) != NULL) {
+	sigfillset(&allsigs);
+	sigprocmask(SIG_BLOCK, &allsigs, &savedsigs);
+	if ((pt = malloc(sizeof *pt)) != NULL) {
 		if ((pt->name = strdup(fname)) != NULL) {
 			pt->dev = dev;
 			pt->ino = ino;
@@ -992,11 +995,13 @@ add_atdir(char *fname, dev_t dev, ino_t ino, time_t mtime, time_t atime)
 			pt->atime = atime;
 			pt->fow = atab[indx];
 			atab[indx] = pt;
+			sigprocmask(SIG_SETMASK, &savedsigs, NULL);
 			return;
 		}
 		(void)free((char *)pt);
 	}
 
+	sigprocmask(SIG_SETMASK, &savedsigs, NULL);
 	paxwarn(1, "Directory access time reset table ran out of memory");
 	return;
 }
@@ -1017,6 +1022,7 @@ get_atdir(dev_t dev, ino_t ino, time_t *mtime, time_t *atime)
 {
 	ATDIR *pt;
 	ATDIR **ppt;
+	sigset_t allsigs, savedsigs;
 	u_int indx;
 
 	if (atab == NULL)
@@ -1048,7 +1054,10 @@ get_atdir(dev_t dev, ino_t ino, time_t *mtime, time_t *atime)
 	/*
 	 * found it. return the times and remove the entry from the table.
 	 */
+	sigfillset(&allsigs);
+	sigprocmask(SIG_BLOCK, &allsigs, &savedsigs);
 	*ppt = pt->fow;
+	sigprocmask(SIG_SETMASK, &savedsigs, NULL);
 	*mtime = pt->mtime;
 	*atime = pt->atime;
 	(void)free((char *)pt->name);
@@ -1118,6 +1127,7 @@ void
 add_dir(char *name, struct stat *psb, int frc_mode)
 {
 	DIRDATA *dblk;
+	sigset_t allsigs, savedsigs;
 	char realname[MAXPATHLEN], *rp;
 
 	if (dirp == NULL)
@@ -1137,8 +1147,10 @@ add_dir(char *name, struct stat *psb, int frc_mode)
 			    " directory: %s", name);
 			return;
 		}
+		sigprocmask(SIG_BLOCK, &allsigs, &savedsigs);
 		dirp = dblk;
 		dirsize *= 2;
+		sigprocmask(SIG_SETMASK, &savedsigs, NULL);
 	}
 	dblk = &dirp[dircnt];
 	if ((dblk->name = strdup(name)) == NULL) {
@@ -1150,17 +1162,20 @@ add_dir(char *name, struct stat *psb, int frc_mode)
 	dblk->mtime = psb->st_mtime;
 	dblk->atime = psb->st_atime;
 	dblk->frc_mode = frc_mode;
+	sigprocmask(SIG_BLOCK, &allsigs, &savedsigs);
 	++dircnt;
+	sigprocmask(SIG_SETMASK, &savedsigs, NULL);
 }
 
 /*
- * proc_dir()
+ * proc_dir(int in_sig)
  *	process all file modes and times stored for directories CREATED
- *	by pax
+ *	by pax.  If in_sig is set, we're in a signal handler and can't
+ *	free stuff.
  */
 
 void
-proc_dir(void)
+proc_dir(int in_sig)
 {
 	DIRDATA *dblk;
 	size_t cnt;
@@ -1181,10 +1196,12 @@ proc_dir(void)
 			set_pmode(dblk->name, dblk->mode);
 		if (patime || pmtime)
 			set_ftime(dblk->name, dblk->mtime, dblk->atime, 0);
-		free(dblk->name);
+		if (!in_sig)
+			free(dblk->name);
 	}
 
-	free(dirp);
+	if (!in_sig)
+		free(dirp);
 	dirp = NULL;
 	dircnt = 0;
 }