[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