On 08/20/2016 10:46 PM, Tom Lane wrote:
Noah Misch <n...@leadboat.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



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to