Re: libpq problems in CVS

Started by Bruce Momjianalmost 26 years ago2 messageshackers
Jump to latest
#1Bruce Momjian
bruce@momjian.us

Can someone comment on this?

PQsetenvPoll has a very bad bug in it. It assumes that the passed-in
PGconn object has a valid setenv_handle if it is non-NULL. This is
true only when it is called from PQconnectdb and friends.

The bad code in PQsetenvPoll is this:

PGsetenvHandle handle = conn->setenv_handle;
...
if (!handle || handle->state == SETENV_STATE_FAILED)
return PGRES_POLLING_FAILED;

After a connection is successfully established, setenv_handle points
to a free(3)'ed handle. Neither PQsetenv, nor PQsetenvStart correctly
update this field with a new setenvHandle. Here is a short test case
demonstrating the memory corruption.

#include <libpq-fe.h>
#include <stdio.h>

main()
{
foo(0);
}

foo(i)
int i;
{
PGconn *P;

P = PQconnectdb("");
if (!P || PQstatus(P) != CONNECTION_OK) {
fprintf(stderr, "connectdb failed\n");
return;
}

PQsetenv(P);
PQfinish(P);

if (i < 1000) {
foo(i+1);
}
}

(gdb) where
#0 0x4007e683 in chunk_free (ar_ptr=0x4010ba80, p=0x80516b0) at malloc.c:3057
#1 0x4007e408 in __libc_free (mem=0x80516c8) at malloc.c:2959
#2 0x4001fce9 in freePGconn () from /usr/local/pgsql/lib/libpq.so.2.1
#3 0x4001fe4d in PQfinish () from /usr/local/pgsql/lib/libpq.so.2.1
#4 0x8048693 in foo ()
#5 0x80486ac in foo ()
#6 0x8048620 in main ()
#7 0x400454be in __libc_start_main (main=0x8048610 <main>, argc=1,
argv=0xbffff8c4, init=0x804846c <_init>, fini=0x80486f4 <_fini>,
rtld_fini=0x4000a130 <_dl_fini>, stack_end=0xbffff8bc)
at ../sysdeps/generic/libc-start.c:90

One fix is to add a `conn->setenv = handle' to PQsetenvStart before
returning, but that won't protect in the case of PQsetenvPoll being
called without a corresponding PQsetenvStart first. Perhaps the
interface should be revisited. Do you really need to store the
setenvHandle in a PGconn? There is no existing way to safely free
setenvHandles.

This bug was also in 7.0beta1.

In the latest patches, an encoding field has been added to the
PGresult object. May I respectfully request an accessor function be
added to retrieve it?

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#1)

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Can someone comment on this?

PQsetenvPoll has a very bad bug in it. It assumes that the passed-in
PGconn object has a valid setenv_handle if it is non-NULL. This is
true only when it is called from PQconnectdb and friends.

Problem is gone: we don't export PQsetenvPoll anymore.

regards, tom lane