Skip to content

Commit 634da7a

Browse files
committed
[sanitizer] Check if directory exists before trying to create
Add a DirExists mechanism, modeled after FileExists. Use it to guard creation of the report path directory. This should avoid failures running the sanitizer in a sandbox where the file creation attempt causes hard failures, even for an existing directory. Problem reported on D109794 for ChromeOS in sandbox (https://issuetracker.google.com/209296420). Differential Revision: https://reviews.llvm.org/D119495
1 parent 0e4ecfa commit 634da7a

File tree

9 files changed

+86
-13
lines changed

9 files changed

+86
-13
lines changed

compiler-rt/lib/sanitizer_common/sanitizer_file.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,12 @@ static void RecursiveCreateParentDirs(char *path) {
8484
if (!IsPathSeparator(path[i]))
8585
continue;
8686
path[i] = '\0';
87-
/* Some of these will fail, because the directory exists, ignore it. */
88-
CreateDir(path);
87+
if (!DirExists(path) && !CreateDir(path)) {
88+
const char *ErrorMsgPrefix = "ERROR: Can't create directory: ";
89+
WriteToFile(kStderrFd, ErrorMsgPrefix, internal_strlen(ErrorMsgPrefix));
90+
WriteToFile(kStderrFd, path, internal_strlen(path));
91+
Die();
92+
}
8993
path[i] = save;
9094
}
9195
}

compiler-rt/lib/sanitizer_common/sanitizer_file.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ bool SupportsColoredOutput(fd_t fd);
7777
// OS
7878
const char *GetPwd();
7979
bool FileExists(const char *filename);
80+
bool DirExists(const char *path);
8081
char *FindPathToBinary(const char *name);
8182
bool IsPathSeparator(const char c);
8283
bool IsAbsolutePath(const char *path);

compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -499,7 +499,18 @@ bool FileExists(const char *filename) {
499499
return S_ISREG(st.st_mode);
500500
}
501501

