From 493c3a127f64766ddc1bc05a297c2c53052bee62 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Wed, 4 Feb 2015 10:36:57 +0000 Subject: [PATCH] Avoid leakage of file descriptors in LLDB and LLGS Summary: Both LLDB and LLGS are leaking file descriptors into the debugged process. This plugs the leak by closing the unneeded descriptors. In one case I use O_CLOEXEC, which I hope is supported on relevant platforms. I also added a regression test and plugged a fd leak in dosep.py. Reviewers: vharron, clayborg Subscribers: lldb-commits Differential Revision: http://reviews.llvm.org/D7372 llvm-svn: 228130 --- .../Plugins/Process/Linux/NativeProcessLinux.cpp | 10 ++++++- .../Plugins/Process/Linux/ProcessMonitor.cpp | 10 ++++++- lldb/source/Target/ProcessLaunchInfo.cpp | 2 +- lldb/test/dosep.py | 4 +-- lldb/test/functionalities/avoids-fd-leak/Makefile | 5 ++++ .../functionalities/avoids-fd-leak/TestFdLeak.py | 32 ++++++++++++++++++++++ lldb/test/functionalities/avoids-fd-leak/main.c | 28 +++++++++++++++++++ 7 files changed, 86 insertions(+), 5 deletions(-) create mode 100644 lldb/test/functionalities/avoids-fd-leak/Makefile create mode 100644 lldb/test/functionalities/avoids-fd-leak/TestFdLeak.py create mode 100644 lldb/test/functionalities/avoids-fd-leak/main.c diff --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp index a933cd3..ccdbdac 100644 --- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp +++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp @@ -1545,6 +1545,11 @@ NativeProcessLinux::Launch(LaunchArgs *args) if (args->m_error.Fail()) exit(ePtraceFailed); + // terminal has already dupped the tty descriptors to stdin/out/err. + // This closes original fd from which they were copied (and avoids + // leaking descriptors to the debugged process. + terminal.CloseSlaveFileDescriptor(); + // Do not inherit setgid powers. if (setgid(getgid()) != 0) exit(eSetGidFailed); @@ -3532,7 +3537,10 @@ NativeProcessLinux::DupDescriptor(const char *path, int fd, int flags) if (target_fd == -1) return false; - return (dup2(target_fd, fd) == -1) ? false : true; + if (dup2(target_fd, fd) == -1) + return false; + + return (close(target_fd) == -1) ? false : true; } void diff --git a/lldb/source/Plugins/Process/Linux/ProcessMonitor.cpp b/lldb/source/Plugins/Process/Linux/ProcessMonitor.cpp index 43a1942..e87e662 100644 --- a/lldb/source/Plugins/Process/Linux/ProcessMonitor.cpp +++ b/lldb/source/Plugins/Process/Linux/ProcessMonitor.cpp @@ -1356,6 +1356,11 @@ ProcessMonitor::Launch(LaunchArgs *args) if (PTRACE(PTRACE_TRACEME, 0, NULL, NULL, 0) < 0) exit(ePtraceFailed); + // terminal has already dupped the tty descriptors to stdin/out/err. + // This closes original fd from which they were copied (and avoids + // leaking descriptors to the debugged process. + terminal.CloseSlaveFileDescriptor(); + // Do not inherit setgid powers. if (setgid(getgid()) != 0) exit(eSetGidFailed); @@ -2319,7 +2324,10 @@ ProcessMonitor::DupDescriptor(const char *path, int fd, int flags) if (target_fd == -1) return false; - return (dup2(target_fd, fd) == -1) ? false : true; + if (dup2(target_fd, fd) == -1) + return false; + + return (close(target_fd) == -1) ? false : true; } void diff --git a/lldb/source/Target/ProcessLaunchInfo.cpp b/lldb/source/Target/ProcessLaunchInfo.cpp index 451c42d..3f22a4e 100644 --- a/lldb/source/Target/ProcessLaunchInfo.cpp +++ b/lldb/source/Target/ProcessLaunchInfo.cpp @@ -344,7 +344,7 @@ ProcessLaunchInfo::FinalizeFileActions (Target *target, bool default_to_use_pty) log->Printf ("ProcessLaunchInfo::%s default_to_use_pty is set, and at least one stdin/stderr/stdout is unset, so generating a pty to use for it", __FUNCTION__); - if (m_pty->OpenFirstAvailableMaster(O_RDWR| O_NOCTTY, NULL, 0)) + if (m_pty->OpenFirstAvailableMaster(O_RDWR | O_NOCTTY | O_CLOEXEC, NULL, 0)) { const char *slave_path = m_pty->GetSlaveName(NULL, 0); diff --git a/lldb/test/dosep.py b/lldb/test/dosep.py index fc4b8ff..e397eab 100755 --- a/lldb/test/dosep.py +++ b/lldb/test/dosep.py @@ -57,8 +57,8 @@ def call_with_timeout(command, timeout): """Run command with a timeout if possible.""" if timeout_command and timeout != "0": return subprocess.call([timeout_command, timeout] + command, - stdin=subprocess.PIPE) - return (ePassed if subprocess.call(command, stdin=subprocess.PIPE) == 0 + stdin=subprocess.PIPE, close_fds=True) + return (ePassed if subprocess.call(command, stdin=subprocess.PIPE, close_fds=True) == 0 else eFailed) def process_dir(root, files, test_root, dotest_options): diff --git a/lldb/test/functionalities/avoids-fd-leak/Makefile b/lldb/test/functionalities/avoids-fd-leak/Makefile new file mode 100644 index 0000000..0d70f25 --- /dev/null +++ b/lldb/test/functionalities/avoids-fd-leak/Makefile @@ -0,0 +1,5 @@ +LEVEL = ../../make + +C_SOURCES := main.c + +include $(LEVEL)/Makefile.rules diff --git a/lldb/test/functionalities/avoids-fd-leak/TestFdLeak.py b/lldb/test/functionalities/avoids-fd-leak/TestFdLeak.py new file mode 100644 index 0000000..8e797bc --- /dev/null +++ b/lldb/test/functionalities/avoids-fd-leak/TestFdLeak.py @@ -0,0 +1,32 @@ +""" +Test whether a process started by lldb has no extra file descriptors open. +""" + +import os +import unittest2 +import lldb +from lldbtest import * +import lldbutil + +class AvoidsFdLeakTestCase(TestBase): + + mydir = TestBase.compute_mydir(__file__) + + @expectedFailureWindows("The check for descriptor leakage needs to be implemented differently") + def test_fd_leak (self): + self.buildDefault() + exe = os.path.join (os.getcwd(), "a.out") + + target = self.dbg.CreateTarget(exe) + + process = target.LaunchSimple (None, None, self.get_process_working_directory()) + self.assertTrue(process, PROCESS_IS_VALID) + + self.assertTrue(process.GetState() == lldb.eStateExited) + self.assertTrue(process.GetExitStatus() == 0) + +if __name__ == '__main__': + import atexit + lldb.SBDebugger.Initialize() + atexit.register(lambda: lldb.SBDebugger.Terminate()) + unittest2.main() diff --git a/lldb/test/functionalities/avoids-fd-leak/main.c b/lldb/test/functionalities/avoids-fd-leak/main.c new file mode 100644 index 0000000..1e5a008 --- /dev/null +++ b/lldb/test/functionalities/avoids-fd-leak/main.c @@ -0,0 +1,28 @@ +#include +#include +#include +#include +#include + +int +main (int argc, char const **argv) +{ + struct stat buf; + int i; + + // Make sure stdin/stdout/stderr exist. + for (i = 0; i <= 2; ++i) { + if (fstat(i, &buf) != 0) + return 1; + } + + // Make sure no other file descriptors are open. + for (i = 3; i <= 256; ++i) { + if (fstat(i, &buf) == 0 || errno != EBADF) { + fprintf(stderr, "File descriptor %d is open.\n", i); + return 2; + } + } + + return 0; +} -- 2.7.4