Add additional debug information in prepare_app

In case a thread is not receiving signal to change its Smack label
& capabilities, additional debugging information is required to
check what has happened.

Printing the debugging information is followed by an explicit
abort() so that the app candidate process can be debugged
to know what was happening inside each of the threads.

Statuses available to inspect each thread:
* thread dead during attempt to send signal
        -> NOT an issue, but info is printed with TID as it can be useful
* thread not synced
        -> thread was on the list when signal sending attempt was made,
           but didn't seem to have received signal
* thread received signal
        -> thread did receive signal, but failed at setting Smack label
* thread changed Smack label
        -> it did receive signal, did change Smack label but cannot change caps
* thread fully synced
        -> all went well - information is printed just to help debugging

Dump of status of each thread like above is printed ONLY if offending
thread is found that didn't seem to process signal handler correctly.

Change-Id: Ia1a560fb4baffadc354a403e60d1ab81d8828c42

diff --git a/src/client/client-security-manager.cpp b/src/client/client-security-manager.cpp
index 9df9d91ec92d346fdd4af672ae0547fdb9cb4dd0..8c5dded6a7724e7f5c28ec527be2f9d75fa601db 100644 (file)
--- a/src/client/client-security-manager.cpp
+++ b/src/client/client-security-manager.cpp
@@ -93,6 +93,21 @@ static const std::string *g_p_app_label;
 static std::atomic<int> g_threads_count;
 static cap_t g_cap;
 
+// needed to keep track which thread did receive signal and which did not
+// additionally keeps information about stage to which each thread got
+#define TID_STATUS_NOT_SYNCED           0
+#define TID_STATUS_RECEIVED_SIGNAL      1
+#define TID_STATUS_SETUP_SMACK          2
+#define TID_STATUS_FULLY_SYNCED         3
+#define TID_STATUS_DEAD                 4
+
+struct TidStatus {
+    int tid;
+    int status;
+};
+
+static TidStatus *g_tid_status;
+
 #define MAX_SIG_WAIT_TIME   1000
 
 // Hackish, based on glibc's definition in sysdeps/unix/sysv/linux/nptl-signals.h
@@ -577,9 +592,8 @@ inline static int sigaction(int signum, const struct sigaction *act, struct siga
 
 } // namespace Syscall
 
