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

Commit98fc31d

Browse files
committed
Avoid race condition between "GRANT role" and "DROP ROLE".
Concurrently dropping either the granted role or the granteedoes not stop GRANT from completing, instead resulting in adangling role reference in pg_auth_members. That's relativelyharmless in the short run, but inconsistent catalog entriesare not a good thing.This patch solves the problem by adding the granted and granteeroles as explicit shared dependencies of the pg_auth_members entry.That's a bit indirect, but it works because the pg_shdepend codeapplies the necessary locking and rechecking.Commit6566133 previously established similar handling forthe grantor column of pg_auth_members; it's not clear why itdidn't cover the other two role OID columns.A side-effect of this approach is that DROP OWNED BY will now droppg_auth_members entries that mention the target role as either thegranted or grantee role. That's clearly appropriate for thegrantee, since we'll drop its other privileges too. It doesn'tseem too far out of line for the granted role, since we'represumably about to drop it and besides we're removing all reasonswhy it'd matter to be a member of it. (One could argue that thismakes DropRole's code to auto-drop pg_auth_members entriesunnecessary, but I chose to leave it in place since perhaps somepeople's workflows expect that to work without a DROP OWNED BY.)Note to patch readers: CreateRole's first CommandCounterIncrementcall is now unconditional, because this change creates anothercase in which it's needed, and it seemed to be more trouble thanit's worth to preserve that micro-optimization.Arguably this is a bug fix, but the fact that it changes theexpected contents of pg_shdepend seems like not a great thingto do in the stable branches, and perhaps we don't want thechange in DROP OWNED BY semantics there either. On the otherhand, I opted not to force a catversion bump in HEAD, becausethe presence or absence of these entries doesn't matter formost purposes.Reported-by: Virender Singla <virender.cse@gmail.com>Reviewed-by: Laurenz Albe <laurenz.albe@cybertec.at>Discussion:https://postgr.es/m/CAM6Zo8woa62ZFHtMKox6a4jb8qQ=w87R2L0K8347iE-juQL2EA@mail.gmail.com
1 parentddb17e3 commit98fc31d

File tree

4 files changed

+63
-7
lines changed

4 files changed

+63
-7
lines changed

‎doc/src/sgml/ref/drop_owned.sgml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ DROP OWNED BY { <replaceable class="parameter">name</replaceable> | CURRENT_ROLE
3333
database that are owned by one of the specified roles. Any
3434
privileges granted to the given roles on objects in the current
3535
database or on shared objects (databases, tablespaces, configuration
36-
parameters) will also be revoked.
36+
parameters, or other roles) will also be revoked.
3737
</para>
3838
</refsect1>
3939

‎src/backend/commands/user.c

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include"commands/defrem.h"
3131
#include"commands/seclabel.h"
3232
#include"commands/user.h"
33+
#include"lib/qunique.h"
3334
#include"libpq/crypt.h"
3435
#include"miscadmin.h"
3536
#include"storage/lmgr.h"
@@ -489,8 +490,7 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
489490
* Advance command counter so we can see new record; else tests in
490491
* AddRoleMems may fail.
491492
*/
492-
if (addroleto||adminmembers||rolemembers)
493-
CommandCounterIncrement();
493+
CommandCounterIncrement();
494494

495495
/* Default grant. */
496496
InitGrantRoleOptions(&popt);
@@ -1904,7 +1904,8 @@ AddRoleMems(Oid currentUserId, const char *rolename, Oid roleid,
19041904
else
19051905
{
19061906
OidobjectId;
1907-
Oid*newmembers=palloc(sizeof(Oid));
1907+
Oid*newmembers= (Oid*)palloc(3*sizeof(Oid));
1908+
intnnewmembers;
19081909

19091910
/*
19101911
* The values for these options can be taken directly from 'popt'.
@@ -1946,12 +1947,22 @@ AddRoleMems(Oid currentUserId, const char *rolename, Oid roleid,
19461947
new_record,new_record_nulls);
19471948
CatalogTupleInsert(pg_authmem_rel,tuple);
19481949

1949-
/* updateAclDependencies wants to pfree array inputs */
1950-
newmembers[0]=grantorId;
1950+
/*
1951+
* Record dependencies on the roleid, member, and grantor, as if a
1952+
* pg_auth_members entry were an object ACL.
1953+
* updateAclDependencies() requires an input array that is
1954+
* palloc'd (it will free it), sorted, and de-duped.
1955+
*/
1956+
newmembers[0]=roleid;
1957+
newmembers[1]=memberid;
1958+
newmembers[2]=grantorId;
1959+
qsort(newmembers,3,sizeof(Oid),oid_cmp);
1960+
nnewmembers=qunique(newmembers,3,sizeof(Oid),oid_cmp);
1961+
19511962
updateAclDependencies(AuthMemRelationId,objectId,
19521963
0,InvalidOid,
19531964
0,NULL,
1954-
1,newmembers);
1965+
nnewmembers,newmembers);
19551966
}
19561967

