Author: labath Date: Wed Feb 4 04:36:57 2015 New Revision: 228130 URL: http://llvm.org/viewvc/llvm-project?rev=228130&view=rev Log: 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 Added: lldb/trunk/test/functionalities/avoids-fd-leak/ lldb/trunk/test/functionalities/avoids-fd-leak/Makefile lldb/trunk/test/functionalities/avoids-fd-leak/TestFdLeak.py lldb/trunk/test/functionalities/avoids-fd-leak/main.c Modified: lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.cpp lldb/trunk/source/Target/ProcessLaunchInfo.cpp lldb/trunk/test/dosep.py Modified: lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp?rev=228130&r1=228129&r2=228130&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp (original) +++ lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp Wed Feb 4 04:36:57 2015 @@ -1545,6 +1545,11 @@ NativeProcessLinux::Launch(LaunchArgs *a 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 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 Modified: lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.cpp?rev=228130&r1=228129&r2=228130&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.cpp (original) +++ lldb/trunk/source/Plugins/Process/Linux/ProcessMonitor.cpp Wed Feb 4 04:36:57 2015 @@ -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 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 Modified: lldb/trunk/source/Target/ProcessLaunchInfo.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/ProcessLaunchInfo.cpp?rev=228130&r1=228129&r2=228130&view=diff ============================================================================== --- lldb/trunk/source/Target/ProcessLaunchInfo.cpp (original) +++ lldb/trunk/source/Target/ProcessLaunchInfo.cpp Wed Feb 4 04:36:57 2015 @@ -344,7 +344,7 @@ ProcessLaunchInfo::FinalizeFileActions ( 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); Modified: lldb/trunk/test/dosep.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/dosep.py?rev=228130&r1=228129&r2=228130&view=diff ============================================================================== --- lldb/trunk/test/dosep.py (original) +++ lldb/trunk/test/dosep.py Wed Feb 4 04:36:57 2015 @@ -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): Added: lldb/trunk/test/functionalities/avoids-fd-leak/Makefile URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/functionalities/avoids-fd-leak/Makefile?rev=228130&view=auto ============================================================================== --- lldb/trunk/test/functionalities/avoids-fd-leak/Makefile (added) +++ lldb/trunk/test/functionalities/avoids-fd-leak/Makefile Wed Feb 4 04:36:57 2015 @@ -0,0 +1,5 @@ +LEVEL = ../../make + +C_SOURCES := main.c + +include $(LEVEL)/Makefile.rules Added: lldb/trunk/test/functionalities/avoids-fd-leak/TestFdLeak.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/functionalities/avoids-fd-leak/TestFdLeak.py?rev=228130&view=auto ============================================================================== --- lldb/trunk/test/functionalities/avoids-fd-leak/TestFdLeak.py (added) +++ lldb/trunk/test/functionalities/avoids-fd-leak/TestFdLeak.py Wed Feb 4 04:36:57 2015 @@ -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() Added: lldb/trunk/test/functionalities/avoids-fd-leak/main.c URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/functionalities/avoids-fd-leak/main.c?rev=228130&view=auto ============================================================================== --- lldb/trunk/test/functionalities/avoids-fd-leak/main.c (added) +++ lldb/trunk/test/functionalities/avoids-fd-leak/main.c Wed Feb 4 04:36:57 2015 @@ -0,0 +1,28 @@ +#include <sys/types.h> +#include <sys/stat.h> +#include <unistd.h> +#include <errno.h> +#include <stdio.h> + +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; +} _______________________________________________ lldb-commits mailing list lldb-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
