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,