Stephen Frost <sfr...@snowman.net> writes: > * Michael Paquier (mich...@paquier.xyz) wrote: >> If the problem is parsing, it could as well be more portable to put that >> in the control file, no?
> Then we'd need a tool to allow changing it for people who do want to > change it. There's no reason we should have to have an extra tool for > this- an administrator who chooses to change the privileges on the data > folder should be able to do so and I don't think anyone is going to > thank us for requiring them to use some special tool to do so for > PostgreSQL. FWIW, I took a quick look through this patch and don't have any problem with the approach, which appears to be "use the data directory's startup-time permissions as the status indicator". I am kinda wondering about this though: + (These files can confuse <application>pg_ctl</application>.) If group read + access is enabled on the data directory and an unprivileged user in the + <productname>PostgreSQL</productname> group is performing the backup, then + <filename>postmaster.pid</filename> will not be readable and must be + excluded. If we're allowing group read on the data directory, why should that not include postmaster.pid? There's nothing terribly security-relevant in there, and being able to find out the postmaster PID seems useful. I can see the point of restricting server key files, as documented elsewhere, but it's possible to keep those somewhere else so they don't cause problems for simple backup software. Also, in general, this patch desperately needs a round of copy-editing, particularly with attention to the comments. For instance, there are a minimum of three grammatical errors in this one comment: + * Group read/execute may optional be enabled on PGDATA so any frontend tools + * That write into PGDATA must know what mask to set and the permissions to + * use for creating files and directories. In a larger sense, this fails to explain the operating principle, namely what I stated above, that we allow the existing permissions on PGDATA to decide what we allow as group permissions. If you don't already understand that, you're going to have a hard time extracting it from either file_perm.h or file_perm.c. (The latter fails to even explain what the argument of DataDirectoryMask is.) regards, tom lane