Add test for denied process attach by pid and fix found bugs in Process/ProcessPOSIX.cpp

[ovyalov]

Summary

New test test/functionalities/process_attach/attach_denied covers the case when attach is denied by potential inferior.
This test uncovered next issues that I fixed along as well:

• lldbtest calls SBProcess::Kill for every inferior within test's tearDown. If attach fails ProcessPOSIX::DoDestroy calls m_monitor->Kill() which sends SIGKILL to pid 0, i.e. killing all processes in the current process group. Added check for invalid pid in ProcessPOSIX::DoDestroy
• Modified ProcessPOSIX::DoDestroy made test to fail since Kill returns an error. Changed Target/Process.cpp in order to transition process to exited state in case of attach failure - so, DoDestroy exits properly since HasExited() returns true.

[ovyalov]

Removed unused imports from TestAttachDenied.py.

[ovyalov]

Hi Ed,

since you monitor POSIX code changes let me add you as reviewer.

[ovyalov]

• Added synchronization between test and inferior via named pipe.
• Restricted test run conditions - either Darwin or Linux.

Please take a look.

[emaste]

For future would you provide full context, i.e. diff -U999999 if manually uploading diffs?

This looks OK to me, but as I think we lack an appropriate interface in FreeBSD right now I can't test, so hope one of the Linux or Darwin developers can comment.

[ovyalov]

Thanks for suggestion - re-did patch with full context.

Yes, I tried to run the test on FreeBSD but prctl isn't available - so, for now BSD will be skipped.

[emaste]

Some of my FreeBSD colleagues recommended trigger the failure by trying to attach to a process that already has a debugger attached, as that's the common cause of the failure we're handling here anyway.

Perhaps attach_denied/main.c can fork(), PT_ATTACH to the child, and then return the child's pid for LLDB to try to attach.

[ovyalov]

I reworked test according to Ed's suggestion with fork to make the test to cover OSX, Linux and FreeBSD.
Along the way FreeBSD/ProcessMonitor bug was discovered which led LLDB to hang when Attach fails - fixed this.
Checked against OSX 10.9.5, Ubuntu 14.0.4 and FreeBSD 10 - no regressions found.

Please take another look.

[ovyalov]

Removed unneeded file from patch.

[emaste]

overall lgtm

source/Plugins/Process/FreeBSD/ProcessMonitor.cpp

1143 ↗

(On Diff #16362)

we probably ought to clean this up to just return; now - the goto is just extra obfuscation

test/functionalities/process_attach/attach_denied/main.cpp

60 ↗

(On Diff #16362)

just 1 probably. -ve return values from main end up with implementation-specific results.

[ovyalov]

Fixed according to your suggestions.

Thank you for the review!

[emaste]

LGTM.

[clayborg]

lgtm

[emaste]

ping?

[emaste]

rL222372

source/Plugins/Process/POSIX/ProcessPOSIX.cpp

 ProcessPOSIX::DoDestroy()
 {
     Error error;
 
     if (!HasExited())
     {
         assert(m_monitor);
         m_exit_now = true;
+        if (GetID() == LLDB_INVALID_PROCESS_ID)
+        {
+            error.SetErrorString("invalid process id");
+            return error;
+        }
         if (!m_monitor->Kill())
         {
             error.SetErrorToErrno();
             return error;
         }
 
         SetPrivateState(eStateExited);
     }

source/Target/Process.cpp

if (attach_pid != LLDB_INVALID_PROCESS_ID)
             {
                 
                 SetNextEventAction(new Process::AttachCompletionHandler(this, attach_info.GetResumeCount()));
                 StartPrivateStateThread();
             }
             else
             {
                 if (GetID() != LLDB_INVALID_PROCESS_ID)
-                {
                     SetID (LLDB_INVALID_PROCESS_ID);
-                    const char *error_string = error.AsCString();
-                    if (error_string == NULL)
-                        error_string = "attach failed";
+
 
-                    SetExitStatus(-1, error_string);
-                }
-            }
-        }
-    }
     return error;
 }
 
 void
 Process::CompleteAttach ()
 {
     Log *log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS));
     if (log)

