From: Tom Lane <tgl@sss.pgh.pa.us>
To: Michael Paquier <michael@paquier.xyz>
cc: Andrew Gierth <andrew@tao11.riddles.org.uk>,
        Jeremy Evans <code@jeremyevans.net>, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #15367: Crash in pg_fe_scram_free when using foreign tables
In-reply-to: <20180908193914.GI32058@paquier.xyz>
References: <20180906222843.GF17425@jeremyevans.local>
 <20180907165103.GH17425@jeremyevans.local>
 <20180907175518.GI17425@jeremyevans.local>
 <20180907181522.GB32058@paquier.xyz> <20016.1536351535@sss.pgh.pa.us>
 <20180907204403.GC32058@paquier.xyz> <16558.1536407783@sss.pgh.pa.us>
 <18398.1536410055@sss.pgh.pa.us> <87o9d8rv08.fsf@news-spur.riddles.org.uk>
 <3979.1536433320@sss.pgh.pa.us> <20180908193914.GI32058@paquier.xyz>
Comments: In-reply-to Michael Paquier <michael@paquier.xyz>
	message dated "Sat, 08 Sep 2018 12:39:14 -0700"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <7632.1536437859.1@sss.pgh.pa.us>
Date: Sat, 08 Sep 2018 16:17:39 -0400
Message-ID: <7633.1536437859@sss.pgh.pa.us>
Precedence: bulk

Michael Paquier <michael@paquier.xyz> writes:
> libpq is quite particular in the fact that it compiles directly with
> stuff in src/common, src/port, etc.  I think that the same problem
> exists for pg_md5_encrypt().  So I like the solution proposed.  I guess
> you have made sure that postgres_fdw becomes stable this way?

It still passes its regression test with the patch, on my OpenBSD image.
See also the reproducer procedure I posted.

The bigger issue we need to worry about is whether applying version-script
symbol filtering would cause issues for third-party code that might be
relying on being able to access libpq "internal" functions on *BSD.
I don't have much sympathy for that and wouldn't hesitate to apply the
filtering in HEAD and v11, but maybe in v10 we ought not do that.

We could possibly band-aid around the problem in v10 by changing the SCRAM
support functions so that they always malloc, never palloc, whether in
frontend or not.  That might lead to memory leaks in the backend case,
but I'm not sure how much we care about memory leaks during backend
authentication failures --- the process would exit shortly anyway.
I've not investigated this idea any further than thinking of it, though,
and I'm not sure it's worth the trouble to avoid symbol filtering.

I'm inclined to add a test in src/test/authentication that exercises
this problem, in hopes of finding out from the buildfarm whether we
have any other platforms with the issue.

			regards, tom lane