public inbox for [email protected]
help / color / mirror / Atom feedFrom: Alexander Korotkov <[email protected]>
To: Dmitry Koval <[email protected]>
Cc: Noah Misch <[email protected]>
Cc: [email protected]
Subject: Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Date: Sat, 10 Aug 2024 18:43:59 +0300
Message-ID: <CAPpHfdtYgQ8PUZMS043c8USeKcKh5isrAW0QQKMAxQacX-+nuA@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <171085360143.2046436.7217841141682511557.pgcf@coridan.postgresql.org>
<171085858750.2046434.12246066729700037354.pgcf@coridan.postgresql.org>
<[email protected]>
<CAPpHfdsVQwnZPxaQPwcu1g_01F0d-o=ECRcf14X3CmavJB6vSQ@mail.gmail.com>
<[email protected]>
<CAPpHfdsCCV8DOv0Ovu4dTSnuWXiEz_AjuW-J6TPj++tcfEGvRw@mail.gmail.com>
<[email protected]>
<CAPpHfduNYkMVuUSxZjD6u4tZG6aKdhgQcmhr-K9OQfeg56MNww@mail.gmail.com>
<[email protected]>
<CAPpHfdtj7YsPaASoVPN+N3H4_Ct+kQw8QY1d_9u7FPnbghkicw@mail.gmail.com>
<[email protected]>
<[email protected]>
On Fri, Aug 9, 2024 at 10:18 AM Dmitry Koval <[email protected]> wrote:
> > This breaks from the CVE-2014-0062 (commit 5f17304) principle of not repeating
> > name lookups. The attached demo uses this defect to make one partition have
> > two parents.
>
> Thank you very much for information (especially for the demo)!
>
> I'm not sure that we can get the identifier of the newly created
> partition from the ProcessUtility() function...
> Maybe it would be enough to check that the new partition is located in
> the namespace in which we created it (see attachment)?
The new partition doesn't necessarily get created in the same
namespace as parent partition. I think it would be better to somehow
open partition by its oid.
It would be quite unfortunate to replicate significant part of
ProcessUtilitySlow(). So, the question is how to get the oid of newly
created relation from ProcessUtility(). I don't like to change the
signature of ProcessUtility() especially as a part of backpatch. So,
I tried to fit this into existing parameters. Probably
QueryCompletion struct fits this purpose best from the existing
parameters. Attached draft patch implements returning oid of newly
created relation as part of QueryCompletion. Thoughts?
------
Regards,
Alexander Korotkov
Supabase
Attachments:
[application/x-patch] v1-0001-Fix-createPartitionTable-security-issue.patch (2.7K, ../CAPpHfdtYgQ8PUZMS043c8USeKcKh5isrAW0QQKMAxQacX-+nuA@mail.gmail.com/2-v1-0001-Fix-createPartitionTable-security-issue.patch)
download | inline diff:
From 83b6d8e38d680daa952542f6ae4a41ae48491a62 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Sat, 10 Aug 2024 16:19:28 +0300
Subject: [PATCH v1] Fix createPartitionTable() security issue
---
src/backend/commands/tablecmds.c | 6 ++++--
src/backend/tcop/utility.c | 2 ++
src/include/tcop/cmdtag.h | 3 +++
3 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 1f94f4fdbbc..c64b4953da8 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -20329,6 +20329,7 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel,
TableLikeClause *tlc;
PlannedStmt *wrapper;
Relation newRel;
+ QueryCompletion qc;
/* If existing rel is temp, it must belong to this session */
if (modelRel->rd_rel->relpersistence == RELPERSISTENCE_TEMP &&
@@ -20373,6 +20374,7 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel,
wrapper->stmt_location = context->pstmt->stmt_location;
wrapper->stmt_len = context->pstmt->stmt_len;
+ qc.commandTag = CMDTAG_UNKNOWN;
ProcessUtility(wrapper,
context->queryString,
false,
@@ -20380,13 +20382,13 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel,
NULL,
NULL,
None_Receiver,
- NULL);
+ &qc);
/*
* Open the new partition with no lock, because we already have
* AccessExclusiveLock placed there after creation.
*/
- newRel = table_openrv(newPartName, NoLock);
+ newRel = table_open(qc.tableAddress.objectId, NoLock);
/*
* We intended to create the partition with the same persistence as the
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index b2ea8125c92..e8a83e0c569 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1165,6 +1165,8 @@ ProcessUtilitySlow(ParseState *pstate,
RELKIND_RELATION,
InvalidOid, NULL,
queryString);
+ if (qc)
+ qc->tableAddress = address;
EventTriggerCollectSimpleCommand(address,
secondaryObject,
stmt);
diff --git a/src/include/tcop/cmdtag.h b/src/include/tcop/cmdtag.h
index 23c99d7eca6..a964a8c0cb7 100644
--- a/src/include/tcop/cmdtag.h
+++ b/src/include/tcop/cmdtag.h
@@ -13,6 +13,8 @@
#ifndef CMDTAG_H
#define CMDTAG_H
+#include "catalog/objectaddress.h"
+
/* buffer size required for command completion tags */
#define COMPLETION_TAG_BUFSIZE 64
@@ -29,6 +31,7 @@ typedef enum CommandTag
typedef struct QueryCompletion
{
CommandTag commandTag;
+ ObjectAddress tableAddress;
uint64 nprocessed;
} QueryCompletion;
--
2.39.3 (Apple Git-146)
view thread (20+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
In-Reply-To: <CAPpHfdtYgQ8PUZMS043c8USeKcKh5isrAW0QQKMAxQacX-+nuA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox