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
Appearance settings

Commit b7e51b3

Browse filesBrowse files
MasaoFujiiMasaoFujii
committed
Make inherited LOCK TABLE perform access permission checks on parent table only.
Previously, LOCK TABLE command through a parent table checked the permissions on not only the parent table but also the children tables inherited from it. This was a bug and inherited queries should perform access permission checks on the parent table only. This commit fixes LOCK TABLE so that it does not check the permission on children tables. This patch is applied only in the master branch. We decided not to back-patch because it's not hard to imagine that there are some applications expecting the old behavior and the change breaks their security. Author: Amit Langote Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/CAHGQGwE+GauyG7POtRfRwwthAGwTjPQYdFHR97+LzA4RHGnJxA@mail.gmail.com
1 parent 958f9fb commit b7e51b3
Copy full SHA for b7e51b3

File tree

Expand file treeCollapse file tree

5 files changed

+37
-27
lines changed
Filter options
Expand file treeCollapse file tree

5 files changed

+37
-27
lines changed

‎src/backend/commands/lockcmds.c

Copy file name to clipboardExpand all lines: src/backend/commands/lockcmds.c
+11-22Lines changed: 11 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
#include "utils/lsyscache.h"
2929
#include "utils/syscache.h"
3030

31-
static void LockTableRecurse(Oid reloid, LOCKMODE lockmode, bool nowait, Oid userid);
31+
static void LockTableRecurse(Oid reloid, LOCKMODE lockmode, bool nowait);
3232
static AclResult LockTableAclCheck(Oid relid, LOCKMODE lockmode, Oid userid);
3333
static void RangeVarCallbackForLockTable(const RangeVar *rv, Oid relid,
3434
Oid oldrelid, void *arg);
@@ -59,7 +59,7 @@ LockTableCommand(LockStmt *lockstmt)
5959
if (get_rel_relkind(reloid) == RELKIND_VIEW)
6060
LockViewRecurse(reloid, lockstmt->mode, lockstmt->nowait, NIL);
6161
else if (recurse)
62-
LockTableRecurse(reloid, lockstmt->mode, lockstmt->nowait, GetUserId());
62+
LockTableRecurse(reloid, lockstmt->mode, lockstmt->nowait);
6363
}
6464
}
6565

@@ -108,35 +108,26 @@ RangeVarCallbackForLockTable(const RangeVar *rv, Oid relid, Oid oldrelid,
108108
/*
109109
* Apply LOCK TABLE recursively over an inheritance tree
110110
*
111-
* We use find_inheritance_children not find_all_inheritors to avoid taking
112-
* locks far in advance of checking privileges. This means we'll visit
113-
* multiply-inheriting children more than once, but that's no problem.
111+
* This doesn't check permission to perform LOCK TABLE on the child tables,
112+
* because getting here means that the user has permission to lock the
113+
* parent which is enough.
114114
*/
115115
static void
116-
LockTableRecurse(Oid reloid, LOCKMODE lockmode, bool nowait, Oid userid)
116+
LockTableRecurse(Oid reloid, LOCKMODE lockmode, bool nowait)
117117
{
118118
List *children;
119119
ListCell *lc;
120120

121-
children = find_inheritance_children(reloid, NoLock);
121+
children = find_all_inheritors(reloid, NoLock, NULL);
122122

123123
foreach(lc, children)
124124
{
125125
Oid childreloid = lfirst_oid(lc);
126-
AclResult aclresult;
127126

128-
/* Check permissions before acquiring the lock. */
129-
aclresult = LockTableAclCheck(childreloid, lockmode, userid);
130-
if (aclresult != ACLCHECK_OK)
131-
{
132-
char *relname = get_rel_name(childreloid);
133-
134-
if (!relname)
135-
continue; /* child concurrently dropped, just skip it */
136-
aclcheck_error(aclresult, get_relkind_objtype(get_rel_relkind(childreloid)), relname);
137-
}
127+
/* Parent already locked. */
128+
if (childreloid == reloid)
129+
continue;
138130

139-
/* We have enough rights to lock the relation; do so. */
140131
if (!nowait)
141132
LockRelationOid(childreloid, lockmode);
142133
else if (!ConditionalLockRelationOid(childreloid, lockmode))
@@ -162,8 +153,6 @@ LockTableRecurse(Oid reloid, LOCKMODE lockmode, bool nowait, Oid userid)
162153
UnlockRelationOid(childreloid, lockmode);
163154
continue;
164155
}
165-
166-
LockTableRecurse(childreloid, lockmode, nowait, userid);
167156
}
168157
}
169158

