From 4dc84bb5fb0d33a5f6fedd31828adda9db8fd2d4 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Thu, 26 Jul 2018 11:51:45 +0900 Subject: [PATCH 1/3] Refactor TRUNCATE execution to avoid early lock lookups A caller of TRUNCATE can perform early lookup obtention which can cause other sessions to block on the request done, causing potentially DOS attacks as even a non-privileged user can attempt a truncation of a critical catalog table to block even all incoming connection attempts. This commit fixes the case of TRUNCATE so as RangeVarGetRelidExtended is used with a callback doing the necessary checks at an earlier stage, avoiding locking issues at early stages. Author: Michael Paquier Discussion: https://postgr.es/m/152512087100.19803.12733865831237526317@wrigleys.postgresql.org --- src/backend/commands/tablecmds.c | 83 +++++++++++++++++++++++++------- 1 file changed, 65 insertions(+), 18 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index eb2d33dd86..b06c9dbf63 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -300,7 +300,10 @@ struct DropRelationCallbackState #define child_dependency_type(child_is_partition) \ ((child_is_partition) ? DEPENDENCY_AUTO : DEPENDENCY_NORMAL) -static void truncate_check_rel(Relation rel); +static void truncate_check_rel(Oid relid, Form_pg_class reltuple); +static void truncate_check_session(Relation rel); +static void RangeVarCallbackForTruncate(const RangeVar *relation, + Oid relId, Oid oldRelId, void *arg); static List *MergeAttributes(List *schema, List *supers, char relpersistence, bool is_partition, List **supOids, List **supconstr, int *supOidCount); @@ -1336,15 +1339,25 @@ ExecuteTruncate(TruncateStmt *stmt) bool recurse = rv->inh; Oid myrelid; - rel = heap_openrv(rv, AccessExclusiveLock); - myrelid = RelationGetRelid(rel); + myrelid = RangeVarGetRelidExtended(rv, AccessExclusiveLock, + false, RangeVarCallbackForTruncate, + NULL); + + /* open the relation, we already hold a lock on it */ + rel = heap_open(myrelid, NoLock); + /* don't throw error for "TRUNCATE foo, foo" */ if (list_member_oid(relids, myrelid)) { heap_close(rel, AccessExclusiveLock); continue; } - truncate_check_rel(rel); + + /* + * RangeVarGetRelidExtended has done some basic checks with its + * callback, and only remain session-level checks. + */ + truncate_check_session(rel); rels = lappend(rels, rel); relids = lappend_oid(relids, myrelid); /* Log this relation only if needed for logical decoding */ @@ -1367,7 +1380,9 @@ ExecuteTruncate(TruncateStmt *stmt) /* find_all_inheritors already got lock */ rel = heap_open(childrelid, NoLock); - truncate_check_rel(rel); + truncate_check_rel(RelationGetRelid(rel), rel->rd_rel); + truncate_check_session(rel); + rels = lappend(rels, rel); relids = lappend_oid(relids, childrelid); /* Log this relation only if needed for logical decoding */ @@ -1450,7 +1465,8 @@ ExecuteTruncateGuts(List *explicit_rels, List *relids, List *relids_logged, ereport(NOTICE, (errmsg("truncate cascades to table \"%s\"", RelationGetRelationName(rel)))); - truncate_check_rel(rel); + truncate_check_rel(relid, rel->rd_rel); + truncate_check_session(rel); rels = lappend(rels, rel); relids = lappend_oid(relids, relid); /* Log this relation only if needed for logical decoding */ @@ -1700,38 +1716,47 @@ ExecuteTruncateGuts(List *explicit_rels, List *relids, List *relids_logged, } /* - * Check that a given rel is safe to truncate. Subroutine for ExecuteTruncate + * Check that a given relation is safe to truncate. Subroutine for + * ExecuteTruncate and RangeVarCallbackForTruncate. */ static void -truncate_check_rel(Relation rel) +truncate_check_rel(Oid relid, Form_pg_class reltuple) { AclResult aclresult; + char *relname = NameStr(reltuple->relname); /* * Only allow truncate on regular tables and partitioned tables (although, * the latter are only being included here for the following checks; no * physical truncation will occur in their case.) */ - if (rel->rd_rel->relkind != RELKIND_RELATION && - rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE) + if (reltuple->relkind != RELKIND_RELATION && + + reltuple->relkind != RELKIND_PARTITIONED_TABLE) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("\"%s\" is not a table", - RelationGetRelationName(rel)))); + errmsg("\"%s\" is not a table", relname))); /* Permissions checks */ - aclresult = pg_class_aclcheck(RelationGetRelid(rel), GetUserId(), - ACL_TRUNCATE); + aclresult = pg_class_aclcheck(relid, GetUserId(), ACL_TRUNCATE); if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, get_relkind_objtype(rel->rd_rel->relkind), - RelationGetRelationName(rel)); + aclcheck_error(aclresult, get_relkind_objtype(reltuple->relkind), + relname); - if (!allowSystemTableMods && IsSystemRelation(rel)) + if (!allowSystemTableMods && IsSystemClass(relid, reltuple)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied: \"%s\" is a system catalog", - RelationGetRelationName(rel)))); + relname))); +} +/* + * Set of sanity checks at session-level to check if a given relation + * is safe to truncate. + */ +static void +truncate_check_session(Relation rel) +{ /* * Don't allow truncate on temp tables of other backends ... their local * buffer manager is not going to cope. @@ -13419,6 +13444,28 @@ RangeVarCallbackOwnsTable(const RangeVar *relation, aclcheck_error(ACLCHECK_NOT_OWNER, get_relkind_objtype(get_rel_relkind(relId)), relation->relname); } +/* + * Callback to RangeVarGetRelidExtended() for TRUNCATE processing. + */ +static void +RangeVarCallbackForTruncate(const RangeVar *relation, + Oid relId, Oid oldRelId, void *arg) +{ + HeapTuple tuple; + + /* Nothing to do if the relation was not found. */ + if (!OidIsValid(relId)) + return; + + tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relId)); + if (!HeapTupleIsValid(tuple)) /* should not happen */ + elog(ERROR, "cache lookup failed for relation %u", relId); + + truncate_check_rel(relId, (Form_pg_class) GETSTRUCT(tuple)); + + ReleaseSysCache(tuple); +} + /* * Callback to RangeVarGetRelidExtended(), similar to * RangeVarCallbackOwnsTable() but without checks on the type of the relation. -- 2.18.0