Tom Lane wrote:
Andres Freund and...@2ndquadrant.com writes:
On 2013-08-19 14:28:28 -0400, Tom Lane wrote:
One possibility is to do the initial check somewhere shortly after
ChangeToDataDir(), and have the GUC check hook only attempt to make a
check in SIGHUP context. Unfortunately we
Tom Lane wrote:
I think we should change 9.3 to be restrictive about ownership/permissions
on the stats_temp_directory (ie, require owner = postgres user,
permissions = 0700, same as for the $PGDATA directory).
Not an easy thing to do, this. It should be done as a GUC check hook,
ISTM, but
Alvaro Herrera wrote:
In addition to that, it might be a good idea to do what the comment in the
code suggests, namely do more than zero checking on each file name to try
to make sure it looks like a stats temp file name that we'd generate
before we delete it. The ownership/permissions
Alvaro Herrera alvhe...@2ndquadrant.com writes:
Alvaro Herrera wrote:
In addition to that, it might be a good idea to do what the comment in the
code suggests, namely do more than zero checking on each file name to try
to make sure it looks like a stats temp file name that we'd generate
Alvaro Herrera alvhe...@2ndquadrant.com writes:
The implementation I chose for the actual check was to separate the
permission checks from checkDataDir() into src/port/pgcheckdir.c that
returns different error codes for each case; see first attachment.
This part seems pretty reasonable, except
On 2013-08-19 14:28:28 -0400, Tom Lane wrote:
One possibility is to do the initial check somewhere shortly after
ChangeToDataDir(), and have the GUC check hook only attempt to make a
check in SIGHUP context. Unfortunately we aren't passing the context to
check hooks, only GucSource which
Andres Freund and...@2ndquadrant.com writes:
On 2013-08-19 14:28:28 -0400, Tom Lane wrote:
One possibility is to do the initial check somewhere shortly after
ChangeToDataDir(), and have the GUC check hook only attempt to make a
check in SIGHUP context. Unfortunately we aren't passing the
On 2013-08-19 13:50:38 -0400, Alvaro Herrera wrote:
Tom Lane wrote:
I think we should change 9.3 to be restrictive about ownership/permissions
on the stats_temp_directory (ie, require owner = postgres user,
permissions = 0700, same as for the $PGDATA directory).
Not an easy thing to
Andres Freund and...@2ndquadrant.com writes:
Hm. Is a check like that actually sufficient? The idea of setting
stats_temp_directory to /dev/shm/postgres or similar in all of several
clusters on one machine doesn't seem to be that far fetched.
Hm. We could fairly easily have the lockfile
On 2013-08-19 15:25:38 -0400, Tom Lane wrote:
Andres Freund and...@2ndquadrant.com writes:
Hm. Is a check like that actually sufficient? The idea of setting
stats_temp_directory to /dev/shm/postgres or similar in all of several
clusters on one machine doesn't seem to be that far fetched.
Tom,
I note BTW that similar complaints could be lodged against the
log_directory setting. We've not worried about that one too much.
Actually, it does happen that when you change log_directory on a reload,
stuff takes an uneven amount of time to cut over; that is, there's a
few seconds while
On 2013-08-19 12:47:05 -0700, Josh Berkus wrote:
Tom,
I note BTW that similar complaints could be lodged against the
log_directory setting. We've not worried about that one too much.
Actually, it does happen that when you change log_directory on a reload,
stuff takes an uneven amount
Josh Berkus j...@agliodbs.com writes:
Tom,
I note BTW that similar complaints could be lodged against the
log_directory setting. We've not worried about that one too much.
Actually, it does happen that when you change log_directory on a reload,
stuff takes an uneven amount of time to cut
On 2013-08-19 15:51:16 -0400, Tom Lane wrote:
Josh Berkus j...@agliodbs.com writes:
Tom,
I note BTW that similar complaints could be lodged against the
log_directory setting. We've not worried about that one too much.
Actually, it does happen that when you change log_directory on a
Andres Freund and...@2ndquadrant.com writes:
On 2013-08-19 15:51:16 -0400, Tom Lane wrote:
Yeah, the stats temp directory will act pretty much the same way: there
will be an interval where backends might not get the most up-to-date data,
but it's not clear that it's worth the trouble to get it
Tom Lane wrote:
Alvaro Herrera alvhe...@2ndquadrant.com writes:
Here's the second attachment.
This looks good except that it can't tell db_123.statfoo isn't a match.
The scan limit/buffer size needs to be greater than the longest string
you care about, not only equal to. I think strcmp
Alvaro Herrera alvhe...@2ndquadrant.com writes:
Pushed with those fixes, thanks. I noticed we also needed to match
files global.stat and global.tmp. Also I added another conversion to
the sscanf pattern, to ignore files named db_1234.tmp.foo and such
(i.e. stuff after whitespace).
After
On Monday, August 19, 2013, Alvaro Herrera wrote:
Tom Lane wrote:
Alvaro Herrera alvhe...@2ndquadrant.com javascript:; writes:
Here's the second attachment.
This looks good except that it can't tell db_123.statfoo isn't a match.
The scan limit/buffer size needs to be greater than the
Tom Lane wrote:
Alvaro Herrera alvhe...@2ndquadrant.com writes:
Pushed with those fixes, thanks. I noticed we also needed to match
files global.stat and global.tmp. Also I added another conversion to
the sscanf pattern, to ignore files named db_1234.tmp.foo and such
(i.e. stuff after
On Aug 15, 2013 3:44 AM, Tom Lane t...@sss.pgh.pa.us wrote:
Josh Berkus j...@agliodbs.com writes:
Before 9.3, it would delete one specific file from a potentially shared
directory. In 9.3 it deletes the entire contents of a potentially
shared
directory. That is a massive expansion in the
Jeff,
Before 9.3, it would delete one specific file from a potentially shared
directory. In 9.3 it deletes the entire contents of a potentially shared
directory. That is a massive expansion in the surface area for
unintentional deletion. If we will disallow using shared directories
before
Josh Berkus j...@agliodbs.com writes:
Before 9.3, it would delete one specific file from a potentially shared
directory. In 9.3 it deletes the entire contents of a potentially shared
directory. That is a massive expansion in the surface area for
unintentional deletion. If we will disallow
Tom Lane wrote:
I think we should change 9.3 to be restrictive about ownership/permissions
on the stats_temp_directory (ie, require owner = postgres user,
permissions = 0700, same as for the $PGDATA directory). I agree that
back-patching such a change to the older branches is probably not a
On Thu, Apr 25, 2013 at 8:24 AM, Peter Eisentraut pete...@gmx.net wrote:
On 4/25/13 12:09 AM, Tom Lane wrote:
I think we need it fixed to reject any stats_temp_directory that is not
postgres-owned with restrictive permissions. The problem here is not
with what it deletes, it's with the
On 08/13/2013 09:57 AM, Jeff Janes wrote:
Is this a blocker for 9.3?
Why would it be? This issue doesn't originate with 9.3.
If it is a concern of not what is deleted but rather that someone can
inject a poisoned stats file into the directory, does it need to be
back-patched all the way, as
On Tuesday, August 13, 2013, Josh Berkus wrote:
On 08/13/2013 09:57 AM, Jeff Janes wrote:
Is this a blocker for 9.3?
Why would it be? This issue doesn't originate with 9.3.
Before 9.3, it would delete one specific file from a potentially shared
directory. In 9.3 it deletes the entire
On Thu, Apr 25, 2013 at 12:09 AM, Tom Lane t...@sss.pgh.pa.us wrote:
Alvaro Herrera alvhe...@2ndquadrant.com writes:
Jeff Janes escribió:
With the stats file split patch 187492b6c2e8cafc5 introduced in 9.3dev, now
after a crash the postmaster will try to delete all files in the directory
On 4/25/13 12:09 AM, Tom Lane wrote:
I think we need it fixed to reject any stats_temp_directory that is not
postgres-owned with restrictive permissions. The problem here is not
with what it deletes, it's with the insanely insecure configuration.
Yeah, the requirements should be similar to
On 04/25/2013 11:24 AM, Peter Eisentraut wrote:
On 4/25/13 12:09 AM, Tom Lane wrote:
I think we need it fixed to reject any stats_temp_directory that is not
postgres-owned with restrictive permissions. The problem here is not
with what it deletes, it's with the insanely insecure
With the stats file split patch 187492b6c2e8cafc5 introduced in 9.3dev, now
after a crash the postmaster will try to delete all files in the directory
stats_temp_directory. When that is just a subdirectory of PGDATA, this is
fine. But it seems rather hostile when it is set to a shared directory,
Jeff Janes escribió:
With the stats file split patch 187492b6c2e8cafc5 introduced in 9.3dev, now
after a crash the postmaster will try to delete all files in the directory
stats_temp_directory. When that is just a subdirectory of PGDATA, this is
fine. But it seems rather hostile when it is
Alvaro Herrera alvhe...@2ndquadrant.com writes:
Jeff Janes escribió:
With the stats file split patch 187492b6c2e8cafc5 introduced in 9.3dev, now
after a crash the postmaster will try to delete all files in the directory
stats_temp_directory. When that is just a subdirectory of PGDATA, this is
32 matches
Mail list logo