@@ -241,7 +230,7 @@ LockViewRecurse_walker(Node *node, LockViewRecurse_context *context)
241230
if (relkind == RELKIND_VIEW)
242231
LockViewRecurse(relid, context->lockmode, context->nowait, context->ancestor_views);
243232
else if (rte->inh)
244-
LockTableRecurse(relid, context->lockmode, context->nowait, context->viewowner);
233+
LockTableRecurse(relid, context->lockmode, context->nowait);
245234
}
246235

247236
return query_tree_walker(query,

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

Copy file name to clipboardExpand all lines: src/test/regress/expected/lock.out
+7-3Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,15 +138,19 @@ CREATE TABLE lock_tbl3 () INHERITS (lock_tbl2);
138138
BEGIN TRANSACTION;
139139
LOCK TABLE lock_tbl1 * IN ACCESS EXCLUSIVE MODE;
140140
ROLLBACK;
141-
-- Verify that we can't lock a child table just because we have permission
142-
-- on the parent, but that we can lock the parent only.
141+
-- Child tables are locked without granting explicit permission to do so as
142+
-- long as we have permission to lock the parent.
143143
GRANT UPDATE ON TABLE lock_tbl1 TO regress_rol_lock1;
144144
SET ROLE regress_rol_lock1;
145+
-- fail when child locked directly
145146
BEGIN;
146-
LOCK TABLE lock_tbl1 * IN ACCESS EXCLUSIVE MODE;
147+
LOCK TABLE lock_tbl2;
147148
ERROR: permission denied for table lock_tbl2
148149
ROLLBACK;
149150
BEGIN;
151+
LOCK TABLE lock_tbl1 * IN ACCESS EXCLUSIVE MODE;
152+
ROLLBACK;
153+
BEGIN;
150154
LOCK TABLE ONLY lock_tbl1;
151155
ROLLBACK;
152156
RESET ROLE;

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

Copy file name to clipboardExpand all lines: src/test/regress/expected/privileges.out
+7Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -716,6 +716,13 @@ ERROR: permission denied for table atestc
716716
TRUNCATE atestp1; -- ok
717717
TRUNCATE atestc; -- fail
718718
ERROR: permission denied for table atestc
719+
BEGIN;
720+
LOCK atestp1;
721+
END;
722+
BEGIN;
723+
LOCK atestc;
724+
ERROR: permission denied for table atestc
725+
END;
719726
-- privileges on functions, languages
720727
-- switch to superuser
721728
\c -

‎src/test/regress/sql/lock.sql

Copy file name to clipboardExpand all lines: src/test/regress/sql/lock.sql
+6-2Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,10 +101,14 @@ BEGIN TRANSACTION;
101101
LOCK TABLE lock_tbl1 * IN ACCESS EXCLUSIVE MODE;
102102
ROLLBACK;
103103

104-
-- Verify that we can't lock a child table just because we have permission
105-
-- on the parent, but that we can lock the parent only.
104+
-- Child tables are locked without granting explicit permission to do so as
105+
-- long as we have permission to lock the parent.
106106
GRANT UPDATE ON TABLE lock_tbl1 TO regress_rol_lock1;
107107
SET ROLE regress_rol_lock1;
108+
-- fail when child locked directly
109+
BEGIN;
110+
LOCK TABLE lock_tbl2;
111+
ROLLBACK;
108112
BEGIN;
109113
LOCK TABLE lock_tbl1 * IN ACCESS EXCLUSIVE MODE;
110114
ROLLBACK;

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

Copy file name to clipboardExpand all lines: src/test/regress/sql/privileges.sql
+6Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,12 @@ UPDATE atestp1 SET f1 = 1; -- ok
459459
UPDATE atestc SET f1 = 1; -- fail
460460
TRUNCATE atestp1; -- ok
461461
TRUNCATE atestc; -- fail
462+
BEGIN;
463+
LOCK atestp1;
464+
END;
465+
BEGIN;
466+
LOCK atestc;
467+
END;
462468

463469
-- privileges on functions, languages
464470

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.