On 2014-01-07 17:40:07 +0100, Magnus Hagander wrote: > On Tue, Dec 24, 2013 at 1:24 PM, Andres Freund <andres@2ndquadrant.com>wrote: > > > On 2013-12-23 18:28:51 +0100, Magnus Hagander wrote: > > > On Dec 19, 2013 12:06 AM, "Andres Freund" <andres@2ndquadrant.com> > > wrote: > > > > > > > > Hi Magnus, > > > > > > > > It looks to me like the path to do_pg_start_backup() outside of a > > > > transaction context comes from your initial commit of the base backup > > > > facility. > > > > The problem is that you're not allowed to do anything leading to a > > > > syscache/catcache lookup in those contexts. > > > > > > I think that may have come with the addition of the replication privilege > > > actually but that doesn't change the fact that yes, it appears broken.. > > > > There was a if (!superuser()) check there before as well, that has the > > same dangers. > > > > > I think the correct fix is to move the security check outside of > do_pg_start_backup() and leave it to the caller. That means > pg_start_backup() for a manual backup. And for a streaming base backup the > check has already been made - you can't get through the authentication step > if you don't have the required permissions.
Yes, that's what I was thinking and was planning to write you about at some point.
Good, then we think the same :)
> Does the attached seem right to you?
I haven't run the code, but it looks right to me.
It worked fine in my testing, so I've pushed that version.
Looks slightly different in both 9.2 and 9.1 (not clean backpatching) due to code reorganization and such, but AFAICT it's just code in different places.