test/functionalities/process_attach/attach_denied/Makefile

+LEVEL = ../../../make
+
+C_SOURCES := main.c
+
+EXE := AttachDenied
+
+include $(LEVEL)/Makefile.rules

test/functionalities/process_attach/attach_denied/TestAttachDenied.py

+"""
+Test denied process attach.
+"""
+
+import os
+import shutil
+import sys
+import tempfile
+import unittest2
+import lldb
+from lldbtest import *
+
+exe_name = 'AttachDenied'  # Must match Makefile
+
+class AttachDeniedTestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    @unittest2.skipUnless(sys.platform.startswith("darwin") or sys.platform.startswith("linux"),
+                          "requires Darwin or Linux")
+    def test_attach_to_process_by_id_denied(self):
+        """Test attach by process id denied"""
+
+        self.buildDefault()
+        exe = os.path.join(os.getcwd(), exe_name)
+
+        temp_dir = tempfile.mkdtemp()
+        self.addTearDownHook(lambda: shutil.rmtree(temp_dir))
+
+        # Use named pipe as a synchronization point between test and inferior.
+        pipe_path = os.path.join(temp_dir, "attach_denied_pipe")
+        os.mkfifo(pipe_path)
+
+        # Spawn a new process
+        popen = self.spawnSubprocess(exe, [pipe_path])
+        self.addTearDownHook(self.cleanupSubprocesses)
+
+        sync_pipe = open(pipe_path, 'r')
+        self.addTearDownHook(lambda: sync_pipe.close())
+        sync_pipe.read()
+
+        self.expect('process attach -p ' + str(popen.pid),
+                    startstr = 'error: attach failed:',
+                    error = True)
+
+
+if __name__ == '__main__':
+    import atexit
+    lldb.SBDebugger.Initialize()
+    atexit.register(lambda: lldb.SBDebugger.Terminate())
+    unittest2.main()

test/functionalities/process_attach/attach_denied/main.c

+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <sys/types.h>
+#include <sys/stat.h>
+
+#if defined(__linux__)
+#include <sys/prctl.h>
+#elif defined(__APPLE__)
+#include <sys/ptrace.h>
+#endif
+
+int main(int argc, char const *argv[])
+{
+#if defined(__linux__)
+#if defined(PR_SET_DUMPABLE)
+    if (prctl(PR_SET_DUMPABLE, 0, 0, 0, 0) == -1)
+    {
+        char* err_msg = strerror(errno);
+        fprintf(stderr, "prctl(PR_SET_DUMPABLE) failed: %s\n", err_msg);
+        return -1;
+    }
+#endif
+#elif defined(__APPLE__)
+    if (ptrace(PT_DENY_ATTACH, 0, NULL, 0) == -1)
+    {
+        char* err_msg = strerror(errno);
+        fprintf(stderr, "ptrace(PT_DENY_ATTACH) failed: %s\n", err_msg);
+        return -1;
+    }
+#else
+#error "Unsupported platform"
+#endif
+
+    if (argc < 2)
+    {
+        fprintf(stderr, "invalid number of command line arguments\n");
+        return -1;
+    }
+
+    int fd = 0;
+    fd = open(argv[1], O_WRONLY);
+    if (fd == -1)
+    {
+        char* err_msg = strerror(errno);
+        fprintf(stderr, "open(%s) failed: %s\n", argv[1], err_msg);
+    }
+
+    const char* msg = "1";
+    if (write(fd, msg, strlen(msg)) == -1)
+    {
+        char* err_msg = strerror(errno);
+        fprintf(stderr, "write(%s) failed: %s\n", msg, err_msg);
+    }
+    close(fd);
+
+    // Waiting to be attempted to attach by the debugger.
+    sleep(30);
+
+    printf("Exiting now\n");
+    return 0;
+}