tempfile: avoid directory cleanup race

The temporary directory created by mks_tempfile_dt() is deleted by first
deleting the file within, then truncating the filename strbuf and
passing the resulting string to rmdir(2).  When the cleanup routine is
invoked concurrently by a signal handler we can end up passing the now
truncated string to unlink(2), however, which could cause problems on
some systems.

Avoid that issue by remembering the directory name separately.  This way
the paths stay unchanged.  A signal handler can still race with normal
cleanup, but deleting the same files and directories twice is harmless.

Reported-by: Jeff King <peff@peff.net>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/tempfile.c b/tempfile.c
index 2024c82691fe4a1a1c9b5c8ebd31d6c382fe8bfc..7414c81e31efb3cef8921990f44a008ce5073a21 100644 (file)
--- a/tempfile.c
+++ b/tempfile.c
@@ -59,14 +59,11 @@ static VOLATILE_LIST_HEAD(tempfile_list);
 static void remove_template_directory(struct tempfile *tempfile,
                                      int in_signal_handler)
 {
-       if (tempfile->directorylen > 0 &&
-           tempfile->directorylen < tempfile->filename.len &&
-           tempfile->filename.buf[tempfile->directorylen] == '/') {
-               strbuf_setlen(&tempfile->filename, tempfile->directorylen);
+       if (tempfile->directory) {
                if (in_signal_handler)
-                       rmdir(tempfile->filename.buf);
+                       rmdir(tempfile->directory);
                else
-                       rmdir_or_warn(tempfile->filename.buf);
+                       rmdir_or_warn(tempfile->directory);
        }
 }
 
@@ -115,7 +112,7 @@ static struct tempfile *new_tempfile(void)
        tempfile->owner = 0;
        INIT_LIST_HEAD(&tempfile->list);
        strbuf_init(&tempfile->filename, 0);
-       tempfile->directorylen = 0;
+       tempfile->directory = NULL;
        return tempfile;
 }
 
@@ -141,6 +138,7 @@ static void deactivate_tempfile(struct tempfile *tempfile)
 {
        tempfile->active = 0;
        strbuf_release(&tempfile->filename);
+       free(tempfile->directory);
        volatile_list_del(&tempfile->list);
        free(tempfile);
 }
@@ -254,7 +252,7 @@ struct tempfile *mks_tempfile_dt(const char *directory_template,
 
        tempfile = new_tempfile();
        strbuf_swap(&tempfile->filename, &sb);
-       tempfile->directorylen = directorylen;
+       tempfile->directory = xmemdupz(tempfile->filename.buf, directorylen);
        tempfile->fd = fd;
        activate_tempfile(tempfile);
        return tempfile;

diff --git a/tempfile.h b/tempfile.h
index d7804a214abb60ee60496ef34a9fc8be110344ea..5b9e8743dd3e3649a6583e0de2aa62bd57322359 100644 (file)
--- a/tempfile.h
+++ b/tempfile.h
@@ -82,7 +82,7 @@ struct tempfile {
        FILE *volatile fp;
        volatile pid_t owner;
        struct strbuf filename;
-       size_t directorylen;
+       char *directory;
 };
 
 /*