Use-after-free in 12- EventTriggerAlterTableEnd

From: Arseny Sher <a(dot)sher(at)postgrespro(dot)ru>
To: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Cc: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Subject: Use-after-free in 12- EventTriggerAlterTableEnd
Date: 2020-10-27 09:36:24
Message-ID: 877drcyprb.fsf@ars-thinkpad
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

Valgrind on our internal buildfarm complained about use-after-free
during currentEventTriggerState->commandList manipulations, e.g. lappend
in EventTriggerCollectSimpleCommand. I've discovered that the source of
problem is EventTriggerAlterTableEnd not bothering to switch into its
own context before appending to the list. ced138e8cba fixed this in
master and 13 but wasn't backpatched further, so I see the problem in
12-.

The particular reproducing scenario is somewhat involved; it combines
pg_pathman [1] extension and SQL interface to it created in our fork of
Postgres. Namely, we allow to create partitions in CREATE TABLE
PARTITIONED by statement (similar to what [2] proposes). Each partition
is created via separate SPI call which in turn ends up making
AlterTableStmt as ProcessUtility PROCESS_UTILITY_SUBCOMMAND; so
EventTriggerAlterTableStart/End is done, but additional
EventTriggerQueryState is not allocated and commands are collected in
toplevel EventTriggerQueryState. Of course SPI frees its memory between
the calls which makes valgrind scream.

Admittedly our case is tricky and I'm not sure it is possible to make up
something like that in the pure core code, but I do believe other
extension writers might run into this, so I propose to backpatch (the
attached) 3 lines healing to all supported branches.

Technically, all you (an extension author) have to do to encounter this
is
1) Have toplevel EvenTriggerQueryState, i.e. catch utility statement.
2) Inside it, run AlterTable as PROCESS_UTILITY_SUBCOMMAND in some
short-living context.

[1] https://github.com/postgrespro/pg_pathman
[2] https://www.postgresql.org/message-id/flat/CALT9ZEFBv05OhLMKO1Lbo_Zg9a0v%2BU9q9twe%3Dt-dixfR45RmVQ%40mail.gmail.com#f86f0fcfa62d5108fb81556a43f42457

Attachment Content-Type Size
use_after_free_EventTriggerAlterTableEnd.diff text/x-diff 736 bytes

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Simon Riggs 2020-10-27 09:43:57 Re: Deleting older versions in unique indexes to avoid page splits
Previous Message tsunakawa.takay@fujitsu.com 2020-10-27 09:30:36 RE: Multiple hosts in connection string failed to failover in non-hot standby mode