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.

2 files changed, 46 insertions, 5 deletions

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 8df917e4e8e..300d0442e08 100644
--- 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
  *
@@ -953,8 +953,31 @@ checkDataDir(void)
 	}
 
 	/*
+	 * 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 322d3a76742..b906ee581d9 100644
--- 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,