On 2020-08-08 10:44, Michael Paquier wrote:
On Fri, Jul 31, 2020 at 03:23:52PM -0400, Robert Haas wrote:
On Fri, Jul 31, 2020 at 4:25 AM torikoshia
<torikos...@oss.nttdata.com> wrote:
And as Fujii-san told me in person, exposing memory address seems
not preferable considering there are security techniques like
address space layout randomization.
Yeah, exactly. ASLR wouldn't do anything to improve security if there
were no other security bugs, but there are, and some of those bugs are
harder to exploit if you don't know the precise memory addresses of
certain data structures. Similarly, exposing the addresses of our
internal data structures is harmless if we have no other security
bugs, but if we do, it might make those bugs easier to exploit. I
don't think this information is useful enough to justify taking that
risk.
FWIW, this is the class of issues where it is possible to print some
areas of memory, or even manipulate the stack so as it was possible to
pass down a custom pointer, so exposing the pointer locations is a
real risk, and this has happened in the past. Anyway, it seems to me
that if this part is done, we could just make it superuser-only with
restrictive REVOKE privileges, but I am not sure that we have enough
user cases to justify this addition.
Thanks for your comments!
I convinced that exposing pointer locations introduce security risks
and it seems better not to do so.
And I now feel identifying exact memory context by exposing memory
address or other means seems overkill.
Showing just the context name of the parent would be sufficient and
0007 pattch takes this way.
On 2020-08-07 16:38, Kasahara Tatsuhito wrote:
The following review has been posted through the commitfest
application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation: tested, passed
I tested the latest
patch(0007-Adding-a-function-exposing-memory-usage-of-local-backend.patch)
with the latest PG-version (199cec9779504c08aaa8159c6308283156547409)
and test was passed.
It looks good to me.
The new status of this patch is: Ready for Committer
Thanks for your testing!
Regards,
--
Atsushi Torikoshi
NTT DATA CORPORATION
