public inbox for [email protected]
help / color / mirror / Atom feedFrom: Alexander Korotkov <[email protected]>
To: Pavel Borisov <[email protected]>
Cc: Dmitry Koval <[email protected]>
Cc: Noah Misch <[email protected]>
Cc: [email protected]
Subject: Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Date: Wed, 21 Aug 2024 14:55:04 +0300
Message-ID: <CAPpHfdsEtAWhpcPEaUky+gO20=rz0P03OskAS0c8teuTtHP9hQ@mail.gmail.com> (raw)
In-Reply-To: <CALT9ZEHhVt-i1vGL_eCfNqAisTiC6VaM6U-fxP3kD306uPLEcw@mail.gmail.com>
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]>
<CAPpHfdtYgQ8PUZMS043c8USeKcKh5isrAW0QQKMAxQacX-+nuA@mail.gmail.com>
<[email protected]>
<CAPpHfdu+iX5TP65Cyn6vUe8JKGtLmMA=sKKKgE7tB6G6Qk=18Q@mail.gmail.com>
<CALT9ZEHhVt-i1vGL_eCfNqAisTiC6VaM6U-fxP3kD306uPLEcw@mail.gmail.com>
Hi, Pavel!
On Wed, Aug 21, 2024 at 1:48 PM Pavel Borisov <[email protected]> wrote:
> On Mon, 19 Aug 2024 at 02:24, Alexander Korotkov <[email protected]> wrote:
>>
>> On Sat, Aug 10, 2024 at 6:57 PM Dmitry Koval <[email protected]> wrote:
>> > > 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?
>> >
>> > I agree, returning the oid of the newly created relation is the best way
>> > to solve the problem.
>> > (Excuse me, I won't have access to a laptop for the next week - and
>> > won't be able to look at the source code).
>>
>> Thank you for your feedback. Although, I decided QueryCompletion is
>> not a good place for this new field. It looks more appropriate to
>> place it to TableLikeClause, which already contains one relation oid
>> inside. The revised patch is attached.
>
>
> I've looked at the patch v2. Remembering the OID of a relation newly created with LIKE in TableLikeClause seems good to me.
> Check-world passes sucessfully.
Thank you.
> Shouldn't we also modify the TableLikeClause node in gram.y accordingly?
On the one hand, makeNode() uses palloc0() and initializes all fields
with zero anyway. On the other hand, there is already assignment of
relationOid. So, yes I'll add assignment of newRelationOid for the
sake of uniformity.
> For the comments:
> Put the Oid -> Store the OID
> so caller might use it -> for the caller to use it.
Accepted.
> (Maybe also caller -> table create function)
I'll prefer to leave it "caller" as more generic term, which could
also fit potential future usages.
The revised patch is attached. I'm going to push it if no objections.
------
Regards,
Alexander Korotkov
Supabase
Attachments:
[application/octet-stream] v3-0001-Avoid-repeated-table-name-lookups-in-createPartit.patch (3.4K, ../CAPpHfdsEtAWhpcPEaUky+gO20=rz0P03OskAS0c8teuTtHP9hQ@mail.gmail.com/2-v3-0001-Avoid-repeated-table-name-lookups-in-createPartit.patch)
download | inline diff:
From 32d2fb0a65663204e2d8f8efb392063afa174d85 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Sat, 10 Aug 2024 16:19:28 +0300
Subject: [PATCH v3] Avoid repeated table name lookups in
createPartitionTable()
Currently, createPartitionTable() opens newly created table using its name.
This approach is prone to privilege escalation attack, because we might end
up opening another table than we just created.
This commit address the issue above by opening newly created table by its
oid. It appears to be tricky to get a relation oid out of ProcessUtility().
We have to extend TableLikeClause with new newRelationOid field, which is
filled within ProcessUtility() to be further accessed by caller.
Security: CVE-2014-0062
Reported-by: Noah Misch
Discussion: https://postgr.es/m/20240808171351.a9.nmisch%40google.com
Reviewed-by: Dmitry Koval
---
src/backend/commands/tablecmds.c | 3 ++-
src/backend/parser/gram.y | 1 +
src/backend/tcop/utility.c | 6 ++++++
src/include/nodes/parsenodes.h | 1 +
4 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index dfba5f357b8..bcc2d9ae0b3 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -20381,6 +20381,7 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel,
tlc->options = CREATE_TABLE_LIKE_ALL &
~(CREATE_TABLE_LIKE_INDEXES | CREATE_TABLE_LIKE_IDENTITY | CREATE_TABLE_LIKE_STATISTICS);
tlc->relationOid = InvalidOid;
+ tlc->newRelationOid = InvalidOid;
createStmt->tableElts = lappend(createStmt->tableElts, tlc);
/* Need to make a wrapper PlannedStmt. */
@@ -20404,7 +20405,7 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel,
* Open the new partition with no lock, because we already have
* AccessExclusiveLock placed there after creation.
*/
- newRel = table_openrv(newPartName, NoLock);
+ newRel = table_open(tlc->newRelationOid, NoLock);
/*
* We intended to create the partition with the same persistence as the
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index c3f25582c38..b7d98eb9f02 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -4138,6 +4138,7 @@ TableLikeClause:
n->relation = $2;
n->options = $3;
n->relationOid = InvalidOid;
+ n->newRelationOid = InvalidOid;
$$ = (Node *) n;
}
;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index b2ea8125c92..29868ed04c5 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1225,6 +1225,12 @@ ProcessUtilitySlow(ParseState *pstate,
morestmts = expandTableLikeClause(table_rv, like);
stmts = list_concat(morestmts, stmts);
+
+ /*
+ * Store the Oid of newly created relation to the
+ * TableLikeClause for the caller to use it.
+ */
+ like->newRelationOid = address.objectId;
}
else
{
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 85a62b538e5..577c4bfef76 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -754,6 +754,7 @@ typedef struct TableLikeClause
RangeVar *relation;
bits32 options; /* OR of TableLikeOption flags */
Oid relationOid; /* If table has been looked up, its OID */
+ Oid newRelationOid; /* OID of newly created table */
} TableLikeClause;
typedef enum TableLikeOption
--
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], [email protected]
Subject: Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
In-Reply-To: <CAPpHfdsEtAWhpcPEaUky+gO20=rz0P03OskAS0c8teuTtHP9hQ@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