[PATCH] (issue139) Check that pid is not running if the lock can be aquired

Wald Commits scm-commit at wald.intevation.org
Wed Oct 15 14:19:51 CEST 2014


# HG changeset patch
# User Andre Heinecke <andre.heinecke at intevation.de>
# Date 1413375586 -7200
# Node ID 8362e30f7b556a8d4f9d0af94affa6a816b1a6d7
# Parent  8897c90b81664cf05c93ee409f08afeb7faa28d7
(issue139) Check that pid is not running if the lock can be aquired

    If the lock on the lockfile can be aquired it might still be
    possible that another process is running. So we read the
    pid from the lockfile and check if another process with the
    same installation prefix as us is running.

diff -r 8897c90b8166 -r 8362e30f7b55 common/linuxlockfile.c
--- a/common/linuxlockfile.c	Wed Oct 15 13:24:59 2014 +0200
+++ b/common/linuxlockfile.c	Wed Oct 15 14:19:46 2014 +0200
@@ -12,8 +12,11 @@
 #include <stdio.h>
 #include <sys/types.h>
 #include <unistd.h>
+#include <string.h>
 
 #include "logging.h"
+#include "util.h"
+#include "strhelp.h"
 
 int
 open_lockfile(char *path)
@@ -30,23 +33,55 @@
       lk.l_whence = SEEK_SET;
       lk.l_start = 0;
       lk.l_len = 0;
+      fcntl(fd, F_SETLK, &lk);
       if (fcntl(fd, F_SETLK, &lk) != -1)
-        {
-          /* FIXME (issue139): For extra security we should check if there is
-             already a pid in the file.  If so we should check in
-             /proc/$PID if there is still a process of the same name
-             as ours running... */
-          ftruncate(fd, 0);
-          pidstrlen = (size_t)snprintf(pidstr, sizeof(pidstr), "%d", getpid());
-          write(fd, pidstr, pidstrlen);
-        }
-      else
-        {
-          /* Lock can not be acquired.  Bail out... */
-          close(fd);
-          DEBUGPRINTF("Could not get an exclusive lock on %s.\n", path);
-          return -1;
-        }
+         {
+           char oldpid[20];
+           FILE *f = fopen(path, "r");
+           size_t bytes_read;
+           if (f)
+             {
+               bytes_read = fread(oldpid, 1, sizeof(oldpid), f);
+               if (bytes_read)
+                 {
+                   char *oldPath,
+                        *newPath;
+                   if (bytes_read == sizeof(oldpid))
+                     {
+                       ERRORPRINTF ("Bad information in pidfile\n");
+                     }
+                   else
+                     {
+                       oldpid[bytes_read] = '\0';
+                       oldPath = get_proc_install_dir(oldpid);
+                       newPath = get_install_dir();
+                       if (oldPath && newPath && strcmp(oldPath, newPath) == 0)
+                         {
+                           DEBUGPRINTF("Got lock but process from %s is still"
+                                       "running.\n", oldPath);
+                           xfree(oldPath);
+                           xfree(newPath);
+                           return -1;
+                         }
+                       xfree(oldPath);
+                       xfree(newPath);
+                     }
+                 }
+               fclose(f);
+             }
+
+           ftruncate(fd, 0);
+           pidstrlen = (size_t)snprintf(pidstr, sizeof(pidstr), "%d",
+                                        getpid());
+           write(fd, pidstr, pidstrlen);
+         }
+       else
+         {
+           /* Lock can not be acquired.  Bail out... */
+           close(fd);
+           DEBUGPRINTF("Could not get an exclusive lock on %s.\n", path);
+           return -1;
+         }
     }
   else
     {
diff -r 8897c90b8166 -r 8362e30f7b55 common/util.c
--- a/common/util.c	Wed Oct 15 13:24:59 2014 +0200
+++ b/common/util.c	Wed Oct 15 14:19:46 2014 +0200
@@ -314,15 +314,26 @@
 #else /* WIN32 */
 
 char *
-get_install_dir()
+get_proc_install_dir(const char *proc)
 {
   char *retval = NULL,
-        *p = NULL,
-         buf[MAX_PATH_LINUX];
+       *procpath = NULL,
+       *p = NULL,
+        buf[MAX_PATH_LINUX];
   ssize_t ret;
   size_t path_len = 0;
 
-  ret = readlink ("/proc/self/exe", buf, MAX_PATH_LINUX);
+  if (!proc)
+    {
+      return NULL;
+    }
+
+  xasprintf(&procpath, "/proc/%s/exe", proc);
+
+  ret = readlink (procpath, buf, MAX_PATH_LINUX);
+  xfree(procpath);
+  procpath = NULL;
+
   if (ret <= 0)
     {
       ERRORPRINTF ("readlink failed\n");
@@ -348,6 +359,12 @@
   return retval;
 }
 
+char *
+get_install_dir()
+{
+  return get_proc_install_dir("self");
+}
+
 bool
 is_system_install()
 {
diff -r 8897c90b8166 -r 8362e30f7b55 common/util.h
--- a/common/util.h	Wed Oct 15 13:24:59 2014 +0200
+++ b/common/util.h	Wed Oct 15 14:19:46 2014 +0200
@@ -69,6 +69,22 @@
  */
 char * get_install_dir();
 
+#ifndef WIN32
+/**@brief Get the directory in which the process proc resides in
+ *
+ * Look up the directory in which the process proc is placed.
+ * If the path is longer then MAX_PATH NULL is returned.
+ *
+ * Returns a utf-8 encoded string that has to be freed by the caller
+ * on linux the path is returned as is including the last /.
+ *
+ * @param[in] A process id or special name from the proc file system.
+ *
+ * @returns The directory of the process
+ */
+char * get_proc_install_dir(const char *proc);
+#endif
+
 #ifdef WIN32
 /**@brief Get a copy of the processes owner sid
  *


More information about the Trustbridge-commits mailing list