From 0a8bc8ffaec8677dcd7c81f9f4b2adc18bbc8211 Mon Sep 17 00:00:00 2001 From: guenther Date: Fri, 23 May 2014 19:47:49 +0000 Subject: [PATCH] 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@ --- bin/pax/ar_io.c | 38 ++++++++++++++++++-------------------- bin/pax/ar_subs.c | 16 ++++++++-------- bin/pax/extern.h | 7 ++++--- bin/pax/pax.c | 21 ++++++++++++--------- bin/pax/tables.c | 31 ++++++++++++++++++++++++------- 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; } -- 2.20.1