File bsc#1131353-bsc#1131356-0002-High-pacemakerd-vs... of Package pacemaker

Overview Repositories Revisions Requests Users Attributes Meta

File bsc#1131353-bsc#1131356-0002-High-pacemakerd-vs.-IPC-procfs-confused-deputy-authe-1.1.patch of Package pacemaker (Revision 75d7df0b6dbe520036bc2431301d7aa9)

Currently displaying revision 75d7df0b6dbe520036bc2431301d7aa9 , Show latest

From ab44422fa955c2dff1ac1822521e7ad335d4aab7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= <jpokorny@redhat.com>
Date: Mon, 15 Apr 2019 23:19:44 +0200
Subject: [PATCH 2/7] High: pacemakerd vs. IPC/procfs confused deputy
 authenticity issue (0/4)

[0/4: make crm_pid_active more precise as to when detections fail]

It would be bad if the function claimed the process is not active
when the only obstacle in the detection process was that none of the
detection methods worked for a plain lack of permissions to apply
them.  Also, do some other minor cleanup of the function and add its
documentation.  As an additional measure, log spamming is kept at
minimum for repeated queries about the same PID.
---
 include/crm_internal.h | 21 +++++++++++
 lib/common/utils.c     | 96 +++++++++++++++++++++++++++-----------------------
 2 files changed, 73 insertions(+), 44 deletions(-)

diff --git a/include/crm_internal.h b/include/crm_internal.h
index 5692929d0..0adeb7b39 100644
--- a/include/crm_internal.h
+++ b/include/crm_internal.h
@@ -155,6 +155,26 @@ crm_strlen_zero(const char *s)

char *add_list_element(char *list, const char *value);

+/*!
+ * \internal
+ * \brief Detect if process per PID and optionally exe path (component) exists
+ *
+ * \param[in] pid     PID of process assumed alive, disproving of which to try
+ * \param[in] daemon  exe path (component) to possibly match with procfs entry
+ *
+ * \return -1 on invalid PID specification, -2 when the calling process has no
+ *         (is refused an) ability to (dis)prove the predicate,
+ *         0 if the negation of the predicate is confirmed (check-through-kill
+ *         indicates so, or the subsequent check-through-procfs-match on
+ *         \p daemon when provided and procfs available at the standard path),
+ *         1 if it cannot be disproved (reliably [modulo race conditions]
+ *         when \p daemon provided, procfs available at the standard path
+ *         and the calling process has permissions to access the respective
+ *         procfs location, less so otherwise, since mere check-through-kill
+ *         is exercised without powers to exclude PID recycled in the interim).
+ *
+ * \note This function cannot be used to verify \e authenticity of the process.
+ */
 int crm_pid_active(long pid, const char *daemon);
 void crm_make_daemon(const char *name, gboolean daemonize, const char *pidfile);
 
diff --git a/lib/common/utils.c b/lib/common/utils.c
index f3f60ed58..2ac7901b4 100644
--- a/lib/common/utils.c
+++ b/lib/common/utils.c
@@ -1,19 +1,10 @@
 /*
- * Copyright (C) 2004 Andrew Beekhof <andrew@beekhof.net>
+ * Copyright 2004-2019 the Pacemaker project contributors
  *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2.1 of the License, or (at your option) any later version.
+ * The version control history for this file may have further details.
  *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ * This source code is licensed under the GNU Lesser General Public License
+ * version 2.1 or later (LGPLv2.1+) WITHOUT ANY WARRANTY.
  */
 
 #include <crm_internal.h>
