Re: Re: PROPOSAL: make PostgreSQL sanitizers-friendly (and prevent information disclosure)

On 08/20/2016 10:46 PM, Tom Lane wrote:
> Noah Misch <noah(at)leadboat(dot)com> writes:
>> On Fri, Aug 19, 2016 at 07:22:02PM -0400, Tom Lane wrote:
>>> So maybe we ought to work towards taking those out?
>
>> Maybe. It's a policy question boiling down to our willingness to spend cycles
>> zeroing memory in order to limit trust in the confidentiality of storage
>> backing the data directory. Think of "INSERT INTO t VALUES(my_encrypt('key',
>> 'cleartext'))", subsequent to which bits of the key or cleartext leak to disk
>> by way of WAL padding bytes. A reasonable person might expect that not to
>> happen; GNU Privacy Guard and a few like-minded programs prevent it. I'm on
>> the fence regarding whether PostgreSQL should target this level of vigilance.
>> An RDBMS is mainly a tool for managing persistent data, and PostgreSQL will
>> never be a superior tool for data that _must not_ persist. Having said that,
>> the runtime cost of zeroing memory and the development cost of reviewing the
>> patches to do so is fairly low.
>
> [ after thinking some more about this... ]
>
> FWIW, I put pretty much zero credence in the proposition that junk left in
> padding bytes in WAL or data files represents a meaningful security issue.
> An attacker who has access to those files will probably find much more
> that is of interest in the non-pad data. My only interest here is in
> making the code sanitizer-clean, which seems like it is useful for
> debugging purposes independently of any security arguments.

Yeah, that's how I view these, too.

> So to me, it seems like the core of this complaint boils down to "my
> sanitizer doesn't understand the valgrind exclusion patterns that have
> been created for Postgres". We can address that to some extent by trying
> to reduce the number of valgrind exclusions we need, but it's unlikely to
> be practical to get that to zero, and it's not very clear that adding
> runtime cycles is a good tradeoff for it either. So maybe we need to push
> back on the assumption that people should expect their sanitizers to
> produce zero warnings without having made some effort to adapt the
> valgrind rules.

I'll mark this as "returned with feedback". I'd be happy to take a patch
that helps to reduce sanitizer complaints, but this seems to need some work.

Aleksander, how did you run the sanitizer? I tried to build with clang
4.0, with the -fsanitize=memory option, and ran "make
installcheck-parallel", but I didn't get any sanitizer errors out of it.
I did get some errors, from failing to load "regress.so", though:

ERROR: could not load library
"/home/heikki/git-sandbox-pgsql/master/src/test/regress/regress.so":
/home/heikki/git-sandbox-pgsql/master/src/test/regress/regress.so:
undefined symbol: __msan_va_arg_overflow_size_tls

How did you do it?

- Heikki