From: Mike Stepanek (mstepane) Date: Tue, 7 May 2019 17:25:19 +0000 (-0400) Subject: Merge pull request #1600 in SNORT/snort3 from ~DDAHIPHA/snort3:fd_leak_fixes to master X-Git-Tag: 3.0.0-256~7 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=51c6942a68a26069a8fa84877d465bde29d2994e;p=thirdparty%2Fsnort3.git Merge pull request #1600 in SNORT/snort3 from ~DDAHIPHA/snort3:fd_leak_fixes to master Squashed commit of the following: commit f6c664bc51a374308a82e13395cfb87f12621ef6 Author: Devendra Dahiphale Date: Tue May 7 13:23:18 2019 -0400 main: Fix File Descriptor leaks --- diff --git a/src/file_api/file_capture.cc b/src/file_api/file_capture.cc index 3df14e031..d58701b18 100644 --- a/src/file_api/file_capture.cc +++ b/src/file_api/file_capture.cc @@ -512,6 +512,7 @@ void FileCapture::store_file() // Get file from file buffer if (!buff || !size ) { + fclose(fh); return; } diff --git a/src/main/CMakeLists.txt b/src/main/CMakeLists.txt index fefd44e17..f522bdd44 100644 --- a/src/main/CMakeLists.txt +++ b/src/main/CMakeLists.txt @@ -16,6 +16,8 @@ if ( ENABLE_SHELL ) set ( SHELL_SOURCES control.cc control.h control_mgmt.cc control_mgmt.h ac_shell_cmd.h ac_shell_cmd.cc) endif ( ENABLE_SHELL ) +add_subdirectory(test) + add_library (main OBJECT analyzer.cc analyzer.h diff --git a/src/main/control.cc b/src/main/control.cc index 276edf2fa..d409f6551 100644 --- a/src/main/control.cc +++ b/src/main/control.cc @@ -62,7 +62,7 @@ void ControlConn::configure() const int ControlConn::shell_execute(int& current_fd, Request*& current_request) { if ( !request->read(fd) ) - return fd; + return -1; current_fd = fd; current_request = request; diff --git a/src/main/request.cc b/src/main/request.cc index 5168204c2..b6f7fa389 100644 --- a/src/main/request.cc +++ b/src/main/request.cc @@ -55,10 +55,7 @@ bool Request::read(int& f) } if ( n <= 0 and errno != EAGAIN and errno != EINTR ) - { - f = -1; return false; - } if ( bytes_read == sizeof(read_buf) ) bytes_read = 0; diff --git a/src/main/test/CMakeLists.txt b/src/main/test/CMakeLists.txt new file mode 100644 index 000000000..4c11cd6fa --- /dev/null +++ b/src/main/test/CMakeLists.txt @@ -0,0 +1,4 @@ +add_cpputest(request_test + SOURCES + ../request.cc +) diff --git a/src/main/test/request_test.cc b/src/main/test/request_test.cc new file mode 100644 index 000000000..f31e21859 --- /dev/null +++ b/src/main/test/request_test.cc @@ -0,0 +1,66 @@ +//-------------------------------------------------------------------------- +// Copyright (C) 2019-2019 Cisco and/or its affiliates. All rights reserved. +// +// This program is free software; you can redistribute it and/or modify it +// under the terms of the GNU General Public License Version 2 as published +// by the Free Software Foundation. You may not use, modify or distribute +// this program under any other version of the GNU General Public License. +// +// This program is distributed in the hope that it will be useful, but +// WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// General Public License for more details. +// +// You should have received a copy of the GNU General Public License along +// with this program; if not, write to the Free Software Foundation, Inc., +// 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +//-------------------------------------------------------------------------- +// request_test.cc author Devendra Dahiphale + +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif + +#include "main/request.h" + +#include +#include + +namespace snort +{ +void ErrorMessage(const char*,...) { } +void LogMessage(const char*,...) { } +} + +using namespace snort; + +//-------------------------------------------------------------------------- +// Request tests +//-------------------------------------------------------------------------- +TEST_GROUP(request_tests) +{}; + +//-------------------------------------------------------------------------- +// Make sure request->read does not modify value of passed-in fd +//-------------------------------------------------------------------------- +TEST(request_tests, request_read_fail_test) +{ + const int fd_orig_val = 10; + int current_fd = fd_orig_val; + + Request *request = new Request(current_fd); + + CHECK(request->read(current_fd) == false); + CHECK(current_fd == fd_orig_val); + + delete request; +} + +//------------------------------------------------------------------------- +// main +//------------------------------------------------------------------------- +int main(int argc, char** argv) +{ + return CommandLineTestRunner::RunAllTests(argc, argv); +} + diff --git a/src/utils/util.cc b/src/utils/util.cc index 316533c2e..1bc4a6493 100644 --- a/src/utils/util.cc +++ b/src/utils/util.cc @@ -284,6 +284,7 @@ void CreatePidFile(pid_t pid) } else { + fclose(pid_lockfile); const char* error = get_error(errno); ErrorMessage("Failed to create pid file %s, Error: %s\n", SnortConfig::get_conf()->pid_filename.c_str(), error); @@ -454,9 +455,16 @@ std::string read_infile(const char* key, const char* fname) int fd = open(fname, O_RDONLY); struct stat buf; + if (fd < 0) + { + ErrorMessage("Failed to open file: %s with error: %s", fname, get_error(errno)); + return ""; + } + if (fstat(fd, &buf) < 0) { ParseError("can't stat %s: %s", fname, get_error(errno)); + close(fd); return ""; } @@ -464,6 +472,7 @@ std::string read_infile(const char* key, const char* fname) if (!S_ISREG(buf.st_mode) ) { ParseError("not a regular file: %s", fname); + close(fd); return ""; } @@ -481,9 +490,10 @@ std::string read_infile(const char* key, const char* fname) else { ParseError("can't open file %s = %s: %s", key, fname, get_error(errno)); + close(fd); return ""; } - + close(fd); return line; }