Обсуждение: [BUGFIX] Fix crash due to sizeof bug in RegisterExtensionExplainOption

Hi hackers,

The allocations in src/backend/commands/explain_state.c
used sizeof(char *) instead of sizeof(ExplainExtensionOption),
which could cause a crash if an extension would register
more than 8 extension EXPLAIN options:

Attached small example extension, test_explain_state.txt,
to demonstrate the bug:

LOAD 'test_explain_state';
LOAD
EXPLAIN (test_explain_opt9) SELECT 1;
server closed the connection unexpectedly
    This probably means the server terminated abnormally
    before or while processing the request.
connection to server was lost

/Joel

On Sun, Mar 01, 2026 at 06:10:10PM +0100, Joel Jacobson wrote:
> The allocations in src/backend/commands/explain_state.c
> used sizeof(char *) instead of sizeof(ExplainExtensionOption),
> which could cause a crash if an extension would register
> more than 8 extension EXPLAIN options:

Indeed, that's wrong as-is.  The problem can be reproduced simply by
saving more options into pg_overexplain, as well, leading to the same
memory chunk issues.  Will fix, thanks for the report.
--
Michael

On 3/2/26 4:18 AM, Michael Paquier wrote:
> On Sun, Mar 01, 2026 at 06:10:10PM +0100, Joel Jacobson wrote:
>> The allocations in src/backend/commands/explain_state.c
>> used sizeof(char *) instead of sizeof(ExplainExtensionOption),
>> which could cause a crash if an extension would register
>> more than 8 extension EXPLAIN options:
> 
> Indeed, that's wrong as-is.  The problem can be reproduced simply by
> saving more options into pg_overexplain, as well, leading to the same
> memory chunk issues.  Will fix, thanks for the report.

Shouldn't the patch have used repalloc_array()? If the code had done so 
in the first place the bug would never have happened.

-- 
Andreas Karlsson
Percona

On Tue, Mar 03, 2026 at 04:39:43AM +0100, Andreas Karlsson wrote:
> Shouldn't the patch have used repalloc_array()? If the code had done so in
> the first place the bug would never have happened.

I was considering that first.  However, after looked at the
MemoryContextAlloc() and the inconsistency that repalloc_array() was
bringing, I have turned the thought away.
--
Michael

On 3/3/26 5:13 AM, Michael Paquier wrote:
> On Tue, Mar 03, 2026 at 04:39:43AM +0100, Andreas Karlsson wrote:
>> Shouldn't the patch have used repalloc_array()? If the code had done so in
>> the first place the bug would never have happened.
> 
> I was considering that first.  However, after looked at the
> MemoryContextAlloc() and the inconsistency that repalloc_array() was
> bringing, I have turned the thought away.

Ah, makes sense but then the question is if we should make 
MemoryContextAlloc() more consistent with palloc() but that is a bigger 
question than just this small bugfix.

Andreas