authorgravatar for info@bnoordhuis.nlBen Noordhuis <info@bnoordhuis.nl> 2019-04-13 12:31:45+02:00
committergravatar for info@bnoordhuis.nlBen Noordhuis <info@bnoordhuis.nl> 2019-04-13 12:31:49+02:00
logea5518f69edc51e8e70c2c4d4c4daa3ad9bcb242
treeb1b34290531dff64b2dde087a1230a13eacf4b50
parent6284a4c534771797406ceb945e5b7985d591d62b

make os_file_close poison file handle after close

This helps track down use-after-close bugs.

3 files changed, 18 insertions(+), 16 deletions(-)

src/cache_hash.cpp+12-12
...@@ -256,10 +256,10 @@ static Error populate_file_hash(CacheHash *ch, CacheHashFile *chf, Buf *contents...@@ -256,10 +256,10 @@ static Error populate_file_hash(CacheHash *ch, CacheHashFile *chf, Buf *contents
256 }256 }
257257
258 if ((err = hash_file(chf->bin_digest, this_file, contents))) {258 if ((err = hash_file(chf->bin_digest, this_file, contents))) {
259 os_file_close(this_file);259 os_file_close(&this_file);
260 return err;260 return err;
261 }261 }
262 os_file_close(this_file);262 os_file_close(&this_file);
263263
264 blake2b_update(&ch->blake, chf->bin_digest, 48);264 blake2b_update(&ch->blake, chf->bin_digest, 48);
265265
...@@ -300,7 +300,7 @@ Error cache_hit(CacheHash *ch, Buf *out_digest) {...@@ -300,7 +300,7 @@ Error cache_hit(CacheHash *ch, Buf *out_digest) {
300 Buf line_buf = BUF_INIT;300 Buf line_buf = BUF_INIT;
301 buf_resize(&line_buf, 512);301 buf_resize(&line_buf, 512);
302 if ((err = os_file_read_all(ch->manifest_file, &line_buf))) {302 if ((err = os_file_read_all(ch->manifest_file, &line_buf))) {
303 os_file_close(ch->manifest_file);303 os_file_close(&ch->manifest_file);
304 return err;304 return err;
305 }305 }
306306
...@@ -389,14 +389,14 @@ Error cache_hit(CacheHash *ch, Buf *out_digest) {...@@ -389,14 +389,14 @@ Error cache_hit(CacheHash *ch, Buf *out_digest) {
389 OsFileAttr actual_attr;389 OsFileAttr actual_attr;
390 if ((err = os_file_open_r(chf->path, &this_file, &actual_attr))) {390 if ((err = os_file_open_r(chf->path, &this_file, &actual_attr))) {
391 fprintf(stderr, "Unable to open %s\n: %s", buf_ptr(chf->path), err_str(err));391 fprintf(stderr, "Unable to open %s\n: %s", buf_ptr(chf->path), err_str(err));
392 os_file_close(ch->manifest_file);392 os_file_close(&ch->manifest_file);
393 return ErrorCacheUnavailable;393 return ErrorCacheUnavailable;
394 }394 }
395 if (chf->attr.mtime.sec == actual_attr.mtime.sec &&395 if (chf->attr.mtime.sec == actual_attr.mtime.sec &&
396 chf->attr.mtime.nsec == actual_attr.mtime.nsec &&396 chf->attr.mtime.nsec == actual_attr.mtime.nsec &&
397 chf->attr.inode == actual_attr.inode)397 chf->attr.inode == actual_attr.inode)
398 {398 {
399 os_file_close(this_file);399 os_file_close(&this_file);
400 } else {400 } else {
401 // we have to recompute the digest.401 // we have to recompute the digest.
402 // later we'll rewrite the manifest with the new mtime/digest values402 // later we'll rewrite the manifest with the new mtime/digest values
...@@ -411,11 +411,11 @@ Error cache_hit(CacheHash *ch, Buf *out_digest) {...@@ -411,11 +411,11 @@ Error cache_hit(CacheHash *ch, Buf *out_digest) {
411411
412 uint8_t actual_digest[48];412 uint8_t actual_digest[48];
413 if ((err = hash_file(actual_digest, this_file, nullptr))) {413 if ((err = hash_file(actual_digest, this_file, nullptr))) {
414 os_file_close(this_file);414 os_file_close(&this_file);
415 os_file_close(ch->manifest_file);415 os_file_close(&ch->manifest_file);
416 return err;416 return err;
417 }417 }
418 os_file_close(this_file);418 os_file_close(&this_file);
419 if (memcmp(chf->bin_digest, actual_digest, 48) != 0) {419 if (memcmp(chf->bin_digest, actual_digest, 48) != 0) {
420 memcpy(chf->bin_digest, actual_digest, 48);420 memcpy(chf->bin_digest, actual_digest, 48);
421 // keep going until we have the input file digests421 // keep going until we have the input file digests
...@@ -433,12 +433,12 @@ Error cache_hit(CacheHash *ch, Buf *out_digest) {...@@ -433,12 +433,12 @@ Error cache_hit(CacheHash *ch, Buf *out_digest) {
433 CacheHashFile *chf = &ch->files.at(file_i);433 CacheHashFile *chf = &ch->files.at(file_i);
434 if ((err = populate_file_hash(ch, chf, nullptr))) {434 if ((err = populate_file_hash(ch, chf, nullptr))) {
435 fprintf(stderr, "Unable to hash %s: %s\n", buf_ptr(chf->path), err_str(err));435 fprintf(stderr, "Unable to hash %s: %s\n", buf_ptr(chf->path), err_str(err));
436 os_file_close(ch->manifest_file);436 os_file_close(&ch->manifest_file);
437 return ErrorCacheUnavailable;437 return ErrorCacheUnavailable;
438 }438 }
439 }439 }
440 if (return_code != ErrorNone) {440 if (return_code != ErrorNone) {
441 os_file_close(ch->manifest_file);441 os_file_close(&ch->manifest_file);
442 }442 }
443 return return_code;443 return return_code;
444 }444 }
...@@ -453,7 +453,7 @@ Error cache_add_file_fetch(CacheHash *ch, Buf *resolved_path, Buf *contents) {...@@ -453,7 +453,7 @@ Error cache_add_file_fetch(CacheHash *ch, Buf *resolved_path, Buf *contents) {
453 CacheHashFile *chf = ch->files.add_one();453 CacheHashFile *chf = ch->files.add_one();
454 chf->path = resolved_path;454 chf->path = resolved_path;
455 if ((err = populate_file_hash(ch, chf, contents))) {455 if ((err = populate_file_hash(ch, chf, contents))) {
456 os_file_close(ch->manifest_file);456 os_file_close(&ch->manifest_file);
457 return err;457 return err;
458 }458 }
459459
...@@ -586,6 +586,6 @@ void cache_release(CacheHash *ch) {...@@ -586,6 +586,6 @@ void cache_release(CacheHash *ch) {
586 }586 }
587 }587 }
588588
589 os_file_close(ch->manifest_file);589 os_file_close(&ch->manifest_file);
590}590}
591591
src/os.cpp+5-3
...@@ -2081,11 +2081,13 @@ Error os_file_overwrite(OsFile file, Buf *contents) {...@@ -2081,11 +2081,13 @@ Error os_file_overwrite(OsFile file, Buf *contents) {
2081#endif2081#endif
2082}2082}
20832083
2084void os_file_close(OsFile file) {2084void os_file_close(OsFile *file) {
2085#if defined(ZIG_OS_WINDOWS)2085#if defined(ZIG_OS_WINDOWS)
2086 CloseHandle(file);2086 CloseHandle(*file);
2087 *file = NULL;
2087#else2088#else
2088 close(file);2089 close(*file);
2090 *file = -1;
2089#endif2091#endif
2090}2092}
20912093
src/os.hpp+1-1
...@@ -121,7 +121,7 @@ Error ATTRIBUTE_MUST_USE os_file_open_lock_rw(Buf *full_path, OsFile *out_file);...@@ -121,7 +121,7 @@ Error ATTRIBUTE_MUST_USE os_file_open_lock_rw(Buf *full_path, OsFile *out_file);
121Error ATTRIBUTE_MUST_USE os_file_read(OsFile file, void *ptr, size_t *len);121Error ATTRIBUTE_MUST_USE os_file_read(OsFile file, void *ptr, size_t *len);
122Error ATTRIBUTE_MUST_USE os_file_read_all(OsFile file, Buf *contents);122Error ATTRIBUTE_MUST_USE os_file_read_all(OsFile file, Buf *contents);
123Error ATTRIBUTE_MUST_USE os_file_overwrite(OsFile file, Buf *contents);123Error ATTRIBUTE_MUST_USE os_file_overwrite(OsFile file, Buf *contents);
124void os_file_close(OsFile file);124void os_file_close(OsFile *file);
125125
126Error ATTRIBUTE_MUST_USE os_write_file(Buf *full_path, Buf *contents);126Error ATTRIBUTE_MUST_USE os_write_file(Buf *full_path, Buf *contents);
127Error ATTRIBUTE_MUST_USE os_copy_file(Buf *src_path, Buf *dest_path);127Error ATTRIBUTE_MUST_USE os_copy_file(Buf *src_path, Buf *dest_path);