pgsql: Make MemoryContextContains work correctly again

From: David Rowley <drowley(at)postgresql(dot)org>
To: pgsql-committers(at)lists(dot)postgresql(dot)org
Subject: pgsql: Make MemoryContextContains work correctly again
Date: 2022-09-07 12:20:53
Message-ID: E1oVu32-001qmN-Jp@gemulon.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

Make MemoryContextContains work correctly again

c6e0fe1f2 recently changed the way we store headers for allocated chunks
of memory. Prior to that commit, we stored a pointer to the owning
MemoryContext directly prior to the pointer to the allocated memory.
That's no longer true and c6e0fe1f2 neglected to update
MemoryContextContains() so that it correctly obtains the owning context
with the new method.

A side effect of this change and c6e0fe1f2, in general, is that it's even
less safe than it was previously to pass MemoryContextContains() an
arbitrary pointer which was not allocated by one of our MemoryContexts.
Previously some comments in MemoryContextContains() seemed to indicate
that the worst that could happen by passing an arbitrary pointer would be
a false positive return value. It seems to me that this was a rather
wishful outlook as we subsequently proceeded to subtract sizeof(void *)
from the given pointer and then dereferenced that memory. So it seems
quite likely that we could have segfaulted instead of returning a false
positive. However, it's not impossible that the memory sizeof(void *)
bytes before the pointer could have been owned by the process, but it's
far less likely to work now as obtaining a pointer to the owning
MemoryContext is less direct than before c6e0fe1f2 and will access memory
that's possibly much further away to obtain the owning MemoryContext.
Because of this, I took the liberty of updating the comment to warn
against any future usages of the function and checked the existing core
usages to ensure that we only ever pass in a pointer to memory allocated
by a MemoryContext.

Extension authors updating their code for PG16 who are using
MemoryContextContains should check to ensure that only NULL pointers and
pointers to chunks allocated with a MemoryContext will ever be passed to
MemoryContextContains.

Reported-by: Andres Freund
Discussion: https://postgr.es/m/20220905230949.kb3x2fkpfwtngz43@awork3.anarazel.de

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/5265e91fd10ddbc47921126571ed64914fd3cb72

Modified Files
--------------
src/backend/utils/mmgr/mcxt.c | 45 ++++++++++++++++++++++++++++++++++---------
1 file changed, 36 insertions(+), 9 deletions(-)

Browse pgsql-committers by date

  From Date Subject
Next Message Alvaro Herrera 2022-09-07 15:34:51 pgsql: Message style fixes
Previous Message Justin Pryzby 2022-09-07 09:17:49 Re: [PATCH] Renumber confusing value for GUC_UNIT_BYTE