Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitea5ae3a

Browse files
committed
Fix misbehavior of DROP OWNED BY with duplicate polroles entries.
Ordinarily, a pg_policy.polroles array wouldn't list the same rolemore than once; but CREATE POLICY does not prevent that. If weperform DROP OWNED BY on a role that is listed more than once,RemoveRoleFromObjectPolicy either suffered an assertion failureor encountered a tuple-updated-by-self error. Rewrite it to copecorrectly with duplicate entries, and add a CommandCounterIncrementcall to prevent the other problem.Per discussion, there's other cleanup that ought to happen here,but this seems like the minimum essential fix.Per bug #17062 from Alexander Lakhin. It's been broken all along,so back-patch to all supported branches.Discussion:https://postgr.es/m/17062-11f471ae3199ca23@postgresql.org
1 parent4b8b356 commitea5ae3a

File tree

3 files changed

+46
-23
lines changed

3 files changed

+46
-23
lines changed

‎src/backend/commands/policy.c

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include"access/htup.h"
1818
#include"access/htup_details.h"
1919
#include"access/sysattr.h"
20+
#include"access/xact.h"
2021
#include"catalog/catalog.h"
2122
#include"catalog/dependency.h"
2223
#include"catalog/indexing.h"
@@ -424,10 +425,9 @@ RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id)
424425
Oidrelid;
425426
Relationrel;
426427
ArrayType*policy_roles;
427-
intnum_roles;
428428
Datumroles_datum;
429429
boolattr_isnull;
430-
boolnoperm= true;
430+
boolkeep_policy= true;
431431

432432
Assert(classid==PolicyRelationId);
433433

@@ -480,31 +480,20 @@ RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id)
480480

481481
policy_roles=DatumGetArrayTypePCopy(roles_datum);
482482

483-
/* We should be removing exactly one entry from the roles array */
484-
num_roles=ARR_DIMS(policy_roles)[0]-1;
485-
486-
Assert(num_roles >=0);
487-
488483
/* Must own relation. */
489-
if (pg_class_ownercheck(relid,GetUserId()))
490-
noperm= false;/* user is allowed to modify this policy */
491-
else
484+
if (!pg_class_ownercheck(relid,GetUserId()))
492485
ereport(WARNING,
493486
(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED),
494487
errmsg("role \"%s\" could not be removed from policy \"%s\" on \"%s\"",
495488
GetUserNameFromId(roleid, false),
496489
NameStr(((Form_pg_policy)GETSTRUCT(tuple))->polname),
497490
RelationGetRelationName(rel))));
498-
499-
/*
500-
* If multiple roles exist on this policy, then remove the one we were
501-
* asked to and leave the rest.
502-
*/
503-
if (!noperm&&num_roles>0)
491+
else
504492
{
505493
inti,
506494
j;
507495
Oid*roles= (Oid*)ARR_DATA_PTR(policy_roles);
496+
intnum_roles=ARR_DIMS(policy_roles)[0];
508497
Datum*role_oids;
509498
char*qual_value;
510499
Node*qual_expr;
@@ -578,16 +567,22 @@ RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id)
578567
else
579568
with_check_qual=NULL;
580569

581-
/* Rebuild the roles array to then update the pg_policy tuple with */
570+
/*
571+
* Rebuild the roles array, without any mentions of the target role.
572+
* Ordinarily there'd be exactly one, but we must cope with duplicate
573+
* mentions, since CREATE/ALTER POLICY historically have allowed that.
574+
*/
582575
role_oids= (Datum*)palloc(num_roles*sizeof(Datum));
583-
for (i=0,j=0;i<ARR_DIMS(policy_roles)[0];i++)
584-
/* Copy over all of the roles which are not the one being removed */
576+
for (i=0,j=0;i<num_roles;i++)
577+
{
585578
if (roles[i]!=roleid)
586579
role_oids[j++]=ObjectIdGetDatum(roles[i]);
580+
}
581+
num_roles=j;
587582