@@ -717,16 +708,21 @@ crm_abort(const char *file, const char *function, int line,
 int
 crm_pid_active(long pid, const char *daemon)
 {
+    static int last_asked_pid = 0;  /* log spam prevention */
+#if SUPPORT_PROCFS
     static int have_proc_pid = 0;
+#else
+    static int have_proc_pid = -1;
+#endif
+    int rc = 0;
 
-    if(have_proc_pid == 0) {
+    if (have_proc_pid == 0) {
+        /* evaluation of /proc/PID/exe applicability via self-introspection */
         char proc_path[PATH_MAX], exe_path[PATH_MAX];
-
-        /* check to make sure pid hasn't been reused by another process */
-        snprintf(proc_path, sizeof(proc_path), "/proc/%lu/exe", (long unsigned int)getpid());
-
+        snprintf(proc_path, sizeof(proc_path), "/proc/%lu/exe",
+                 (long unsigned int) getpid());
         have_proc_pid = 1;
-        if(readlink(proc_path, exe_path, PATH_MAX - 1) < 0) {
+        if (readlink(proc_path, exe_path, sizeof(exe_path) - 1) < 0) {
             have_proc_pid = -1;
         }
     }
@@ -734,40 +730,52 @@ crm_pid_active(long pid, const char *daemon)
     if (pid <= 0) {
         return -1;
 
-    } else if (kill(pid, 0) < 0 && errno == ESRCH) {
-        return 0;
+    } else if ((rc = kill(pid, 0)) < 0 && errno == ESRCH) {
+        return 0;  /* no such PID detected */
 
-    } else if(daemon == NULL || have_proc_pid == -1) {
-        return 1;
+    } else if (rc < 0 && have_proc_pid == -1) {
+        if (last_asked_pid != pid) {
+            crm_info("Cannot examine PID %ld: %s", pid, strerror(errno));
+            last_asked_pid = pid;
+        }
+        return -2;  /* errno != ESRCH */
+
+    } else if (rc == 0 && (daemon == NULL || have_proc_pid == -1)) {
+        return 1;  /* kill as the only indicator, cannot double check */
 
     } else {
-        int rc = 0;
+        /* make sure PID hasn't been reused by another process
+           XXX: might still be just a zombie, which could confuse decisions */
+        bool checked_through_kill = (rc == 0);
         char proc_path[PATH_MAX], exe_path[PATH_MAX], myexe_path[PATH_MAX];
-
-        /* check to make sure pid hasn't been reused by another process */
-        snprintf(proc_path, sizeof(proc_path), "/proc/%lu/exe", pid);
-
-        rc = readlink(proc_path, exe_path, PATH_MAX - 1);
-        if (rc < 0 && errno == EACCES) {
-            crm_perror(LOG_INFO, "Could not read from %s", proc_path);
-            return 1;
+        snprintf(proc_path, sizeof(proc_path), "/proc/%ld/exe", pid);
+
+        rc = readlink(proc_path, exe_path, sizeof(exe_path) - 1);
+        if ((rc < 0) && (errno == EACCES)) {
+            if (last_asked_pid != pid) {
+                crm_info("Could not read from %s: %s", proc_path,
+                         strerror(errno));
+                last_asked_pid = pid;
+            }
+            return checked_through_kill ? 1 : -2;
         } else if (rc < 0) {
-            crm_perror(LOG_ERR, "Could not read from %s", proc_path);
-            return 0;
+            if (last_asked_pid != pid) {
+                crm_err("Could not read from %s: %s (%d)", proc_path,
+                        strerror(errno), errno);
+                last_asked_pid = pid;
+            }
+            return 0;  /* most likely errno == ENOENT */
         }
-        
+        exe_path[rc] = '\0';
 
-        exe_path[rc] = 0;
-
-        if(daemon[0] != '/') {
-            rc = snprintf(myexe_path, sizeof(proc_path), CRM_DAEMON_DIR"/%s", daemon);
-            myexe_path[rc] = 0;
+        if (daemon[0] != '/') {
+            rc = snprintf(myexe_path, sizeof(myexe_path), CRM_DAEMON_DIR"/%s",
+                          daemon);
         } else {
-            rc = snprintf(myexe_path, sizeof(proc_path), "%s", daemon);
-            myexe_path[rc] = 0;
+            rc = snprintf(myexe_path, sizeof(myexe_path), "%s", daemon);
         }
-        
-        if (strcmp(exe_path, myexe_path) == 0) {
+
+        if (rc > 0 && rc < sizeof(myexe_path) && !strcmp(exe_path, myexe_path)) {
             return 1;
         }
     }
-- 
2.16.4

Places

File bsc#1131353-bsc#1131356-0002-High-pacemakerd-vs.-IPC-procfs-confused-deputy-authe-1.1.patch of Package pacemaker (Revision 75d7df0b6dbe520036bc2431301d7aa9)

Places