Treat EPERM as a non-error case when checking to see if old postmaster

is still alive.  This improves our odds of not getting fooled by an
unrelated process when checking a stale lock file.  Other checks already
in place, plus one newly added in checkDataDir(), ensure that we cannot
attempt to usurp the place of a postmaster belonging to a different userid,
so there is no need to error out.  Add comments indicating the importance
of these other checks.

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 8df917e4e8e643bfffd8cc316db9d3f8a98320e0..300d0442e0839d263ec67cf0bc549f135cbde0a8 100644 (file)
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -37,7 +37,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/postmaster/postmaster.c,v 1.446 2005/03/10 07:14:03 neilc Exp $
+ *   $PostgreSQL: pgsql/src/backend/postmaster/postmaster.c,v 1.447 2005/03/18 03:48:49 tgl Exp $
  *
  * NOTES
  *
@@ -952,9 +952,32 @@ checkDataDir(void)
                    DataDir)));
    }
 
+   /*
+    * Check that the directory belongs to my userid; if not, reject.
+    *
+    * This check is an essential part of the interlock that prevents two
+    * postmasters from starting in the same directory (see CreateLockFile()).
+    * Do not remove or weaken it.
+    *
+    * XXX can we safely enable this check on Windows?
+    */
+#if !defined(WIN32) && !defined(__CYGWIN__)
+   if (stat_buf.st_uid != geteuid())
+       ereport(FATAL,
+               (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+                errmsg("data directory \"%s\" has wrong ownership",
+                       DataDir),
+                errhint("The server must be started by the user that owns the data directory.")));
+#endif
+
    /*
     * Check if the directory has group or world access.  If so, reject.
     *
+    * It would be possible to allow weaker constraints (for example, allow
+    * group access) but we cannot make a general assumption that that is
+    * okay; for example there are platforms where nearly all users customarily
+    * belong to the same group.  Perhaps this test should be configurable.
+    *
     * XXX temporarily suppress check when on Windows, because there may not
     * be proper support for Unix-y file permissions.  Need to think of a
     * reasonable check to apply on Windows.

diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index 322d3a767428783774d8ab28dee950ee51d99ac5..b906ee581d95cca2555087a6e6fa2ee80817422e 100644 (file)
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/utils/init/miscinit.c,v 1.137 2004/12/31 22:01:40 pgsql Exp $
+ *   $PostgreSQL: pgsql/src/backend/utils/init/miscinit.c,v 1.138 2005/03/18 03:48:49 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -505,6 +505,9 @@ CreateLockFile(const char *filename, bool amPostmaster,
    {
        /*
         * Try to create the lock file --- O_EXCL makes this atomic.
+        *
+        * Think not to make the file protection weaker than 0600.  See
+        * comments below.
         */
        fd = open(filename, O_RDWR | O_CREAT | O_EXCL, 0600);
        if (fd >= 0)
@@ -564,6 +567,21 @@ CreateLockFile(const char *filename, bool amPostmaster,
         * then all but the immediate parent shell will be root-owned processes
         * and so the kill test will fail with EPERM.
         *
+        * We can treat the EPERM-error case as okay because that error implies
+        * that the existing process has a different userid than we do, which
+        * means it cannot be a competing postmaster.  A postmaster cannot
+        * successfully attach to a data directory owned by a userid other
+        * than its own.  (This is now checked directly in checkDataDir(),
+        * but has been true for a long time because of the restriction that
+        * the data directory isn't group- or world-accessible.)  Also,
+        * since we create the lockfiles mode 600, we'd have failed above
+        * if the lockfile belonged to another userid --- which means that
+        * whatever process kill() is reporting about isn't the one that
+        * made the lockfile.  (NOTE: this last consideration is the only
+        * one that keeps us from blowing away a Unix socket file belonging
+        * to an instance of Postgres being run by someone else, at least
+        * on machines where /tmp hasn't got a stickybit.)
+        *
         * Windows hasn't got getppid(), but doesn't need it since it's not
         * using real kill() either...
         *
@@ -577,11 +595,11 @@ CreateLockFile(const char *filename, bool amPostmaster,
            )
        {
            if (kill(other_pid, 0) == 0 ||
-               (errno != ESRCH
+               (errno != ESRCH &&
 #ifdef __BEOS__
-                && errno != EINVAL
+                errno != EINVAL &&
 #endif
-                ))
+                errno != EPERM))
            {
                /* lockfile belongs to a live process */
                ereport(FATAL,