Re: [HACKERS] OpenFile() Permissions Refactor

On 8/29/17 12:15, David Steele wrote:
> While working on the patch to allow group reads of $PGDATA I refactored
> the various OpenFile() functions to use default/global permissions
> rather than permissions defined in each call.
> 
> I think the patch stands on its own regardless of whether we accept the
> patch to allow group permissions (which won't make this CF).  We had a
> couple different ways of defining permissions (e.g. 0600 vs S_IRUSR |
> S_IWUSR) and in any case it represented quite a bit of duplication.
> This way seems simpler and less error-prone.

Yeah, it would be good to clean some of that up.

I wonder whether we even need that much flexibility.  We already set a
global umask, so we could just open all files with 0666 and let the
umask sort it out.  Then we don't need all the *Perm() variants.

I don't like the function-like macros in fd.h.  We can use proper functions.

I also wonder whether the umask save-and-restore code in copy.c and
be-fsstubs.c is sound.  If the AllocateFile() call in between errors
out, then there is nothing that restores the original umask.  This might
need a TRY/CATCH block, or maybe just a chmod() afterwards.

The mkdir() calls could perhaps use some further refactoring so you
don't have to specify the mode everywhere.

This kind of code:

-   if (stat_buf.st_mode & (S_IRWXG | S_IRWXO))
+   if (stat_buf.st_mode & PG_MODE_MASK_DEFAULT)       ereport(FATAL,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),               errmsg("data directory \"%s\" has group or world
access",                      DataDir),
 
-                errdetail("Permissions should be u=rwx (0700).")));
+                errdetail("Permissions should be (%04o).",
PG_DIR_MODE_DEFAULT)));

can be problematic, because we are hoping that PG_MODE_MASK_DEFAULT,
PG_DIR_MODE_DEFAULT, and the wording in the error message can stay
consistent.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services