1 | diff --git a/Source/PLCrashAsyncFile.cpp b/Source/PLCrashAsyncFile.cpp |
---|
2 | index c0c6e36..83f59e2 100644 |
---|
3 | --- a/Source/PLCrashAsyncFile.cpp |
---|
4 | +++ b/Source/PLCrashAsyncFile.cpp |
---|
5 | @@ -26,6 +26,8 @@ |
---|
6 | |
---|
7 | #include "PLCrashAsyncFile.hpp" |
---|
8 | |
---|
9 | +#include "SecureRandom.hpp" |
---|
10 | + |
---|
11 | #include <unistd.h> |
---|
12 | #include <errno.h> |
---|
13 | |
---|
14 | @@ -114,14 +116,14 @@ ssize_t AsyncFile::readn (int fd, void *data, size_t len) { |
---|
15 | * |
---|
16 | * @param pathTemplate A writable template. |
---|
17 | * @param mode The file mode for the target file. The file will be created with this mode, modified by the process' umask value. |
---|
18 | + * @param outfd On success, a read/write file descriptor. |
---|
19 | * |
---|
20 | - * @return A file descriptor open for reading and writing, or an error if the target path can not be opened. This |
---|
21 | - * method may fail for any reason specified by open(2). |
---|
22 | + * @return PLCRASH_ESUCCESS if the temporary file was successfully opened, or an error if the target path can not be opened. |
---|
23 | * |
---|
24 | * @warning While this method is a loose analogue of libc mkstemp(3), the semantics differ, and API clients should not |
---|
25 | - * rely on any particular similarities with the standard library function. |
---|
26 | + * rely on any similarities with the standard library function that are not explicitly documented. |
---|
27 | */ |
---|
28 | -int AsyncFile::mktemp (char *ptemplate, mode_t mode) { |
---|
29 | +plcrash_error_t AsyncFile::mktemp (char *ptemplate, mode_t mode, int *outfd) { |
---|
30 | /* Characters to use for padding */ |
---|
31 | static const char padchar[] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"; |
---|
32 | |
---|
33 | @@ -149,21 +151,57 @@ int AsyncFile::mktemp (char *ptemplate, mode_t mode) { |
---|
34 | } |
---|
35 | } |
---|
36 | } |
---|
37 | - |
---|
38 | - int fd; |
---|
39 | - do { |
---|
40 | - /* Insert random suffix into the template. */ |
---|
41 | - for (size_t i = 0; i < suffix_len; i++) { |
---|
42 | - // XXXTODO: The use of arc4random_uniform() is not async-safe. |
---|
43 | - // This is a stand-in until we can implement async-safe random deriviation. |
---|
44 | - ptemplate[suffix_i + i] = padchar[arc4random_uniform(sizeof(padchar) - 1)]; |
---|
45 | + |
---|
46 | + /* Insert random suffix into the template. */ |
---|
47 | + SecureRandom rnd = SecureRandom(); |
---|
48 | + for (size_t i = 0; i < suffix_len; i++) { |
---|
49 | + plcrash_error_t err; |
---|
50 | + uint32_t charIndex; |
---|
51 | + |
---|
52 | + /* Try to read a random suffix character */ |
---|
53 | + if ((err = rnd.uniform(sizeof(padchar) - 1, &charIndex)) != PLCRASH_ESUCCESS) { |
---|
54 | + PLCF_DEBUG("Failed to fetch bytes from SecureRandom.uniform(): %s", plcrash_async_strerror(err)); |
---|
55 | + return err; |
---|
56 | } |
---|
57 | |
---|
58 | + /* Update the suffix. */ |
---|
59 | + PLCF_ASSERT(charIndex < sizeof(padchar)); |
---|
60 | + ptemplate[suffix_i + i] = padchar[charIndex]; |
---|
61 | + } |
---|
62 | + |
---|
63 | + /* Save a copy of the suffix. We use this to determine when roll-over occurs while trying all possible combinations. */ |
---|
64 | + char original_suffix[suffix_len]; |
---|
65 | + plcrash_async_memcpy(original_suffix, &ptemplate[suffix_i], suffix_len); |
---|
66 | + |
---|
67 | + /* Loop until open(2) either succeeds, returns an error other than EEXIST, or we've tried every available combination. */ |
---|
68 | + int fd; |
---|
69 | + int open_errno = 0; |
---|
70 | + do { |
---|
71 | /* Try to open the file. */ |
---|
72 | fd = open(ptemplate, O_CREAT|O_EXCL|O_RDWR, mode); |
---|
73 | + |
---|
74 | + /* Terminate our loop on success */ |
---|
75 | + if (fd >= 0) |
---|
76 | + break; |
---|
77 | + |
---|
78 | + /* Save errno so we can safely log it later */ |
---|
79 | + open_errno = errno; |
---|
80 | + |
---|
81 | + /* Terminate if we get an error other than EEXIST */ |
---|
82 | + if (open_errno != EEXIST) |
---|
83 | + break; |
---|
84 | } while (fd < 0); |
---|
85 | + |
---|
86 | + /* Check for open() failure */ |
---|
87 | + if (fd < 0) { |
---|
88 | + PLCF_DEBUG("Failed to open output file in mktemp(): %d", open_errno); |
---|
89 | + return PLCRASH_OUTPUT_ERR; |
---|
90 | + } |
---|
91 | |
---|
92 | - return fd; |
---|
93 | + /* Provide the successful result */ |
---|
94 | + PLCF_ASSERT(fd >= 0); |
---|
95 | + *outfd = fd; |
---|
96 | + return PLCRASH_ESUCCESS; |
---|
97 | } |
---|
98 | |
---|
99 | /** |
---|
100 | diff --git a/Source/PLCrashAsyncFile.hpp b/Source/PLCrashAsyncFile.hpp |
---|
101 | index 20e7994..49566a9 100644 |
---|
102 | --- a/Source/PLCrashAsyncFile.hpp |
---|
103 | +++ b/Source/PLCrashAsyncFile.hpp |
---|
104 | @@ -51,7 +51,7 @@ public: |
---|
105 | static ssize_t writen (int fd, const void *data, size_t len); |
---|
106 | static ssize_t readn (int fd, void *data, size_t len); |
---|
107 | |
---|
108 | - static int mktemp (char *ptemplate, mode_t mode); |
---|
109 | + static plcrash_error_t mktemp (char *ptemplate, mode_t mode, int *outfd); |
---|
110 | |
---|
111 | AsyncFile (int fd, off_t output_limit); |
---|
112 | |
---|
113 | diff --git a/Source/PLCrashAsyncFileTests.mm b/Source/PLCrashAsyncFileTests.mm |
---|
114 | index 41fb574..52de465 100644 |
---|
115 | --- a/Source/PLCrashAsyncFileTests.mm |
---|
116 | +++ b/Source/PLCrashAsyncFileTests.mm |
---|
117 | @@ -120,8 +120,8 @@ using namespace plcrash::async; |
---|
118 | } |
---|
119 | |
---|
120 | /* Try creating the temporary file */ |
---|
121 | - _tempFD = AsyncFile::mktemp(ptemplate, 0600); |
---|
122 | - STAssertTrue(_tempFD >= 0, @"Failed to create output file"); |
---|
123 | + STAssertEquals(PLCRASH_ESUCCESS, AsyncFile::mktemp(ptemplate, 0600, &_tempFD), @"mktemp() returned an error"); |
---|
124 | + STAssertTrue(_tempFD >= 0, @"Failed to create output file descriptor"); |
---|
125 | _tempOutputFile = [[[NSString alloc] initWithUTF8String: ptemplate] retain]; |
---|
126 | STAssertNotNil(_tempOutputFile, @"String initialization failed for '%s'", ptemplate); |
---|
127 | |
---|
128 | @@ -142,6 +142,21 @@ using namespace plcrash::async; |
---|
129 | free(ptemplate); |
---|
130 | } |
---|
131 | |
---|
132 | +/** |
---|
133 | + * Test temporary file handling; verify that termination occurs if all possible combinations exist on disk. |
---|
134 | + */ |
---|
135 | +- (void) testMkTempTermination { |
---|
136 | + /* Generate a template string; the lack of 'X' suffix characters gaurantees that there's only one possible combination. */ |
---|
137 | + char *ptemplate = strdup([[NSTemporaryDirectory() stringByAppendingPathComponent: @"mktemp_test"] fileSystemRepresentation]); |
---|
138 | + |
---|
139 | + /* Verify that mktemp() succeds when the path does not exist ... */ |
---|
140 | + STAssertEquals(PLCRASH_ESUCCESS, AsyncFile::mktemp(ptemplate, 0600, &_tempFD), @"mktemp() returned an error"); |
---|
141 | + close(_tempFD); |
---|
142 | + |
---|
143 | + /* ... and returns an error when the path does exist */ |
---|
144 | + STAssertEquals(PLCRASH_OUTPUT_ERR, AsyncFile::mktemp(ptemplate, 0600, &_tempFD), @"mktemp() did not return an error"); |
---|
145 | +} |
---|
146 | + |
---|
147 | /* Writer thread used for testReadWriteN */ |
---|
148 | struct background_writer_ctx { |
---|
149 | int wfd; |
---|