From: Sergey Shinderuk <s.shinderuk@postgrespro.ru>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Amit Langote <amitlangote09@gmail.com>
Cc: o.tselebrovskiy@postgrespro.ru
Cc: pgsql-hackers@postgresql.org
Subject: Re: A bug with ExecCheckPermissions
Date: Thu, 9 Feb 2023 12:14:44 +0300
Message-ID: <03cac228-9bcc-00ee-ff76-c097c7dea493@postgrespro.ru> (raw)
In-Reply-To: <20230208182318.pduek52uzl2zq7o4@alvherre.pgsql>
References: <20230208182318.pduek52uzl2zq7o4@alvherre.pgsql>
On 08.02.2023 21:23, Alvaro Herrera wrote:
> On 2023-Feb-08, Amit Langote wrote:
>
>> On Wed, Feb 8, 2023 at 16:19 Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
>>> I think we should also patch ExecCheckPermissions to use forboth(),
>>> scanning the RTEs as it goes over the perminfos, and make sure that the
>>> entries are consistent.
>>
>> Hmm, we can’t use forboth here, because not all RTEs have the corresponding
>> RTEPermissionInfo, inheritance children RTEs, for example.
>
> Doh, of course.
>
>> Also, it doesn’t make much sense to reinstate the original loop over
>> range table and fetch the RTEPermissionInfo for the RTEs with non-0
>> perminfoindex, because the main goal of the patch was to make
>> ExecCheckPermissions() independent of range table length.
>
> Yeah, I'm thinking in a mechanism that would allow us to detect bugs in
> development builds — no need to have it run in production builds.
> However, I can't see any useful way to implement it.
>
Maybe something like the attached would do?
--
Sergey Shinderuk https://postgrespro.com/Attachments:check-rteperminfos.diff (text/plain, 1.9K) downloaddiff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.cindex 39bfb48dc22..94866743f48 100644--- a/src/backend/executor/execMain.c+++ b/src/backend/executor/execMain.c@@ -577,6 +577,28 @@ ExecCheckPermissions(List *rangeTable, List *rteperminfos,
ListCell *l;
bool result = true;
+#ifdef USE_ASSERT_CHECKING+ Bitmapset *indexset = NULL;++ /* Check that rteperminfos is consistent with rangeTable */+ foreach(l, rangeTable)+ {+ RangeTblEntry *rte = lfirst_node(RangeTblEntry, l);++ if (rte->perminfoindex != 0)+ {+ /* Sanity checks */+ (void) getRTEPermissionInfo(rteperminfos, rte);+ /* Many-to-one mapping not allowed */+ Assert(!bms_is_member(rte->perminfoindex, indexset));+ indexset = bms_add_member(indexset, rte->perminfoindex);+ }+ }++ /* All rteperminfos are referenced */+ Assert(bms_num_members(indexset) == list_length(rteperminfos));+#endif+
foreach(l, rteperminfos)
{
RTEPermissionInfo *perminfo = lfirst_node(RTEPermissionInfo, l);
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.cindex 375b17b9f3b..f6d19226a0a 100644--- a/src/backend/utils/adt/ri_triggers.c+++ b/src/backend/utils/adt/ri_triggers.c@@ -1399,6 +1399,7 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
pkrte->relid = RelationGetRelid(pk_rel);
pkrte->relkind = pk_rel->rd_rel->relkind;
pkrte->rellockmode = AccessShareLock;
+ pkrte->perminfoindex = 2;
pk_perminfo = makeNode(RTEPermissionInfo);
pk_perminfo->relid = RelationGetRelid(pk_rel);
@@ -1409,6 +1410,7 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
fkrte->relid = RelationGetRelid(fk_rel);
fkrte->relkind = fk_rel->rd_rel->relkind;
fkrte->rellockmode = AccessShareLock;
+ fkrte->perminfoindex = 1;
fk_perminfo = makeNode(RTEPermissionInfo);
fk_perminfo->relid = RelationGetRelid(fk_rel);
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: pgsql-hackers@postgresql.org
Cc: s.shinderuk@postgrespro.ru, alvherre@alvh.no-ip.org, amitlangote09@gmail.com, o.tselebrovskiy@postgrespro.ru
Subject: Re: A bug with ExecCheckPermissions
In-Reply-To: <03cac228-9bcc-00ee-ff76-c097c7dea493@postgrespro.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox