Re: [HACKERS] [sqlsmith] Crash reading pg_stat_activity

Thomas Munro writes:

> On Wed, Dec 28, 2016 at 10:40 AM, Thomas Munro <thomas.munro@enterprisedb.com> wrote:
>> On Wed, Dec 28, 2016 at 10:01 AM, Andreas Seltenreich <seltenreich@gmx.de> wrote:
>>> testing master as of fe591f8bf6 produced a crash reading
>>> pg_stat_activity (backtrace below).  Digging around with with gdb
>>> revealed that pgstat_get_wait_event() returned an invalid pointer for a
>>> classId PG_WAIT_LWLOCK.
>>>
>>> I think the culprit is dsa.c passing a pointer to memory that goes away
>>> on dsa_free() as a name to LWLockRegisterTranche.
[..]
>> Maybe we should replace it with another value when the DSA area is
>> detached, using a constant string.  Something like

I'm wondering: Is it safe to pass a pointer into a DSA at all?  If I
understand the comments correctly, they are not necessarily mapped (at
the same address) in an unrelated backend looking into pg_stat_activity,
and in this case a dsa_free() is not actually needed to trigger a crash.

> That line of thinking suggests another potential solution: go and
> register the name in RegisterLWLockTranches, and stop registering it
> in dsa.c.  For other potential uses of DSA including extensions that
> call LWLockNewTrancheId() we'd have to decide whether to make the name
> an optional argument, or require those users to register it
> themselves.

Maybe instead of copying the name, just put the passed pointer itself
into the area?  Extensions using LWLockNewTrancheId need to use
shared_preload_libraries anyway, so static strings would be mapped in
all backends.

regards,
Andreas