Thread: BUG #15541: Use after release in PQprint
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
=?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
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
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
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
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