Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 7a969ca

Browse files
committed
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.
1 parent d344505 commit 7a969ca

File tree

2 files changed

+46
-5
lines changed

2 files changed

+46
-5
lines changed

src/backend/postmaster/postmaster.c

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
*
3838
*
3939
* IDENTIFICATION
40-
* $PostgreSQL: pgsql/src/backend/postmaster/postmaster.c,v 1.446 2005/03/10 07:14:03 neilc Exp $
40+
* $PostgreSQL: pgsql/src/backend/postmaster/postmaster.c,v 1.447 2005/03/18 03:48:49 tgl Exp $
4141
*
4242
* NOTES
4343
*
@@ -952,9 +952,32 @@ checkDataDir(void)
952952
DataDir)));
953953
}
954954

955+
/*
956+
* Check that the directory belongs to my userid; if not, reject.
957+
*
958+
* This check is an essential part of the interlock that prevents two
959+
* postmasters from starting in the same directory (see CreateLockFile()).
960+
* Do not remove or weaken it.
961+
*
962+
* XXX can we safely enable this check on Windows?
963+
*/
964+
#if !defined(WIN32) && !defined(__CYGWIN__)
965+
if (stat_buf.st_uid != geteuid())
966+
ereport(FATAL,
967+
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
968+
errmsg("data directory \"%s\" has wrong ownership",
969+
DataDir),
970+
errhint("The server must be started by the user that owns the data directory.")));
971+
#endif
972+
955973
/*
956974
* Check if the directory has group or world access. If so, reject.
957975
*
976+
* It would be possible to allow weaker constraints (for example, allow
977+
* group access) but we cannot make a general assumption that that is
978+
* okay; for example there are platforms where nearly all users customarily
979+
* belong to the same group. Perhaps this test should be configurable.
980+
*
958981
* XXX temporarily suppress check when on Windows, because there may not
959982
* be proper support for Unix-y file permissions. Need to think of a
960983
* reasonable check to apply on Windows.

src/backend/utils/init/miscinit.c

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/utils/init/miscinit.c,v 1.137 2004/12/31 22:01:40 pgsql Exp $
11+
* $PostgreSQL: pgsql/src/backend/utils/init/miscinit.c,v 1.138 2005/03/18 03:48:49 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -505,6 +505,9 @@ CreateLockFile(const char *filename, bool amPostmaster,
505505
{
506506
/*
507507
* Try to create the lock file --- O_EXCL makes this atomic.
508+
*
509+
* Think not to make the file protection weaker than 0600. See
510+
* comments below.
508511
*/
509512
fd = open(filename, O_RDWR | O_CREAT | O_EXCL, 0600);
510513
if (fd >= 0)
@@ -564,6 +567,21 @@ CreateLockFile(const char *filename, bool amPostmaster,
564567
* then all but the immediate parent shell will be root-owned processes
565568
* and so the kill test will fail with EPERM.
566569
*
570+
* We can treat the EPERM-error case as okay because that error implies
571+
* that the existing process has a different userid than we do, which
572+
* means it cannot be a competing postmaster. A postmaster cannot
573+
* successfully attach to a data directory owned by a userid other
574+
* than its own. (This is now checked directly in checkDataDir(),
575+
* but has been true for a long time because of the restriction that
576+
* the data directory isn't group- or world-accessible.) Also,
577+
* since we create the lockfiles mode 600, we'd have failed above
578+
* if the lockfile belonged to another userid --- which means that
579+
* whatever process kill() is reporting about isn't the one that
580+
* made the lockfile. (NOTE: this last consideration is the only
581+
* one that keeps us from blowing away a Unix socket file belonging
582+
* to an instance of Postgres being run by someone else, at least
583+
* on machines where /tmp hasn't got a stickybit.)
584+
*
567585
* Windows hasn't got getppid(), but doesn't need it since it's not
568586
* using real kill() either...
569587
*
@@ -577,11 +595,11 @@ CreateLockFile(const char *filename, bool amPostmaster,
577595
)
578596
{
579597
if (kill(other_pid, 0) == 0 ||
580-
(errno != ESRCH
598+
(errno != ESRCH &&
581599
#ifdef __BEOS__
582-
&& errno != EINVAL
600+
errno != EINVAL &&
583601
#endif
584-
))
602+
errno != EPERM))
585603
{
586604
/* lockfile belongs to a live process */
587605
ereport(FATAL,

0 commit comments

Comments
 (0)