From 519e6c5ed246a57b1c94d07d7db2f44576ffb3e3 Mon Sep 17 00:00:00 2001 From: Numan Siddique Date: Wed, 19 Nov 2014 22:39:45 +0530 Subject: [PATCH] Close all the fds of the child before exec This patch closes all the fds (starting from 3) of the child before exec in InstanceTask. Please see the bug description for more details. Change-Id: I9f18f88c9c58540ad6358a2a989f344f60be86d8 Closes-bug: #1394300 --- src/vnsw/agent/cmn/agent_cmn.h | 8 ++ src/vnsw/agent/oper/instance_task.cc | 2 + src/vnsw/agent/oper/instance_task.h | 1 + src/vnsw/agent/oper/test/SConscript | 1 + .../agent/oper/test/test_instance_task.cc | 94 +++++++++++++++++++ src/vnsw/agent/uve/vm_stat.cc | 4 + 6 files changed, 110 insertions(+) create mode 100644 src/vnsw/agent/oper/test/test_instance_task.cc diff --git a/src/vnsw/agent/cmn/agent_cmn.h b/src/vnsw/agent/cmn/agent_cmn.h index c54b4664804..ecad648ba26 100644 --- a/src/vnsw/agent/cmn/agent_cmn.h +++ b/src/vnsw/agent/cmn/agent_cmn.h @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -73,6 +74,13 @@ static inline void CfgUuidSet(uint64_t ms_long, uint64_t ls_long, } } +static inline void CloseTaskFds(void) { + int max_open_fds = sysconf(_SC_OPEN_MAX); + int fd; + for(fd = 3; fd < max_open_fds; fd++) + close(fd); +} + extern SandeshTraceBufferPtr OperDBTraceBuf; extern bool GetBuildInfo(std::string &build_info_str); diff --git a/src/vnsw/agent/oper/instance_task.cc b/src/vnsw/agent/oper/instance_task.cc index ede60d9a99a..ec62122457a 100644 --- a/src/vnsw/agent/oper/instance_task.cc +++ b/src/vnsw/agent/oper/instance_task.cc @@ -93,6 +93,8 @@ pid_t InstanceTask::Run() { close(STDOUT_FILENO); close(STDIN_FILENO); + /* Close all the open fds before execvp */ + CloseTaskFds(); execvp(c_argv[0], (char **) c_argv.data()); perror("execvp"); diff --git a/src/vnsw/agent/oper/instance_task.h b/src/vnsw/agent/oper/instance_task.h index 03da27bbc2e..1a659cba929 100644 --- a/src/vnsw/agent/oper/instance_task.h +++ b/src/vnsw/agent/oper/instance_task.h @@ -9,6 +9,7 @@ #include "base/timer.h" #include "base/queue_task.h" #include "cmn/agent_signal.h" +#include "cmn/agent_cmn.h" #include "db/db_entry.h" class EventManager; diff --git a/src/vnsw/agent/oper/test/SConscript b/src/vnsw/agent/oper/test/SConscript index b4da801ea09..4b8fa2614bd 100644 --- a/src/vnsw/agent/oper/test/SConscript +++ b/src/vnsw/agent/oper/test/SConscript @@ -22,6 +22,7 @@ test_fabric_interface = AgentEnv.MakeTestCmd(env, 'test_fabric_interface', oper_flaky_test_suite) test_aap = AgentEnv.MakeTestCmd(env, 'test_aap', oper_flaky_test_suite) test_ipv6 = AgentEnv.MakeTestCmd(env, 'test_ipv6', oper_test_suite) +test_instance_task = AgentEnv.MakeTestCmd(env, 'test_instance_task', oper_test_suite) env.Alias('agent:test_ipv6', test_ipv6) env.Append(LIBPATH = [ diff --git a/src/vnsw/agent/oper/test/test_instance_task.cc b/src/vnsw/agent/oper/test/test_instance_task.cc new file mode 100644 index 00000000000..55acd59419f --- /dev/null +++ b/src/vnsw/agent/oper/test/test_instance_task.cc @@ -0,0 +1,94 @@ +#include "oper/instance_task.h" +#include "testing/gunit.h" + +#include +#include +#include +#include + +#include +#include + +#include "io/event_manager.h" +#include "base/logging.h" +#include "base/os.h" +#include "base/test/task_test_util.h" + +using namespace std; +using namespace boost::filesystem; + +class InstanceTaskFdTest : public ::testing::Test { +public: + virtual void SetUp() { + memset(tmpfilename, 0, sizeof(tmpfilename)); + strcpy(tmpfilename, "task_fd_XXXXXX"); + mkstemp(tmpfilename); + std::fstream testfile(tmpfilename); + testfile << "while true; do sleep 1; done"; + testfile.close(); + std::stringstream cmd_str; + cmd_str << "/bin/sh" << " " <Run(); + return task_pid; + } + + int StopTask() { + task_->Stop(); + close(pipe_fds[0]); + close(pipe_fds[1]); + close(sock_fd); + } + + int GetTaskFds() { + std::stringstream proc_path; + int no_of_fds; + proc_path << "/proc/" <(is_other), + bind( &directory_entry::path, _1))); + return no_of_fds; + } + +protected: + int pipe_fds[2]; + int sock_fd; + InstanceTask *task_; + int task_pid; + EventManager evm; + char tmpfilename[L_tmpnam]; +}; + +TEST_F(InstanceTaskFdTest, TestCloseTaskFds) { + task_pid = StartTask(); + EXPECT_TRUE(task_pid > 0); + int task_open_fds = GetTaskFds(); + StopTask(); + /* The task should have only 1 opened fd (i.e fd 2) */ + EXPECT_EQ(1U, task_open_fds); +}; + +int main(int argc, char **argv) { + LoggingInit(); + ::testing::InitGoogleTest(&argc, argv); + + int result = RUN_ALL_TESTS(); + return result; +} diff --git a/src/vnsw/agent/uve/vm_stat.cc b/src/vnsw/agent/uve/vm_stat.cc index 8c5dd7189f3..49bb5ad3507 100644 --- a/src/vnsw/agent/uve/vm_stat.cc +++ b/src/vnsw/agent/uve/vm_stat.cc @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -87,6 +88,9 @@ void VmStat::ExecCmd(std::string cmd, DoneCb cb) { dup2(out[1], STDOUT_FILENO); //Close out[1] as stdout is a exact replica of out[1] close(out[1]); + + /* Close all the open fds before execvp */ + CloseTaskFds(); execvp(argv[0], argv); perror("execvp"); exit(127);