Thread: BUG #15541: Use after release in PQprint

BUG #15541: Use after release in PQprint

From
PG Bug reporting form
Date:
The following bug has been logged on the website:

Bug reference:      15541
Logged by:          Pan Bian
Email address:      bianpan2016@163.com
PostgreSQL version: 11.1
Operating system:   Linux
Description:

File: src/interfaces/libpq/fe-print.c
Function: PQprint
Issue details:

The function PQprint releases the file hander fout via pclose or _pclose
when usePipe is true. After that, fout is used again to write message
"fputs("</table>\n", fout);".

For your convenience, I copy-and-paste related code as follows.

 67 void
 68 PQprint(FILE *fout, const PGresult *res, const PQprintOpt *po)
 69 {
               ...
310         if (usePipe)
311         {
312 #ifdef WIN32
313             _pclose(fout);
314 #else
315             pclose(fout);
316 
317 #ifdef ENABLE_THREAD_SAFETY
318             /* we can't easily verify if EPIPE occurred, so say it did
*/
319             if (sigpipe_masked)
320                 pq_reset_sigpipe(&osigset, sigpipe_pending, true);
321 #else
322             pqsignal(SIGPIPE, oldsigpipehandler);
323 #endif                          /* ENABLE_THREAD_SAFETY */
324 #endif                          /* WIN32 */
325         }
326         if (po->html3 && !po->expanded)
327             fputs("</table>\n", fout);
328     }
329 }

Thank you,
Pan Bian


Re: BUG #15541: Use after release in PQprint

From
Tom Lane
Date:
=?utf-8?q?PG_Bug_reporting_form?= <noreply@postgresql.org> writes:
> The function PQprint releases the file hander fout via pclose or _pclose
> when usePipe is true. After that, fout is used again to write message
> "fputs("</table>\n", fout);".

Wow, that is ancient.  It looks like commit edb519b14 of 27-Jul-1996
added the pipe close stanza in the wrong place, and nobody's noticed
it since.  That's probably not so surprising given the multiple
conditions required to trigger it plus the fact that this whole
function is, if not completely dead code, at least not used anywhere
in the PG distribution.  Still, we oughta fix it.

Thanks for the report!

            regards, tom lane


Re: BUG #15541: Use after release in PQprint

From
Tom Lane
Date:
I wrote:
> Wow, that is ancient.  It looks like commit edb519b14 of 27-Jul-1996
> added the pipe close stanza in the wrong place, and nobody's noticed
> it since.  That's probably not so surprising given the multiple
> conditions required to trigger it plus the fact that this whole
> function is, if not completely dead code, at least not used anywhere
> in the PG distribution.  Still, we oughta fix it.

Actually, looking closer at just what those conditions are: the
giant bit of spaghetti code at lines 177-188 will never enable the
pager if po->html3 is on.  Probably that was because of laziness
about counting lines correctly in that case, but anyway it means
the problem situation is unreachable, AFAICS.

It still seems worth fixing in HEAD, in case anyone ever tries to
remove that restriction; but I'm no longer feeling that we need
to back-patch it.

            regards, tom lane


Re: BUG #15541: Use after release in PQprint

From
Alvaro Herrera
Date:
On 2018-Dec-07, Tom Lane wrote:

> It still seems worth fixing in HEAD, in case anyone ever tries to
> remove that restriction; but I'm no longer feeling that we need
> to back-patch it.

I wonder why don't we just remove PQprint altogether.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: BUG #15541: Use after release in PQprint

From
Tom Lane
Date:
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> I wonder why don't we just remove PQprint altogether.

Yeah, I was wondering that too.  AFAICS the whole of fe-print.c
is dead code so far as our own usage is concerned.  Is there any
evidence that outside code is using PQprint, PQprintTuples, or
PQdisplayTuples?  (The latter two aren't even documented...)

Still, I'm not sure if we can remove them outright without it
being considered a library compatibility break by some distros.

            regards, tom lane


Re: BUG #15541: Use after release in PQprint

From
Tom Lane
Date:
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> I wonder why don't we just remove PQprint altogether.

Yeah, I was wondering that too.  AFAICS the whole of fe-print.c
is dead code so far as our own usage is concerned.  Is there any
evidence that outside code is using PQprint, PQprintTuples, or
PQdisplayTuples?  (The latter two aren't even documented...)

Still, I'm not sure if we can remove them outright without it
being considered a library compatibility break by some distros.

            regards, tom lane