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 <ddahipha@cisco.com>
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 3df14e031cb48b6c61e776d61cce610487ee0704..d58701b18ecf97a60bc89435700d88092a807457 100644 (file)
--- 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 fefd44e1782d8cf73a79b00f2115867fb3191b5b..f522bdd44ca4e28b9199ffae28a771447d06c306 100644 (file)
--- 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 276edf2faa0dc88fc4ac8a0e48c1a243560f2ee8..d409f65510f934f685a7cb76056f2a6a9eb1b86c 100644 (file)
--- 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 5168204c20d781ad38fdddf1507aef09cb4a1137..b6f7fa389901e7482c014404d290f756b4c8ec81 100644 (file)
--- 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 (file)
index 0000000..4c11cd6
--- /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 (file)
index 0000000..f31e218
--- /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 <ddahipha@cisco.com>
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include "main/request.h"
+
+#include <CppUTest/CommandLineTestRunner.h>
+#include <CppUTest/TestHarness.h>
+
+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 316533c2eacfedc411b3f088b83f4e0f6e922971..1bc4a6493776e5af1371d30d384b4c6a50e09845 100644 (file)
--- 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;
 }