Tom Lane wrote:
> Andres Freund 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 passin
Tom Lane wrote:
> Alvaro Herrera 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).
>
On Monday, August 19, 2013, Alvaro Herrera wrote:
> Tom Lane wrote:
> > Alvaro Herrera > 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
> > yo
Alvaro Herrera 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 reading the sscanf ma
Tom Lane wrote:
> Alvaro Herrera 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 not strncmp would
Andres Freund 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 to be more near
On 2013-08-19 15:51:16 -0400, Tom Lane wrote:
> Josh Berkus 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,
>
Josh Berkus 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 over"; that
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
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 wh
On 2013-08-19 15:25:38 -0400, Tom Lane wrote:
> Andres Freund 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
Andres Freund 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 management code also
writ
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 th
Andres Freund 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 context to
>> check
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 i
Alvaro Herrera 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 that the code should
Alvaro Herrera 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
>>> before we delete
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/permis
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
On Aug 15, 2013 3:44 AM, "Tom Lane" wrote:
>
> Josh Berkus 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
> >>
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
Josh Berkus 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 using share
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
> be
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 entir
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,
On Thu, Apr 25, 2013 at 8: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 insecu
On Thu, Apr 25, 2013 at 12:09 AM, Tom Lane wrote:
> Alvaro Herrera 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
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 configuration.
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
Alvaro Herrera 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
>> fine. But it s
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
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,
l
32 matches
Mail list logo
