Re: Creating a function for exposing memory usage of backend process

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
>> <torikoshia(at)oss(dot)nttdata(dot)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