From fc33bf2014704fe0ae512b76dae64de7fc5e83ac Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Mon, 18 Sep 2017 14:43:42 -0700 Subject: [PATCH] Make strerror_r portable. This uses the same code that broker already uses to determine if we use the XSI or gnu version of strerror_r. Patch by Thomas Petersen. --- CHANGES | 18 +++++++++++++ VERSION | 2 +- src/File.cc | 8 +++--- src/Flare.cc | 2 +- src/Pipe.cc | 2 +- src/PolicyFile.cc | 2 +- src/analyzer/protocol/tcp/TCP_Endpoint.cc | 2 +- src/file_analysis/analyzer/extract/Extract.cc | 2 +- src/input/readers/raw/Raw.cc | 6 ++--- src/logging/writers/ascii/Ascii.cc | 2 +- src/threading/BasicThread.cc | 2 +- src/util.cc | 27 ++++++++++++++++--- src/util.h | 6 +++++ 13 files changed, 63 insertions(+), 18 deletions(-) diff --git a/CHANGES b/CHANGES index 3e2b575dd6..71be3f743b 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,22 @@ +2.5-306 | 2017-09-18 14:43:42 -0700 + + * Make strerror_r portable, supporting XSI/gnu versions. (Thomas Petersen) + + * Prevent crash when calling bro -U. (Thomas Petersen) + + * Remove annoying error message from connsize bifs. (Johanna Amann) + + * Add test to verify that log rotation works with gzipped logs (Daniel Thayer) + + * Fix ascii writer to not discard a ".gz" file extension. (Daniel Thayer) + + When Bro writes a compressed log, it uses a file extension of ".gz". + However, upon log rotation the ascii writer script function + "default_rotation_postprocessor_func" was discarding the ".gz" + file extension. Fixed so that the correct file extension is + preserved after rotation. (Daniel Thayer) + 2.5-297 | 2017-09-11 09:26:33 -0700 * Fix small OCSP parser bug; serial numbers were not passed to events diff --git a/VERSION b/VERSION index a5ed34e608..e018451797 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.5-297 +2.5-306 diff --git a/src/File.cc b/src/File.cc index 7c4a21d5e8..e0e0d63332 100644 --- a/src/File.cc +++ b/src/File.cc @@ -302,7 +302,7 @@ FILE* BroFile::BringIntoCache() if ( ! f ) { - strerror_r(errno, buf, sizeof(buf)); + bro_strerror_r(errno, buf, sizeof(buf)); reporter->Error("can't open %s: %s", name, buf); f = fopen("/dev/null", "w"); @@ -313,7 +313,7 @@ FILE* BroFile::BringIntoCache() return f; } - strerror_r(errno, buf, sizeof(buf)); + bro_strerror_r(errno, buf, sizeof(buf)); reporter->Error("can't open /dev/null: %s", buf); return 0; } @@ -323,7 +323,7 @@ FILE* BroFile::BringIntoCache() if ( fseek(f, position, SEEK_SET) < 0 ) { - strerror_r(errno, buf, sizeof(buf)); + bro_strerror_r(errno, buf, sizeof(buf)); reporter->Error("reopen seek failed: %s", buf); } @@ -413,7 +413,7 @@ void BroFile::Suspend() if ( (position = ftell(f)) < 0 ) { char buf[256]; - strerror_r(errno, buf, sizeof(buf)); + bro_strerror_r(errno, buf, sizeof(buf)); reporter->Error("ftell failed: %s", buf); position = 0; } diff --git a/src/Flare.cc b/src/Flare.cc index 5df6d663aa..87dc946955 100644 --- a/src/Flare.cc +++ b/src/Flare.cc @@ -16,7 +16,7 @@ Flare::Flare() static void bad_pipe_op(const char* which) { char buf[256]; - strerror_r(errno, buf, sizeof(buf)); + bro_strerror_r(errno, buf, sizeof(buf)); reporter->FatalErrorWithCore("unexpected pipe %s failure: %s", which, buf); } diff --git a/src/Pipe.cc b/src/Pipe.cc index 3f60409fdb..3775ca705d 100644 --- a/src/Pipe.cc +++ b/src/Pipe.cc @@ -12,7 +12,7 @@ using namespace bro; static void pipe_fail(int eno) { char tmp[256]; - strerror_r(eno, tmp, sizeof(tmp)); + bro_strerror_r(eno, tmp, sizeof(tmp)); reporter->FatalError("Pipe failure: %s", tmp); } diff --git a/src/PolicyFile.cc b/src/PolicyFile.cc index bd41c15e9d..22f09e6970 100644 --- a/src/PolicyFile.cc +++ b/src/PolicyFile.cc @@ -84,7 +84,7 @@ bool LoadPolicyFileText(const char* policy_filename) if ( fstat(fileno(f), &st) != 0 ) { char buf[256]; - strerror_r(errno, buf, sizeof(buf)); + bro_strerror_r(errno, buf, sizeof(buf)); reporter->Error("fstat failed on %s: %s", policy_filename, buf); fclose(f); return false; diff --git a/src/analyzer/protocol/tcp/TCP_Endpoint.cc b/src/analyzer/protocol/tcp/TCP_Endpoint.cc index 7c359623f3..c3175ec9f5 100644 --- a/src/analyzer/protocol/tcp/TCP_Endpoint.cc +++ b/src/analyzer/protocol/tcp/TCP_Endpoint.cc @@ -229,7 +229,7 @@ int TCP_Endpoint::DataSent(double t, uint64 seq, int len, int caplen, if ( fwrite(data, 1, len, f) < unsigned(len) ) { char buf[256]; - strerror_r(errno, buf, sizeof(buf)); + bro_strerror_r(errno, buf, sizeof(buf)); reporter->Error("TCP contents write failed: %s", buf); if ( contents_file_write_failure ) diff --git a/src/file_analysis/analyzer/extract/Extract.cc b/src/file_analysis/analyzer/extract/Extract.cc index c758414a6e..f936a5156b 100644 --- a/src/file_analysis/analyzer/extract/Extract.cc +++ b/src/file_analysis/analyzer/extract/Extract.cc @@ -20,7 +20,7 @@ Extract::Extract(RecordVal* args, File* file, const string& arg_filename, { fd = 0; char buf[128]; - strerror_r(errno, buf, sizeof(buf)); + bro_strerror_r(errno, buf, sizeof(buf)); reporter->Error("cannot open %s: %s", filename.c_str(), buf); } } diff --git a/src/input/readers/raw/Raw.cc b/src/input/readers/raw/Raw.cc index ae1f0939a8..27d8b0c685 100644 --- a/src/input/readers/raw/Raw.cc +++ b/src/input/readers/raw/Raw.cc @@ -90,7 +90,7 @@ bool Raw::SetFDFlags(int fd, int cmd, int flags) return true; char buf[256]; - strerror_r(errno, buf, sizeof(buf)); + bro_strerror_r(errno, buf, sizeof(buf)); Error(Fmt("failed to set fd flags: %s", buf)); return false; } @@ -197,7 +197,7 @@ bool Raw::Execute() else { char buf[256]; - strerror_r(errno, buf, sizeof(buf)); + bro_strerror_r(errno, buf, sizeof(buf)); Warning(Fmt("Could not set child process group: %s", buf)); } } @@ -293,7 +293,7 @@ bool Raw::OpenInput() if ( fseek(file.get(), pos, whence) < 0 ) { char buf[256]; - strerror_r(errno, buf, sizeof(buf)); + bro_strerror_r(errno, buf, sizeof(buf)); Error(Fmt("Seek failed in init: %s", buf)); } } diff --git a/src/logging/writers/ascii/Ascii.cc b/src/logging/writers/ascii/Ascii.cc index dec1689df4..baaba22665 100644 --- a/src/logging/writers/ascii/Ascii.cc +++ b/src/logging/writers/ascii/Ascii.cc @@ -414,7 +414,7 @@ bool Ascii::DoRotate(const char* rotated_path, double open, double close, bool t if ( rename(fname.c_str(), nname.c_str()) != 0 ) { char buf[256]; - strerror_r(errno, buf, sizeof(buf)); + bro_strerror_r(errno, buf, sizeof(buf)); Error(Fmt("failed to rename %s to %s: %s", fname.c_str(), nname.c_str(), buf)); FinishedRotation(); diff --git a/src/threading/BasicThread.cc b/src/threading/BasicThread.cc index d63b307470..3b6f5d6532 100644 --- a/src/threading/BasicThread.cc +++ b/src/threading/BasicThread.cc @@ -98,7 +98,7 @@ const char* BasicThread::Strerror(int err) if ( ! strerr_buffer ) strerr_buffer = new char[256]; - strerror_r(err, strerr_buffer, 256); + bro_strerror_r(err, strerr_buffer, 256); return strerr_buffer; } diff --git a/src/util.cc b/src/util.cc index a035da1739..a2f0cb8c94 100644 --- a/src/util.cc +++ b/src/util.cc @@ -1012,7 +1012,7 @@ FILE* open_file(const string& path, const string& mode) if ( ! rval ) { char buf[256]; - strerror_r(errno, buf, sizeof(buf)); + bro_strerror_r(errno, buf, sizeof(buf)); reporter->Error("Failed to open file %s: %s", filename, buf); } @@ -1396,7 +1396,7 @@ void _set_processing_status(const char* status) if ( fd < 0 ) { char buf[256]; - strerror_r(errno, buf, sizeof(buf)); + bro_strerror_r(errno, buf, sizeof(buf)); if ( reporter ) reporter->Error("Failed to open process status file '%s': %s", proc_status_file, buf); @@ -1616,7 +1616,7 @@ void safe_close(int fd) if ( close(fd) < 0 && errno != EINTR ) { char buf[128]; - strerror_r(errno, buf, sizeof(buf)); + bro_strerror_r(errno, buf, sizeof(buf)); fprintf(stderr, "safe_close error %d: %s\n", errno, buf); abort(); } @@ -1749,3 +1749,24 @@ std::string canonify_name(const std::string& name) return nname; } + +static void strerror_r_helper(char* result, char* buf, size_t buflen) + { + // Seems the GNU flavor of strerror_r may return a pointer to a static + // string. So try to copy as much as possible into desired buffer. + auto len = strlen(result); + strncpy(buf, result, buflen); + + if ( len >= buflen ) + buf[buflen - 1] = 0; + } + +static void strerror_r_helper(int result, char* buf, size_t buflen) + { /* XSI flavor of strerror_r, no-op. */ } + +void bro_strerror_r(int bro_errno, char* buf, size_t buflen) + { + auto res = strerror_r(bro_errno, buf, buflen); + // GNU vs. XSI flavors make it harder to use strerror_r. + strerror_r_helper(res, buf, buflen); + } diff --git a/src/util.h b/src/util.h index a2c1b78db3..30ef8a61da 100644 --- a/src/util.h +++ b/src/util.h @@ -516,4 +516,10 @@ struct CompareString */ std::string canonify_name(const std::string& name); +/** + * Reentrant version of strerror(). Takes care of the difference between the + * XSI-compliant and the GNU-specific version of strerror_r(). + */ +void bro_strerror_r(int bro_errno, char* buf, size_t buflen); + #endif