@matheusbsbs / matheus-bagliano / commits / babe2e0559

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>

René Scharfe committed Aug 27, 2022 at 00:46 UTC babe2e05592f0e8025061ffc97e387e2aa70c99b

2 files changed +7 -9

tempfile.c

+6 -8

@@ -59,14 +59,11 @@ static VOLATILE_LIST_HEAD(tempfile_list);

59 static void remove_template_directory(struct tempfile *tempfile,

60 int in_signal_handler)

61 {

62 - if (tempfile->directorylen > 0 &&

63 - tempfile->directorylen < tempfile->filename.len &&

64 - tempfile->filename.buf[tempfile->directorylen] == '/') {

65 - strbuf_setlen(&tempfile->filename, tempfile->directorylen);

62 + if (tempfile->directory) {

63 if (in_signal_handler)

67 - rmdir(tempfile->filename.buf);

64 + rmdir(tempfile->directory);

65 else

69 - rmdir_or_warn(tempfile->filename.buf);

66 + rmdir_or_warn(tempfile->directory);

67 }

68 }

@@ -115,7 +112,7 @@ static struct tempfile *new_tempfile(void)

112 tempfile->owner = 0;

113 INIT_LIST_HEAD(&tempfile->list);

114 strbuf_init(&tempfile->filename, 0);

118 - tempfile->directorylen = 0;

115 + tempfile->directory = NULL;

116 return tempfile;

117 }

118

@@ -141,6 +138,7 @@ static void deactivate_tempfile(struct tempfile *tempfile)

138 {

139 tempfile->active = 0;

140 strbuf_release(&tempfile->filename);

141 + free(tempfile->directory);

142 volatile_list_del(&tempfile->list);

143 free(tempfile);

144 }

@@ -254,7 +252,7 @@ struct tempfile *mks_tempfile_dt(const char *directory_template,

252

253 tempfile = new_tempfile();

254 strbuf_swap(&tempfile->filename, &sb);

257 - tempfile->directorylen = directorylen;

255 + tempfile->directory = xmemdupz(tempfile->filename.buf, directorylen);

256 tempfile->fd = fd;

257 activate_tempfile(tempfile);

258 return tempfile;

tempfile.h

+1 -1

@@ -82,7 +82,7 @@ struct tempfile {

82 FILE *volatile fp;

83 volatile pid_t owner;

84 struct strbuf filename;

85 - size_t directorylen;

85 + char *directory;

86 };

88 /*