502-
#if !SANITIZER_NETBSD
502+
bool DirExists(const char *path) {
503+
struct stat st;
504+
# if SANITIZER_USES_CANONICAL_LINUX_SYSCALLS
505+
if (internal_syscall(SYSCALL(newfstatat), AT_FDCWD, path, &st, 0))
506+
# else
507+
if (internal_stat(path, &st))
508+
# endif
509+
return false;
510+
return S_ISDIR(st.st_mode);
511+
}
512+
513+
# if !SANITIZER_NETBSD
503514
tid_t GetTid() {
504515
#if SANITIZER_FREEBSD
505516
long Tid;

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,13 @@ bool FileExists(const char *filename) {
393393
return S_ISREG(st.st_mode);
394394
}
395395

396+
bool DirExists(const char *path) {
397+
struct stat st;
398+
if (stat(path, &st))
399+
return false;
400+
return S_ISDIR(st.st_mode);
401+
}
402+
396403
tid_t GetTid() {
397404
tid_t tid;
398405
pthread_threadid_np(nullptr, &tid);

compiler-rt/lib/sanitizer_common/sanitizer_win.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,11 @@ bool FileExists(const char *filename) {
9393
return ::GetFileAttributesA(filename) != INVALID_FILE_ATTRIBUTES;
9494
}
9595

96+
bool DirExists(const char *path) {
97+
auto attr = ::GetFileAttributesA(path);
98+
return (attr != INVALID_FILE_ATTRIBUTES) && (attr & FILE_ATTRIBUTE_DIRECTORY);
99+
}
100+
96101
uptr internal_getpid() {
97102
return GetProcessId(GetCurrentProcess());
98103
}

compiler-rt/lib/sanitizer_common/tests/sanitizer_libc_test.cpp

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,20 @@ struct stat_and_more {
6363
unsigned char z;
6464
};
6565

66+
static void get_temp_dir(char *buf, size_t bufsize) {
67+
#if SANITIZER_WINDOWS
68+
buf[0] = '\0';
69+
if (!::GetTempPathA(bufsize, buf))
70+
return;
71+
#else
72+
const char *tmpdir = "/tmp";
73+
# if SANITIZER_ANDROID
74+
tmpdir = GetEnv("TMPDIR");
75+
# endif
76+
internal_snprintf(buf, bufsize, "%s", tmpdir);
77+
#endif
78+
}
79+
6680
static void temp_file_name(char *buf, size_t bufsize, const char *prefix) {
6781
#if SANITIZER_WINDOWS
6882
buf[0] = '\0';
@@ -340,3 +354,19 @@ TEST(SanitizerCommon, ReportFile) {
340354
report_file.SetReportPath("stderr");
341355
Unlink(tmpfile);
342356
}
357+
358+
TEST(SanitizerCommon, FileExists) {
359+
char tmpfile[128];
360+
temp_file_name(tmpfile, sizeof(tmpfile), "sanitizer_common.fileexists.tmp.");
361+
fd_t fd = OpenFile(tmpfile, WrOnly);
362+
ASSERT_NE(fd, kInvalidFd);
363+
EXPECT_TRUE(FileExists(tmpfile));
364+
CloseFile(fd);
365+
Unlink(tmpfile);
366+
}
367+
368+
TEST(SanitizerCommon, DirExists) {
369+
char tmpdir[128];
370+
get_temp_dir(tmpdir, sizeof(tmpdir));
371+
EXPECT_TRUE(DirExists(tmpdir));
372+
}

compiler-rt/test/asan/TestCases/log-path_test.cpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,14 @@
1616
// RUN: %env_asan_opts=log_path=%t.log not %run %t 2> %t.out
1717
// RUN: FileCheck %s --check-prefix=CHECK-ERROR < %t.log.*
1818

19-
// Invalid log_path.
20-
// RUN: %env_asan_opts=log_path=/dev/null/INVALID not %run %t 2> %t.out
19+
// Invalid log_path in existing directory.
20+
// RUN: %env_asan_opts=log_path=/INVALID not %run %t 2> %t.out
2121
// RUN: FileCheck %s --check-prefix=CHECK-INVALID < %t.out
2222

23+
// Directory of log_path can't be created.
24+
// RUN: %env_asan_opts=log_path=/dev/null/INVALID not %run %t 2> %t.out
25+
// RUN: FileCheck %s --check-prefix=CHECK-BAD-DIR < %t.out
26+
2327
// Too long log_path.
2428
// RUN: %env_asan_opts=log_path=`for((i=0;i<10000;i++)); do echo -n $i; done` \
2529
// RUN: not %run %t 2> %t.out
@@ -44,5 +48,6 @@ int main(int argc, char **argv) {
4448
return res;
4549
}
4650
// CHECK-ERROR: ERROR: AddressSanitizer
47-
// CHECK-INVALID: ERROR: Can't open file: /dev/null/INVALID
51+
// CHECK-INVALID: ERROR: Can't open file: /INVALID
52+
// CHECK-BAD-DIR: ERROR: Can't create directory: /dev/null
4853
// CHECK-LONG: ERROR: Path is too long: 01234

compiler-rt/test/memprof/TestCases/log_path_test.cpp

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,21 @@
33
//
44
// RUN: %clangxx_memprof %s -o %t
55

6-
// stderr print_text=true:log_path
6+
// stderr log_path
77
// RUN: %env_memprof_opts=print_text=true:log_path=stderr %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-GOOD --dump-input=always
88

9-
// Good print_text=true:log_path.
9+
// Good log_path.
1010
// RUN: rm -f %t.log.*
1111
// RUN: %env_memprof_opts=print_text=true:log_path=%t.log %run %t
1212
// RUN: FileCheck %s --check-prefix=CHECK-GOOD --dump-input=always < %t.log.*
1313

14-
// Invalid print_text=true:log_path.
15-
// RUN: %env_memprof_opts=print_text=true:log_path=/dev/null/INVALID not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-INVALID --dump-input=always
14+
// Invalid log_path.
15+
// RUN: %env_memprof_opts=print_text=true:log_path=/INVALID not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-INVALID --dump-input=always
1616

17-
// Too long print_text=true:log_path.
17+
// Directory of log_path can't be created.
18+
// RUN: %env_memprof_opts=print_text=true:log_path=/dev/null/INVALID not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-BAD-DIR --dump-input=always
19+
20+
// Too long log_path.
1821
// RUN: %env_memprof_opts=print_text=true:log_path=`for((i=0;i<10000;i++)); do echo -n $i; done` \
1922
// RUN: not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-LONG --dump-input=always
2023

@@ -24,7 +27,7 @@
2427
// Using an automatically generated name via %t can cause weird issues with
2528
// unexpected macro expansion if the path includes tokens that match a build
2629
// system macro (e.g. "linux").
27-
// RUN: %clangxx_memprof %s -o %t -DPROFILE_NAME_VAR="/dev/null/INVALID"
30+
// RUN: %clangxx_memprof %s -o %t -DPROFILE_NAME_VAR="/INVALID"
2831
// RUN: not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-INVALID --dump-input=always
2932

3033
#include <sanitizer/memprof_interface.h>
@@ -45,5 +48,6 @@ int main(int argc, char **argv) {
4548
return 0;
4649
}
4750
// CHECK-GOOD: Memory allocation stack id
48-
// CHECK-INVALID: ERROR: Can't open file: /dev/null/INVALID
51+
// CHECK-INVALID: ERROR: Can't open file: /INVALID
52+
// CHECK-BAD-DIR: ERROR: Can't create directory: /dev/null
4953
// CHECK-LONG: ERROR: Path is too long: 01234

compiler-rt/test/sanitizer_common/TestCases/Posix/sanitizer_set_report_path_test.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
// Test __sanitizer_set_report_path and __sanitizer_get_report_path:
2+
// RUN: rm -rf %t.report_path
23
// RUN: %clangxx -O2 %s -o %t
34
// RUN: %run %t | FileCheck %s
5+
// Try again with a directory without write access.
6+
// RUN: rm -rf %t.report_path && mkdir -p %t.report_path
7+
// RUN: chmod u-w %t.report_path || true
8+
// RUN: not %run %t 2>&1 | FileCheck %s --check-prefix=FAIL
49

510
#include <assert.h>
611
#include <sanitizer/common_interface_defs.h>
@@ -18,3 +23,4 @@ int main(int argc, char **argv) {
1823
}
1924

2025
// CHECK: Path {{.*}}Posix/Output/sanitizer_set_report_path_test.cpp.tmp.report_path/report.
26+
// FAIL: ERROR: Can't open file: {{.*}}Posix/Output/sanitizer_set_report_path_test.cpp.tmp.report_path/report.

0 commit comments

Comments
 (0)