🌐 AI搜索 & 代理 主页
Skip to content

Commit cc7053a

Browse files
tglsfdctglsfdc
committed
Revert "Avoid race condition between "GRANT role" and "DROP ROLE"".
This reverts commit 98fc31d. That change allowed DROP OWNED BY to drop grants of the target role to other roles, arguing that nobody would need those privileges anymore. But that's not so: if you're not superuser, you still need admin privilege on the target role so you can drop it. It's not clear whether or how the dependency-based approach to solving the original problem can be adapted to keep these grants. Since v18 release is fast approaching, the sanest thing to do seems to be to revert this patch for now. The race-condition problem is low severity and not worth taking risks for. I didn't force a catversion bump in 98fc31d, so I won't do so here either. Reported-by: Dipesh Dhameliya <dipeshdhameliya125@gmail.com> Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CABgZEgczOFicCJoqtrH9gbYMe_BV3Hq8zzCBRcMgmU6LRsihUA@mail.gmail.com Backpatch-through: 18
1 parent 4ad8464 commit cc7053a

File tree

4 files changed

+7
-63
lines changed

4 files changed

+7
-63
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, or other roles) will also be revoked.
36+
parameters) will also be revoked.
3737
</para>
3838
</refsect1>
3939

‎src/backend/commands/user.c‎

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

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

19101909
/*
19111910
* The values for these options can be taken directly from 'popt'.
@@ -1947,22 +1946,12 @@ AddRoleMems(Oid currentUserId, const char *rolename, Oid roleid,
19471946
new_record, new_record_nulls);
19481947
CatalogTupleInsert(pg_authmem_rel, tuple);
19491948

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-
1949+
/* updateAclDependencies wants to pfree array inputs */
1950+
newmembers[0] = grantorId;
19621951
updateAclDependencies(AuthMemRelationId, objectId,
19631952
0, InvalidOid,
19641953
0, NULL,
1965-
nnewmembers, newmembers);
1954+
1, newmembers);
19661955
}
19671956

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

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

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -113,36 +113,6 @@ 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-
146116
GRANT pg_read_all_data TO regress_priv_user6;
147117
GRANT pg_write_all_data TO regress_priv_user7;
148118
GRANT pg_read_all_settings TO regress_priv_user8 WITH ADMIN OPTION;

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

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -90,21 +90,6 @@ CREATE USER regress_priv_user3;
9090
CREATE USER regress_priv_user4;
9191
CREATE USER regress_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::regrole FROM pg_auth_members
97-
WHERE roleid IN ('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::regrole FROM pg_auth_members
101-
WHERE roleid IN ('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::regrole FROM pg_auth_members
105-
WHERE roleid IN ('regress_priv_user1'::regrole,'regress_priv_user2'::regrole)
106-
ORDER BY roleid::regrole::text;
107-
10893
GRANT pg_read_all_data TO regress_priv_user6;
10994
GRANT pg_write_all_data TO regress_priv_user7;
11095
GRANT pg_read_all_settings TO regress_priv_user8 WITH ADMIN OPTION;

0 commit comments

Comments
 (0)