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@

diff --git a/bin/pax/ar_io.c b/bin/pax/ar_io.c
index ead7352..e4bf39b 100644 (file)
--- 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 bbefbe9..48493bd 100644 (file)
--- 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 a8ef317..0fe50d6 100644 (file)
--- 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 511c9ca..abb6ecb 100644 (file)
--- 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 5fc78e4..3377fdc 100644 (file)
--- 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;
 }