From: | Christian Ullrich <chris(at)chrullrich(dot)net> |
---|---|
To: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> |
Cc: | "pgsql-bugs(at)postgresql(dot)org" <pgsql-bugs(at)postgresql(dot)org>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Re: [HACKERS] BUG #13854: SSPI authentication failure: wrong realm name used |
Date: | 2016-03-24 20:58:08 |
Message-ID: | AM3PR06MB069696FF646703B41159B6C6D4820@AM3PR06MB0696.eurprd06.prod.outlook.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-bugs pgsql-hackers |
* From: Alvaro Herrera [mailto:alvherre(at)2ndquadrant(dot)com]
> Christian Ullrich wrote:
> > * Christian Ullrich wrote:
> >
> > >* From: Magnus Hagander [mailto:magnus(at)hagander(dot)net]
>
> > >>Code uses a mix of malloc() and palloc() (through sprintf). Is there
> > >>a reason for that?
> > >
> > >I wasn't sure which to prefer, so I looked around in auth.c, and
> > >other than RADIUS, everything seems to use malloc() (although the
> > >sample size is not too great). Should I use palloc() instead?
> >
> > The single instance of malloc() has been replaced with palloc().
>
> I'm wary of palloc() in this code actually ... if the allocation fails,
> I'm not sure it's okay to use ereport(ERROR) which is what would happen
> with palloc. With the malloc code, you report the problem with
> elog(LOG) and then return STATUS_ERROR which lets the calling code
> handle the failure in a different way. I didn't actually review your
> new code, but I recall this from previous readings of auth code; so if
> you're going to use palloc(), you better audit what happens on OOM.
>
> For the same reason, using psprintf is probably not acceptable either.
To be honest, I'm not sure what can and cannot be done in auth code. I took inspiration from the existing SSPI code and nearly every error check in pg_SSPI_recvauth() ends up doing ereport(ERROR) already, directly or via pg_SSPI_error(). If this could cause serious trouble, someone would have noticed yet.
What *could* happen, anyway? Can ereport(ERROR) in a backend make the postmaster panic badly enough to force a shared memory reset?
--
Christian
From | Date | Subject | |
---|---|---|---|
Next Message | Alvaro Herrera | 2016-03-24 21:22:49 | Re: Re: [HACKERS] BUG #13854: SSPI authentication failure: wrong realm name used |
Previous Message | josnyder | 2016-03-24 20:45:50 | BUG #14044: Queries immediately conflict with recovery when recovery_min_apply_delay is used |
From | Date | Subject | |
---|---|---|---|
Next Message | Petr Jelinek | 2016-03-24 21:12:53 | Re: Sequence Access Method WIP |
Previous Message | Peter Geoghegan | 2016-03-24 20:28:08 | Re: Re: Missing rows with index scan when collation is not "C" (PostgreSQL 9.5) |