-inline static int label_for_self_internal()
+inline static int label_for_self_internal(int tid)
 {
-    const auto tid = Syscall::gettid();
     char threadSelf[sizeof "/proc//attr/current" + 1 + std::numeric_limits<unsigned>::digits10];
     sprintf(threadSelf, "/proc/%u/attr/current", unsigned(tid));
     int fd = open(threadSelf, O_WRONLY);
@@ -594,6 +608,7 @@ inline static int label_for_self_internal()
 static inline int security_manager_sync_threads_internal(const std::string &app_label, std::vector<int> &synced_tids)
 {
     static_assert(ATOMIC_INT_LOCK_FREE == 2, "std::atomic<int> is not always lock free");
+    g_tid_status = NULL;
 
     FS::FileNameVector files = FS::getSubDirectoriesFromDirectory("/proc/self/task", true);
 
@@ -609,12 +624,22 @@ static inline int security_manager_sync_threads_internal(const std::string &app_
 
     LogDebug("number of threads to sync: " << files.size() - 1);
 
-    const pid_t cur_tid = Syscall::gettid();
+    const int cur_tid = Syscall::gettid();
     synced_tids.push_back(cur_tid);
 
     if (files.size() > 1) {
         const pid_t cur_pid = getpid();
         g_threads_count = files.size() - 1;
+        g_tid_status = new TidStatus[g_threads_count];
+        if (!g_tid_status) {
+            LogError("Cannot allocate g_tid_status");
+            return SECURITY_MANAGER_ERROR_MEMORY;
+        } else {
+            for (int i = 0; i < g_threads_count; ++i) {
+                g_tid_status[i].tid = 0;    // to be sure its not yet any actual TID
+                g_tid_status[i].status = TID_STATUS_NOT_SYNCED;
+            }
+        }
 
         struct sigaction act;
         struct sigaction old;
@@ -624,16 +649,24 @@ static inline int security_manager_sync_threads_internal(const std::string &app_
         act.sa_flags = SA_RESTART;
         act.sa_handler = [](int signo) {
             (void)signo;
-
             std::atomic_thread_fence(std::memory_order_acquire);
 
+            int tid = Syscall::gettid();
+            int i = 0;
+            for (; i < g_threads_count; ++i) {
+                if (g_tid_status[i].tid == tid) {
+                    break;
+                }
+            }
+            g_tid_status[i].status = TID_STATUS_RECEIVED_SIGNAL;
             if (g_p_app_label)
-                if (label_for_self_internal() != 0)
+                if (label_for_self_internal(tid) != 0)
                     return;
-
+            g_tid_status[i].status = TID_STATUS_SETUP_SMACK;
             if (cap_set_proc(g_cap))
                 return;
 
+            g_tid_status[i].status = TID_STATUS_FULLY_SYNCED;
             g_threads_count--;
         };
 
@@ -645,17 +678,21 @@ static inline int security_manager_sync_threads_internal(const std::string &app_
         std::atomic_thread_fence(std::memory_order_release);
 
         int threads_gone = 0;
+        int tid_index = 0;
         for (auto const &e : files) {
             const int tid = atoi(e.c_str());
 
-            if (tid == static_cast<int>(cur_tid))
+            if (tid == cur_tid)
                 continue;
             synced_tids.push_back(tid); // this will not add current TID (but its already added)
+            g_tid_status[tid_index++].tid = tid;
+
             if (Syscall::tgkill(cur_pid, tid, SIGSETXID) < 0) {
                 const auto err = errno;
-                if (ESRCH == err) // thread already gone
+                if (ESRCH == err) { // thread already gone
                     threads_gone++;
-                else
+                    g_tid_status[tid_index-1].status = TID_STATUS_DEAD;
+                } else
                     LogWithErrno(err, "tgkill()");
 
                 continue;
@@ -672,11 +709,36 @@ static inline int security_manager_sync_threads_internal(const std::string &app_
 
         if (g_threads_count) {
             LogError("Not all threads synchronized: threads left: " << g_threads_count);
-            goto err;
+            for (int i = 0; i < g_threads_count; ++i) {
+                switch (g_tid_status[i].status) {
+                    case TID_STATUS_DEAD:
+                        /* this is totally fine */
+                        LogError("Thread " << g_tid_status[i].tid << " died during attempt to send signal - info can be useful, but this is OK");
+                        break;
+                    case TID_STATUS_NOT_SYNCED:
+                        LogError("Thread " << g_tid_status[i].tid << " didn't receive the signal, serious ERROR!!!");
+                        break;
+                    case TID_STATUS_RECEIVED_SIGNAL:
+                        LogError("Thread " << g_tid_status[i].tid << " received signal but failed at Smack label change, serious ERROR!!!");
+                        break;
+                    case TID_STATUS_SETUP_SMACK:
+                        LogError("Thread " << g_tid_status[i].tid << " received signal, changed Smack label, couldn't change caps, serious ERROR!!!");
+                        break;
+                    case TID_STATUS_FULLY_SYNCED:
+                        /* this is totally fine */
+                        LogError("Thread " << g_tid_status[i].tid << " received signal and was fully synchronized - info can be useful, but this is OK");
+                        break;
+                    default:
+                        LogError("Thread " << g_tid_status[i].tid << " has strange status: " << g_tid_status[i].status << " this is an ERROR!!!");
+                        break;
+                }
+            }
+            LogError("Aborting - please check coredump and check offending threads!");
+            abort(); // needed to debug the offending thread(s)
         }
     }
 
-    if (g_p_app_label && label_for_self_internal() != 0) {
+    if (g_p_app_label && label_for_self_internal(cur_tid) != 0) {
         LogError("label_for_self_internal failed");
         goto err;
     }
@@ -687,11 +749,15 @@ static inline int security_manager_sync_threads_internal(const std::string &app_
     }
 
     cap_free(g_cap);
+    if (g_tid_status)
+        delete [] g_tid_status;
 
     return SECURITY_MANAGER_SUCCESS;
 
 err:
     cap_free(g_cap);
+    if (g_tid_status)
+        delete [] g_tid_status;
     return SECURITY_MANAGER_ERROR_UNKNOWN;
 }