588-
/*We should have only removed theone role */
589-
Assert(j==num_roles);
590-
583+
/*If any roles remain, update thepolicy entry. */
584+
if (num_roles>0)
585+
{
591586
/* This is the array for the new tuple */
592587
role_ids=construct_array(role_oids,num_roles,OIDOID,
593588
sizeof(Oid), true,'i');
@@ -642,8 +637,17 @@ RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id)
642637

643638
heap_freetuple(new_tuple);
644639

640+
/* Make updates visible */
641+
CommandCounterIncrement();
642+
645643
/* Invalidate Relation Cache */
646644
CacheInvalidateRelcache(rel);
645+
}
646+
else
647+
{
648+
/* No roles would remain, so drop the policy instead */
649+
keep_policy= false;
650+
}
647651
}
648652

649653
/* Clean up. */
@@ -653,7 +657,7 @@ RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id)
653657

654658
heap_close(pg_policy_rel,RowExclusiveLock);
655659

656-
return(noperm||num_roles>0);
660+
returnkeep_policy;
657661
}
658662

659663
/*

‎src/test/regress/expected/rowsecurity.out

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3906,6 +3906,15 @@ ERROR: policy "p1" for table "dob_t1" does not exist
39063906
CREATE POLICY p1 ON dob_t1 TO regress_rls_dob_role1,regress_rls_dob_role2 USING (true);
39073907
DROP OWNED BY regress_rls_dob_role1;
39083908
DROP POLICY p1 ON dob_t1; -- should succeed
3909+
-- same cases with duplicate polroles entries
3910+
CREATE POLICY p1 ON dob_t1 TO regress_rls_dob_role1,regress_rls_dob_role1 USING (true);
3911+
DROP OWNED BY regress_rls_dob_role1;
3912+
DROP POLICY p1 ON dob_t1; -- should fail, already gone
3913+
ERROR: policy "p1" for table "dob_t1" does not exist
3914+
CREATE POLICY p1 ON dob_t1 TO regress_rls_dob_role1,regress_rls_dob_role1,regress_rls_dob_role2 USING (true);
3915+
DROP OWNED BY regress_rls_dob_role1;
3916+
DROP POLICY p1 ON dob_t1; -- should succeed
3917+
-- partitioned target
39093918
CREATE POLICY p1 ON dob_t2 TO regress_rls_dob_role1,regress_rls_dob_role2 USING (true);
39103919
DROP OWNED BY regress_rls_dob_role1;
39113920
DROP POLICY p1 ON dob_t2; -- should succeed

‎src/test/regress/sql/rowsecurity.sql

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1760,6 +1760,16 @@ CREATE POLICY p1 ON dob_t1 TO regress_rls_dob_role1,regress_rls_dob_role2 USING
17601760
DROP OWNED BY regress_rls_dob_role1;
17611761
DROP POLICY p1ON dob_t1;-- should succeed
17621762

1763+
-- same cases with duplicate polroles entries
1764+
CREATE POLICY p1ON dob_t1 TO regress_rls_dob_role1,regress_rls_dob_role1 USING (true);
1765+
DROP OWNED BY regress_rls_dob_role1;
1766+
DROP POLICY p1ON dob_t1;-- should fail, already gone
1767+
1768+
CREATE POLICY p1ON dob_t1 TO regress_rls_dob_role1,regress_rls_dob_role1,regress_rls_dob_role2 USING (true);
1769+
DROP OWNED BY regress_rls_dob_role1;
1770+
DROP POLICY p1ON dob_t1;-- should succeed
1771+
1772+
-- partitioned target
17631773
CREATE POLICY p1ON dob_t2 TO regress_rls_dob_role1,regress_rls_dob_role2 USING (true);
17641774
DROP OWNED BY regress_rls_dob_role1;
17651775
DROP POLICY p1ON dob_t2;-- should succeed

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp