Skip to content

Commit da9b580

Browse files
sfrostsfrost
committed
Refactor dir/file permissions
Consolidate directory and file create permissions for tools which work with the PG data directory by adding a new module (common/file_perm.c) that contains variables (pg_file_create_mode, pg_dir_create_mode) and constants to initialize them (0600 for files and 0700 for directories). Convert mkdir() calls in the backend to MakePGDirectory() if the original call used default permissions (always the case for regular PG directories). Add tests to make sure permissions in PGDATA are set correctly by the tools which modify the PG data directory. Authors: David Steele <david@pgmasters.net>, Adam Brightwell <adam.brightwell@crunchydata.com> Reviewed-By: Michael Paquier, with discussion amongst many others. Discussion: https://postgr.es/m/ad346fe6-b23e-59f1-ecb7-0e08390ad629%40pgmasters.net
1 parent 499be01 commit da9b580

File tree

34 files changed

+330
-75
lines changed

34 files changed

+330
-75
lines changed

‎src/backend/access/transam/xlog.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4107,7 +4107,7 @@ ValidateXLOGDirectoryStructure(void)
41074107
{
41084108
ereport(LOG,
41094109
(errmsg("creating missing WAL directory \"%s\"", path)));
4110-
if (mkdir(path, S_IRWXU) < 0)
4110+
if (MakePGDirectory(path) < 0)
41114111
ereport(FATAL,
41124112
(errmsg("could not create missing directory \"%s\": %m",
41134113
path)));

‎src/backend/commands/tablespace.c

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@
6868
#include "commands/seclabel.h"
6969
#include "commands/tablecmds.h"
7070
#include "commands/tablespace.h"
71+
#include "common/file_perm.h"
7172
#include "miscadmin.h"
7273
#include "postmaster/bgwriter.h"
7374
#include "storage/fd.h"
@@ -151,7 +152,7 @@ TablespaceCreateDbspace(Oid spcNode, Oid dbNode, bool isRedo)
151152
else
152153
{
153154
/* Directory creation failed? */
154-
if (mkdir(dir, S_IRWXU) < 0)
155+
if (MakePGDirectory(dir) < 0)
155156
{
156157
char *parentdir;
157158

@@ -173,7 +174,7 @@ TablespaceCreateDbspace(Oid spcNode, Oid dbNode, bool isRedo)
173174
get_parent_directory(parentdir);
174175
get_parent_directory(parentdir);
175176
/* Can't create parent and it doesn't already exist? */
176-
if (mkdir(parentdir, S_IRWXU) < 0 && errno != EEXIST)
177+
if (MakePGDirectory(parentdir) < 0 && errno != EEXIST)
177178
ereport(ERROR,
178179
(errcode_for_file_access(),
179180
errmsg("could not create directory \"%s\": %m",
@@ -184,15 +185,15 @@ TablespaceCreateDbspace(Oid spcNode, Oid dbNode, bool isRedo)
184185
parentdir = pstrdup(dir);
185186
get_parent_directory(parentdir);
186187
/* Can't create parent and it doesn't already exist? */
187-
if (mkdir(parentdir, S_IRWXU) < 0 && errno != EEXIST)
188+
if (MakePGDirectory(parentdir) < 0 && errno != EEXIST)
188189
ereport(ERROR,
189190
(errcode_for_file_access(),
190191
errmsg("could not create directory \"%s\": %m",
191192
parentdir)));
192193
pfree(parentdir);
193194

194195
/* Create database directory */
195-
if (mkdir(dir, S_IRWXU) < 0)
196+
if (MakePGDirectory(dir) < 0)
196197
ereport(ERROR,
197198
(errcode_for_file_access(),
198199
errmsg("could not create directory \"%s\": %m",
@@ -279,7 +280,8 @@ CreateTableSpace(CreateTableSpaceStmt *stmt)
279280
/*
280281
* Check that location isn't too long. Remember that we're going to append
281282
* 'PG_XXX/<dboid>/<relid>_<fork>.<nnn>'. FYI, we never actually
282-
* reference the whole path here, but mkdir() uses the first two parts.
283+
* reference the whole path here, but MakePGDirectory() uses the first two
284+
* parts.
283285
*/
284286
if (strlen(location) + 1 + strlen(TABLESPACE_VERSION_DIRECTORY) + 1 +
285287
OIDCHARS + 1 + OIDCHARS + 1 + FORKNAMECHARS + 1 + OIDCHARS > MAXPGPATH)
@@ -574,7 +576,7 @@ create_tablespace_directories(const char *location, const Oid tablespaceoid)
574576
* Attempt to coerce target directory to safe permissions. If this fails,
575577
* it doesn't exist or has the wrong owner.
576578
*/
577-
if (chmod(location, S_IRWXU) != 0)
579+
if (chmod(location, pg_dir_create_mode) != 0)
578580
{
579581
if (errno == ENOENT)
580582
ereport(ERROR,
@@ -599,7 +601,7 @@ create_tablespace_directories(const char *location, const Oid tablespaceoid)
599601
if (stat(location_with_version_dir, &st) == 0 && S_ISDIR(st.st_mode))
600602
{
601603
if (!rmtree(location_with_version_dir, true))
602-
/* If this failed, mkdir() below is going to error. */
604+
/* If this failed, MakePGDirectory() below is going to error. */
603605
ereport(WARNING,
604606
(errmsg("some useless files may be left behind in old database directory \"%s\"",
605607
location_with_version_dir)));
@@ -610,7 +612,7 @@ create_tablespace_directories(const char *location, const Oid tablespaceoid)
610612
* The creation of the version directory prevents more than one tablespace
611613
* in a single location.
612614
*/
613-
if (mkdir(location_with_version_dir, S_IRWXU) < 0)
615+
if (MakePGDirectory(location_with_version_dir) < 0)
614616
{
615617
if (errno == EEXIST)
616618
ereport(ERROR,

‎src/backend/postmaster/postmaster.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@
9797
#include "access/xlog.h"
9898
#include "bootstrap/bootstrap.h"
9999
#include "catalog/pg_control.h"
100+
#include "common/file_perm.h"
100101
#include "common/ip.h"
101102
#include "lib/ilist.h"
102103
#include "libpq/auth.h"
@@ -589,7 +590,7 @@ PostmasterMain(int argc, char *argv[])
589590
/*
590591
* for security, no dir or file created can be group or other accessible
591592
*/
592-
umask(S_IRWXG | S_IRWXO);
593+
umask(PG_MODE_MASK_OWNER);
593594

594595
/*
595596
* Initialize random(3) so we don't get the same values in every run.
@@ -4490,9 +4491,9 @@ internal_forkexec(int argc, char *argv[], Port *port)
44904491
{
44914492
/*
44924493
* As in OpenTemporaryFileInTablespace, try to make the temp-file
4493-
* directory
4494+
* directory, ignoring errors.
44944495
*/
4495-
mkdir(PG_TEMP_FILES_DIR, S_IRWXU);
4496+
(void) MakePGDirectory(PG_TEMP_FILES_DIR);
44964497

44974498
fp = AllocateFile(tmpfilename, PG_BINARY_W);
44984499
if (!fp)

‎src/backend/postmaster/syslogger.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
#include "postmaster/postmaster.h"
4242
#include "postmaster/syslogger.h"
4343
#include "storage/dsm.h"
44+
#include "storage/fd.h"
4445
#include "storage/ipc.h"
4546
#include "storage/latch.h"
4647
#include "storage/pg_shmem.h"
@@ -322,7 +323,7 @@ SysLoggerMain(int argc, char *argv[])
322323
/*
323324
* Also, create new directory if not present; ignore errors
324325
*/
325-
mkdir(Log_directory, S_IRWXU);
326+
(void) MakePGDirectory(Log_directory);
326327
}
327328
if (strcmp(Log_filename, currentLogFilename) != 0)
328329
{
@@ -564,7 +565,7 @@ SysLogger_Start(void)
564565
/*
565566
* Create log directory if not present; ignore errors
566567
*/
567-
mkdir(Log_directory, S_IRWXU);
568+
(void) MakePGDirectory(Log_directory);
568569

569570
/*
570571
* The initial logfile is created right in the postmaster, to verify that

‎src/backend/replication/basebackup.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "access/xlog_internal.h" /* for pg_start/stop_backup */
2020
#include "catalog/catalog.h"
2121
#include "catalog/pg_type.h"
22+
#include "common/file_perm.h"
2223
#include "lib/stringinfo.h"
2324
#include "libpq/libpq.h"
2425
#include "libpq/pqformat.h"
@@ -930,7 +931,7 @@ sendFileWithContent(const char *filename, const char *content)
930931
statbuf.st_gid = getegid();
931932
#endif
932933
statbuf.st_mtime = time(NULL);
933-
statbuf.st_mode = S_IRUSR | S_IWUSR;
934+
statbuf.st_mode = pg_file_create_mode;
934935
statbuf.st_size = len;
935936

936937
_tarWriteHeader(filename, NULL, &statbuf, false);
@@ -1628,7 +1629,7 @@ _tarWriteDir(const char *pathbuf, int basepathlen, struct stat *statbuf,
16281629
#else
16291630
if (pgwin32_is_junction(pathbuf))
16301631
#endif
1631-
statbuf->st_mode = S_IFDIR | S_IRWXU;
1632+
statbuf->st_mode = S_IFDIR | pg_dir_create_mode;
16321633

16331634
return _tarWriteHeader(pathbuf + basepathlen + 1, NULL, statbuf, sizeonly);
16341635
}

‎src/backend/replication/slot.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1166,13 +1166,14 @@ CreateSlotOnDisk(ReplicationSlot *slot)
11661166
* It's just barely possible that some previous effort to create or drop a
11671167
* slot with this name left a temp directory lying around. If that seems
11681168
* to be the case, try to remove it. If the rmtree() fails, we'll error
1169-
* out at the mkdir() below, so we don't bother checking success.
1169+
* out at the MakePGDirectory() below, so we don't bother checking
1170+
* success.
11701171
*/
11711172
if (stat(tmppath, &st) == 0 && S_ISDIR(st.st_mode))
11721173
rmtree(tmppath, true);
11731174

11741175
/* Create and fsync the temporary slot directory. */
1175-
if (mkdir(tmppath, S_IRWXU) < 0)
1176+
if (MakePGDirectory(tmppath) < 0)
11761177
ereport(ERROR,
11771178
(errcode_for_file_access(),
11781179
errmsg("could not create directory \"%s\": %m",

‎src/backend/storage/file/copydir.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ copydir(char *fromdir, char *todir, bool recurse)
4141
char fromfile[MAXPGPATH * 2];
4242
char tofile[MAXPGPATH * 2];
4343

44-
if (mkdir(todir, S_IRWXU) != 0)
44+
if (MakePGDirectory(todir) != 0)
4545
ereport(ERROR,
4646
(errcode_for_file_access(),
4747
errmsg("could not create directory \"%s\": %m", todir)));

‎src/backend/storage/file/fd.c

Lines changed: 35 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@
8484
#include "access/xlog.h"
8585
#include "catalog/catalog.h"
8686
#include "catalog/pg_tablespace.h"
87+
#include "common/file_perm.h"
8788
#include "pgstat.h"
8889
#include "portability/mem.h"
8990
#include "storage/fd.h"
@@ -124,12 +125,6 @@
124125
*/
125126
#define FD_MINFREE 10
126127

127-
/*
128-
* Default mode for created files, unless something else is specified using
129-
* the *Perm() function variants.
130-
*/
131-
#define PG_FILE_MODE_DEFAULT (S_IRUSR | S_IWUSR)
132-
133128
/*
134129
* A number of platforms allow individual processes to open many more files
135130
* than they can really support when *many* processes do the same thing.
@@ -937,7 +932,7 @@ set_max_safe_fds(void)
937932
int
938933
BasicOpenFile(const char *fileName, int fileFlags)
939934
{
940-
return BasicOpenFilePerm(fileName, fileFlags, PG_FILE_MODE_DEFAULT);
935+
return BasicOpenFilePerm(fileName, fileFlags, pg_file_create_mode);
941936
}
942937

943938
/*
@@ -1356,7 +1351,7 @@ FileInvalidate(File file)
13561351
File
13571352
PathNameOpenFile(const char *fileName, int fileFlags)
13581353
{
1359-
return PathNameOpenFilePerm(fileName, fileFlags, PG_FILE_MODE_DEFAULT);
1354+
return PathNameOpenFilePerm(fileName, fileFlags, pg_file_create_mode);
13601355
}
13611356

13621357
/*
@@ -1434,7 +1429,7 @@ PathNameOpenFilePerm(const char *fileName, int fileFlags, mode_t fileMode)
14341429
void
14351430
PathNameCreateTemporaryDir(const char *basedir, const char *directory)
14361431
{
1437-
if (mkdir(directory, S_IRWXU) < 0)
1432+
if (MakePGDirectory(directory) < 0)
14381433
{
14391434
if (errno == EEXIST)
14401435
return;
@@ -1444,14 +1439,14 @@ PathNameCreateTemporaryDir(const char *basedir, const char *directory)
14441439
* EEXIST to close a race against another process following the same
14451440
* algorithm.
14461441
*/
1447-
if (mkdir(basedir, S_IRWXU) < 0 && errno != EEXIST)
1442+
if (MakePGDirectory(basedir) < 0 && errno != EEXIST)
14481443
ereport(ERROR,
14491444
(errcode_for_file_access(),
14501445
errmsg("cannot create temporary directory \"%s\": %m",
14511446
basedir)));
14521447

14531448
/* Try again. */
1454-
if (mkdir(directory, S_IRWXU) < 0 && errno != EEXIST)
1449+
if (MakePGDirectory(directory) < 0 && errno != EEXIST)
14551450
ereport(ERROR,
14561451
(errcode_for_file_access(),
14571452
errmsg("cannot create temporary subdirectory \"%s\": %m",
@@ -1601,11 +1596,11 @@ OpenTemporaryFileInTablespace(Oid tblspcOid, bool rejectError)
16011596
* We might need to create the tablespace's tempfile directory, if no
16021597
* one has yet done so.
16031598
*
1604-
* Don't check for error from mkdir; it could fail if someone else
1605-
* just did the same thing. If it doesn't work then we'll bomb out on
1606-
* the second create attempt, instead.
1599+
* Don't check for an error from MakePGDirectory; it could fail if
1600+
* someone else just did the same thing. If it doesn't work then
1601+
* we'll bomb out on the second create attempt, instead.
16071602
*/
1608-
mkdir(tempdirpath, S_IRWXU);
1603+
(void) MakePGDirectory(tempdirpath);
16091604

16101605
file = PathNameOpenFile(tempfilepath,
16111606
O_RDWR | O_CREAT | O_TRUNC | PG_BINARY);
@@ -2401,7 +2396,7 @@ AllocateFile(const char *name, const char *mode)
24012396
int
24022397
OpenTransientFile(const char *fileName, int fileFlags)
24032398
{
2404-
return OpenTransientFilePerm(fileName, fileFlags, PG_FILE_MODE_DEFAULT);
2399+
return OpenTransientFilePerm(fileName, fileFlags, pg_file_create_mode);
24052400
}
24062401

24072402
/*
@@ -3554,3 +3549,27 @@ fsync_parent_path(const char *fname, int elevel)
35543549

35553550
return 0;
35563551
}
3552+
3553+
/*
3554+
* Create a PostgreSQL data sub-directory
3555+
*
3556+
* The data directory itself, along with most other directories, are created at
3557+
* initdb-time, but we do have some occations where we create directories from
3558+
* the backend (CREATE TABLESPACE, for example). In those cases, we want to
3559+
* make sure that those directories are created consistently. Today, that means
3560+
* making sure that the created directory has the correct permissions, which is
3561+
* what pg_dir_create_mode tracks for us.
3562+
*
3563+
* Note that we also set the umask() based on what we understand the correct
3564+
* permissions to be (see file_perm.c).
3565+
*
3566+
* For permissions other than the default mkdir() can be used directly, but be
3567+
* sure to consider carefully such cases -- a directory with incorrect
3568+
* permissions in a PostgreSQL data directory could cause backups and other
3569+
* processes to fail.
3570+
*/
3571+
int
3572+
MakePGDirectory(const char *directoryName)
3573+
{
3574+
return mkdir(directoryName, pg_dir_create_mode);
3575+
}

‎src/backend/storage/ipc/dsm_impl.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
#ifdef HAVE_SYS_SHM_H
6161
#include <sys/shm.h>
6262
#endif
63+
#include "common/file_perm.h"
6364
#include "pgstat.h"
6465

6566
#include "portability/mem.h"
@@ -285,7 +286,7 @@ dsm_impl_posix(dsm_op op, dsm_handle handle, Size request_size,
285286
* returning.
286287
*/
287288
flags = O_RDWR | (op == DSM_OP_CREATE ? O_CREAT | O_EXCL : 0);
288-
if ((fd = shm_open(name, flags, 0600)) == -1)
289+
if ((fd = shm_open(name, flags, PG_FILE_MODE_OWNER)) == -1)
289290
{
290291
if (errno != EEXIST)
291292
ereport(elevel,

‎src/backend/storage/ipc/ipc.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,10 @@ proc_exit(int code)
137137
else
138138
snprintf(gprofDirName, 32, "gprof/%d", (int) getpid());
139139

140+
/*
141+
* Use mkdir() instead of MakePGDirectory() since we aren't making a
142+
* PG directory here.
143+
*/
140144
mkdir("gprof", S_IRWXU | S_IRWXG | S_IRWXO);
141145
mkdir(gprofDirName, S_IRWXU | S_IRWXG | S_IRWXO);
142146
chdir(gprofDirName);

‎src/backend/utils/init/miscinit.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232

3333
#include "access/htup_details.h"
3434
#include "catalog/pg_authid.h"
35+
#include "common/file_perm.h"
3536
#include "libpq/libpq.h"
3637
#include "mb/pg_wchar.h"
3738
#include "miscadmin.h"
@@ -831,7 +832,7 @@ CreateLockFile(const char *filename, bool amPostmaster,
831832
* Think not to make the file protection weaker than 0600. See
832833
* comments below.
833834
*/
834-
fd = open(filename, O_RDWR | O_CREAT | O_EXCL, 0600);
835+
fd = open(filename, O_RDWR | O_CREAT | O_EXCL, pg_file_create_mode);
835836
if (fd >= 0)
836837
break; /* Success; exit the retry loop */
837838

@@ -848,7 +849,7 @@ CreateLockFile(const char *filename, bool amPostmaster,
848849
* Read the file to get the old owner's PID. Note race condition
849850
* here: file might have been deleted since we tried to create it.
850851
*/
851-
fd = open(filename, O_RDONLY, 0600);
852+
fd = open(filename, O_RDONLY, pg_file_create_mode);
852853
if (fd < 0)
853854
{
854855
if (errno == ENOENT)

0 commit comments

Comments
 (0)