================================================== ==========================
POSTGRESQL BUG REPORT TEMPLATE
================================================== ==========================


Your name : Olaf Hartmann
Your email address : hao (AT) telemedsys (DOT) de


System Configuration
---------------------
Architecture (example: Intel Pentium) : i386

Operating System (example: Linux 2.0.26 ELF) : linux-2.4.21

PostgreSQL version (example: PostgreSQL-7.3.4): PostgreSQL-7.4

Compiler used (example: gcc 2.95.2) : gcc 3.3.2


Please enter a FULL description of your problem:
------------------------------------------------

We expirience a memory leak with every connection when using an SSL
encrypted TCP connection to connect a postgres database.
A simple code example is included in this mail. It simply opens and
closes a database connection. (You also need a postgresql server set up
for ssl.)

'valgrind' detects the main memory leak (which gets bigger with every
connection) to be in libcrypto.so.0.9.7. But you can also verify the
increased memory consumption using 'ps axl', if you add a 'sleep(..);'
at the end of the program.


Please describe a way to repeat the problem. Please try to provide a
concise reproducible example, if at all possible:
----------------------------------------------------------------------


/*
* begin leak.c
*/

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

int main()
{
int i;
PGconn *conn;

for (i = 0; i < 10; i ++)
{
conn = PQconnectdb("host=... dbname=template1 ... requiressl=1");
if (PQstatus(conn) == CONNECTION_BAD)
{
fprintf(stderr, "Error: %s", PQerrorMessage(conn));
PQfinish(conn);
exit(1);
}
PQfinish(conn);
}
return 0;
}

/*
* end leak.c
*/


$ gcc leak.c -lpq -o leak


$ valgrind --leak-check=yes --error-limit=no ./leak


If you know how this problem might be fixed, list the solution below:
---------------------------------------------------------------------





---------------------------(end of broadcast)---------------------------
TIP 8: explain analyze is your friend

Olaf Hartmann <hao (AT) telemedsys (DOT) de> writes:
I can verify this locally. Unfortunately, my copy of valgrind doesn't
seem to be picking up the debugging symbols for OpenSSL, which makes
it difficult to track down if this is a fault in OpenSSL or libpq.

If someone else wants to tackle this, go ahead -- my plate is full
until around December 10th. Otherwise I'll look into it then.

-Neil


---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faqs/FAQ.html

--=-=-=

Okay, I've attached a patch that fixes the problem for me. The problem
turned out to be pretty simple: the PostgreSQL code (both backend and
frontend SSL support) was calling SSL_get_peer_certificate() without
properly free'ing its return value.

I haven't actually confirmed the backend memory leak, but it should be
readily reproduceable (the same OpenSSL API call is made and the
return value is never free'd).

Olaf, can you please test the attached patch? (Against CVS HEAD, but
should apply easily enough to 7.4.0) At the very least you'll need to
rebuild libpq and ensure that the test app picks up the new .so, but
testing the backend with the patch applied would also be helpful.

This fix needs to be in 7.4.1, so if anyone wants to review this
patch, that would be great.

-Neil

P.S. I added an assertion in the backend code to help catch any other
memory leaks in this area. I didn't add an equivalent one to the
frontend code, because libpq doesn't seem to be setup for
assertions. When this is applied to 7.4.1, we probably shouldn't
include the assertion at the risk of suffering backend crashes.


--=-=-=
Content-Type: text/x-patch
Content-Disposition: attachment; filename=ssl-libpq-mem-leak-1.patch

Index: src/backend/libpq/be-secure.c
================================================== =================
RCS file: /var/lib/cvs/pgsql-server/src/backend/libpq/be-secure.c,v
retrieving revision 1.44
diff -c -r1.44 be-secure.c
*** src/backend/libpq/be-secure.c 29 Nov 2003 19:51:49 -0000 1.44
--- src/backend/libpq/be-secure.c 13 Dec 2003 01:09:13 -0000
***************
*** 714,719 ****
--- 714,722 ----
static int
open_server_SSL(Port *port)
{
+ Assert(!port->ssl);
+ Assert(!port->peer);
+
if (!(port->ssl = SSL_new(SSL_context)) ||
!SSL_set_fd(port->ssl, port->sock) ||
SSL_accept(port->ssl) <= 0)
***************
*** 764,769 ****
--- 767,778 ----
SSL_free(port->ssl);
port->ssl = NULL;
}
+
+ if (port->peer)
+ {
+ X509_free(port->peer);
+ port->peer = NULL;
+ }
}

/*
Index: src/interfaces/libpq/fe-secure.c
================================================== =================
RCS file: /var/lib/cvs/pgsql-server/src/interfaces/libpq/fe-secure.c,v
retrieving revision 1.33
diff -c -r1.33 fe-secure.c
*** src/interfaces/libpq/fe-secure.c 29 Nov 2003 19:52:12 -0000 1.33
--- src/interfaces/libpq/fe-secure.c 13 Dec 2003 00:57:56 -0000
***************
*** 1004,1009 ****
--- 1004,1015 ----
SSL_free(conn->ssl);
conn->ssl = NULL;
}
+
+ if (conn->peer)
+ {
+ X509_free(conn->peer);
+ conn->peer = NULL;
+ }
}

/*

--=-=-=
Content-Type: text/plain
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0


---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo (AT) postgresql (DOT) org so that your
message can get through to the mailing list cleanly

--=-=-=--

Neil Conway <neilc (AT) samurai (DOT) com> writes:
Looks reasonable from here, but confirmation from the complainant that
it fixes his problem would be real good ...

Yeah, we can't assume that a decent support mechanism exists in an
arbitrary client application.

Assert away. People who are concerned about assertions destabilizing
their systems don't build with asserts enabled in the first place.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo (AT) postgresql (DOT) org so that your
message can get through to the mailing list cleanly

Tom Lane wrote:
Good. I have given up getting an update from the original bug reporter.

--
Bruce Momjian | http://candle.pha.pa.us
pgman (AT) candle (DOT) pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to majordomo (AT) postgresql (DOT) org