19571968
/* CCI after each change, in case there are duplicates in list */

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,36 @@ CREATE USER regress_priv_user2;
113113
CREATE USER regress_priv_user3;
114114
CREATE USER regress_priv_user4;
115115
CREATE USER regress_priv_user5;
116+
-- DROP OWNED should also act on granted and granted-to roles
117+
GRANT regress_priv_user1 TO regress_priv_user2;
118+
GRANT regress_priv_user2 TO regress_priv_user3;
119+
SELECT roleid::regrole, member::regrole FROM pg_auth_members
120+
WHERE roleid IN ('regress_priv_user1'::regrole,'regress_priv_user2'::regrole)
121+
ORDER BY roleid::regrole::text;
122+
roleid | member
123+
--------------------+--------------------
124+
regress_priv_user1 | regress_priv_user2
125+
regress_priv_user2 | regress_priv_user3
126+
(2 rows)
127+
128+
REASSIGN OWNED BY regress_priv_user2 TO regress_priv_user4; -- no effect
129+
SELECT roleid::regrole, member::regrole FROM pg_auth_members
130+
WHERE roleid IN ('regress_priv_user1'::regrole,'regress_priv_user2'::regrole)
131+
ORDER BY roleid::regrole::text;
132+
roleid | member
133+
--------------------+--------------------
134+
regress_priv_user1 | regress_priv_user2
135+
regress_priv_user2 | regress_priv_user3
136+
(2 rows)
137+
138+
DROP OWNED BY regress_priv_user2; -- removes both grants
139+
SELECT roleid::regrole, member::regrole FROM pg_auth_members
140+
WHERE roleid IN ('regress_priv_user1'::regrole,'regress_priv_user2'::regrole)
141+
ORDER BY roleid::regrole::text;
142+
roleid | member
143+
--------+--------
144+
(0 rows)
145+
116146
GRANT pg_read_all_data TO regress_priv_user6;
117147
GRANT pg_write_all_data TO regress_priv_user7;
118148
GRANT pg_read_all_settings TO regress_priv_user8 WITH ADMIN OPTION;

‎src/test/regress/sql/privileges.sql

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,21 @@ CREATE USER regress_priv_user3;
9090
CREATEUSERregress_priv_user4;
9191
CREATEUSERregress_priv_user5;
9292

93+
-- DROP OWNED should also act on granted and granted-to roles
94+
GRANT regress_priv_user1 TO regress_priv_user2;
95+
GRANT regress_priv_user2 TO regress_priv_user3;
96+
SELECT roleid::regrole, member::regroleFROM pg_auth_members
97+
WHERE roleidIN ('regress_priv_user1'::regrole,'regress_priv_user2'::regrole)
98+
ORDER BY roleid::regrole::text;
99+
REASSIGN OWNED BY regress_priv_user2 TO regress_priv_user4;-- no effect
100+
SELECT roleid::regrole, member::regroleFROM pg_auth_members
101+
WHERE roleidIN ('regress_priv_user1'::regrole,'regress_priv_user2'::regrole)
102+
ORDER BY roleid::regrole::text;
103+
DROP OWNED BY regress_priv_user2;-- removes both grants
104+
SELECT roleid::regrole, member::regroleFROM pg_auth_members
105+
WHERE roleidIN ('regress_priv_user1'::regrole,'regress_priv_user2'::regrole)
106+
ORDER BY roleid::regrole::text;
107+
93108
GRANT pg_read_all_data TO regress_priv_user6;
94109
GRANT pg_write_all_data TO regress_priv_user7;
95110
GRANT pg_read_all_settings TO regress_priv_user8 WITH ADMIN OPTION;

0 commit comments

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