Re: Assertion failure in base backup code path

On 2014-01-07 17:40:07 +0100, Magnus Hagander wrote:
> On Tue, Dec 24, 2013 at 1:24 PM, Andres Freund <andres(at)2ndquadrant(dot)com>wrote:
>
> > On 2013-12-23 18:28:51 +0100, Magnus Hagander wrote:
> > > On Dec 19, 2013 12:06 AM, "Andres Freund" <andres(at)2ndquadrant(dot)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.

> Does the attached seem right to you?

I haven't run the code, but it looks right to me.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services