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

Commitc8af501

Browse files
Fix lookups in pg_{clear,restore}_{attribute,relation}_stats().
Presently, these functions look up the relation's OID, lock it, andthen check privileges. Not only does this approach provide noguarantee that the locked relation matches the arguments of thelookup, but it also allows users to briefly lock relations forwhich they do not have privileges, which might enabledenial-of-service attacks. This commit adjusts these functions touse RangeVarGetRelidExtended(), which is purpose-built to avoidboth of these issues. The new RangeVarGetRelidCallback function issomewhat complicated because it must handle both tables andindexes, and for indexes, we must check privileges on the parenttable and lock it first. Also, it needs to handle a couple ofextremely unlikely race conditions involving concurrent OID reuse.A downside of this change is that the coding doesn't allow forlocking indexes in AccessShare mode anymore; everything is lockedin ShareUpdateExclusive mode. Per discussion, the original choiceof lock levels was intended for a now defunct implementation thatused in-place updates, so we believe this change is okay.Reviewed-by: Jeff Davis <pgsql@j-davis.com>Discussion:https://postgr.es/m/Z8zwVmGzXyDdkAXj%40nathanBackpatch-through: 18
1 parentb141443 commitc8af501

File tree

5 files changed

+103
-87
lines changed

5 files changed

+103
-87
lines changed

‎src/backend/statistics/attribute_stats.c‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,10 @@
1919

2020
#include"access/heapam.h"
2121
#include"catalog/indexing.h"
22+
#include"catalog/namespace.h"
2223
#include"catalog/pg_collation.h"
2324
#include"catalog/pg_operator.h"
25+
#include"nodes/makefuncs.h"
2426
#include"nodes/nodeFuncs.h"
2527
#include"statistics/statistics.h"
2628
#include"statistics/stat_utils.h"
@@ -143,6 +145,7 @@ attribute_statistics_update(FunctionCallInfo fcinfo)
143145
char*attname;
144146
AttrNumberattnum;
145147
boolinherited;
148+
Oidlocked_table=InvalidOid;
146149

147150
Relationstarel;
148151
HeapTuplestatup;
@@ -182,16 +185,16 @@ attribute_statistics_update(FunctionCallInfo fcinfo)
182185
nspname=TextDatumGetCString(PG_GETARG_DATUM(ATTRELSCHEMA_ARG));
183186
relname=TextDatumGetCString(PG_GETARG_DATUM(ATTRELNAME_ARG));
184187

185-
reloid=stats_lookup_relid(nspname,relname);
186-
187188
if (RecoveryInProgress())
188189
ereport(ERROR,
189190
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
190191
errmsg("recovery is in progress"),
191192
errhint("Statistics cannot be modified during recovery.")));
192193

193194
/* lock before looking up attribute */
194-
stats_lock_check_privileges(reloid);
195+
reloid=RangeVarGetRelidExtended(makeRangeVar(nspname,relname,-1),
196+
ShareUpdateExclusiveLock,0,
197+
RangeVarCallbackForStats,&locked_table);
195198

196199
/* user can specify either attname or attnum, but not both */
197200
if (!PG_ARGISNULL(ATTNAME_ARG))
@@ -917,6 +920,7 @@ pg_clear_attribute_stats(PG_FUNCTION_ARGS)
917920
char*attname;
918921
AttrNumberattnum;
919922
boolinherited;
923+
Oidlocked_table=InvalidOid;
920924

921925
stats_check_required_arg(fcinfo,cleararginfo,C_ATTRELSCHEMA_ARG);
922926
stats_check_required_arg(fcinfo,cleararginfo,C_ATTRELNAME_ARG);
@@ -926,15 +930,15 @@ pg_clear_attribute_stats(PG_FUNCTION_ARGS)
926930
nspname=TextDatumGetCString(PG_GETARG_DATUM(C_ATTRELSCHEMA_ARG));
927931
relname=TextDatumGetCString(PG_GETARG_DATUM(C_ATTRELNAME_ARG));
928932

929-
reloid=stats_lookup_relid(nspname,relname);
930-
931933
if (RecoveryInProgress())
932934
ereport(ERROR,
933935
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
934936
errmsg("recovery is in progress"),
935937
errhint("Statistics cannot be modified during recovery.")));
936938

937-
stats_lock_check_privileges(reloid);
939+
reloid=RangeVarGetRelidExtended(makeRangeVar(nspname,relname,-1),
940+
ShareUpdateExclusiveLock,0,
941+
RangeVarCallbackForStats,&locked_table);
938942

939943
attname=TextDatumGetCString(PG_GETARG_DATUM(C_ATTNAME_ARG));
940944
attnum=get_attnum(reloid,attname);

‎src/backend/statistics/relation_stats.c‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include"access/heapam.h"
2121
#include"catalog/indexing.h"
2222
#include"catalog/namespace.h"
23+
#include"nodes/makefuncs.h"
2324
#include"statistics/stat_utils.h"
2425
#include"utils/builtins.h"
2526
#include"utils/fmgroids.h"
@@ -82,22 +83,23 @@ relation_statistics_update(FunctionCallInfo fcinfo)
8283
Datumvalues[4]= {0};
8384
boolnulls[4]= {0};
8485
intnreplaces=0;
86+
Oidlocked_table=InvalidOid;
8587

8688
stats_check_required_arg(fcinfo,relarginfo,RELSCHEMA_ARG);
8789
stats_check_required_arg(fcinfo,relarginfo,RELNAME_ARG);
8890

8991
nspname=TextDatumGetCString(PG_GETARG_DATUM(RELSCHEMA_ARG));
9092
relname=TextDatumGetCString(PG_GETARG_DATUM(RELNAME_ARG));
9193

92-
reloid=stats_lookup_relid(nspname,relname);
93-
9494
if (RecoveryInProgress())
9595
ereport(ERROR,
9696
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
9797
errmsg("recovery is in progress"),
9898
errhint("Statistics cannot be modified during recovery.")));
9999

100-
stats_lock_check_privileges(reloid);
100+
reloid=RangeVarGetRelidExtended(makeRangeVar(nspname,relname,-1),
101+
ShareUpdateExclusiveLock,0,
102+
RangeVarCallbackForStats,&locked_table);
101103

102104
if (!PG_ARGISNULL(RELPAGES_ARG))
103105
{

‎src/backend/statistics/stat_utils.c‎

Lines changed: 80 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,11 @@
1616

1717
#include"postgres.h"
1818

19+
#include"access/htup_details.h"
1920
#include"access/relation.h"
2021
#include"catalog/index.h"
2122
#include"catalog/namespace.h"
23+
#include"catalog/pg_class.h"
2224
#include"catalog/pg_database.h"
2325
#include"funcapi.h"
2426
#include"miscadmin.h"
@@ -29,6 +31,7 @@
2931
#include"utils/builtins.h"
3032
#include"utils/lsyscache.h"
3133
#include"utils/rel.h"
34+
#include"utils/syscache.h"
3235

3336
/*
3437
* Ensure that a given argument is not null.
@@ -119,53 +122,84 @@ stats_check_arg_pair(FunctionCallInfo fcinfo,
119122
}
120123

121124
/*
122-
* Lock relation in ShareUpdateExclusive mode, check privileges, and close the
123-
* relation (but retain the lock).
124-
*
125125
* A role has privileges to set statistics on the relation if any of the
126126
* following are true:
127127
* - the role owns the current database and the relation is not shared
128128
* - the role has the MAINTAIN privilege on the relation
129129
*/
130130
void
131-
stats_lock_check_privileges(Oidreloid)
131+
RangeVarCallbackForStats(constRangeVar*relation,
132+
OidrelId,OidoldRelId,void*arg)
132133
{
133-
Relationtable;
134-
Oidtable_oid=reloid;
135-
Oidindex_oid=InvalidOid;
136-
LOCKMODEindex_lockmode=NoLock;
134+
Oid*locked_oid= (Oid*)arg;
135+
Oidtable_oid=relId;
136+
HeapTupletuple;
137+
Form_pg_classform;
138+
charrelkind;
137139

138140
/*
139-
* For indexes, we follow the locking behavior in do_analyze_rel() and
140-
* check_lock_if_inplace_updateable_rel(), which is to lock the table
141-
* first in ShareUpdateExclusive mode and then the index in AccessShare
142-
* mode.
143-
*
144-
* Partitioned indexes are treated differently than normal indexes in
145-
* check_lock_if_inplace_updateable_rel(), so we take a
146-
* ShareUpdateExclusive lock on both the partitioned table and the
147-
* partitioned index.
141+
* If we previously locked some other index's heap, and the name we're
142+
* looking up no longer refers to that relation, release the now-useless
143+
* lock.
148144
*/
149-
switch (get_rel_relkind(reloid))
145+
if (relId!=oldRelId&&OidIsValid(*locked_oid))
150146
{
151-
caseRELKIND_INDEX:
152-
index_oid=reloid;
153-
table_oid=IndexGetRelation(index_oid, false);
154-
index_lockmode=AccessShareLock;
155-
break;
156-
caseRELKIND_PARTITIONED_INDEX:
157-
index_oid=reloid;
158-
table_oid=IndexGetRelation(index_oid, false);
159-
index_lockmode=ShareUpdateExclusiveLock;
160-
break;
161-
default:
162-
break;
147+
UnlockRelationOid(*locked_oid,ShareUpdateExclusiveLock);
148+
*locked_oid=InvalidOid;
149+
}
150+
151+
/* If the relation does not exist, there's nothing more to do. */
152+
if (!OidIsValid(relId))
153+
return;
154+
155+
/* If the relation does exist, check whether it's an index. */
156+
relkind=get_rel_relkind(relId);
157+
if (relkind==RELKIND_INDEX||
158+
relkind==RELKIND_PARTITIONED_INDEX)
159+
table_oid=IndexGetRelation(relId, false);
160+
161+
/*
162+
* If retrying yields the same OID, there are a couple of extremely
163+
* unlikely scenarios we need to handle.
164+
*/
165+
if (relId==oldRelId)
166+
{
167+
/*
168+
* If a previous lookup found an index, but the current lookup did
169+
* not, the index was dropped and the OID was reused for something
170+
* else between lookups. In theory, we could simply drop our lock on
171+
* the index's parent table and proceed, but in the interest of
172+
* avoiding complexity, we just error.
173+
*/
174+
if (table_oid==relId&&OidIsValid(*locked_oid))
175+
ereport(ERROR,
176+
(errcode(ERRCODE_UNDEFINED_OBJECT),
177+
errmsg("index \"%s\" was concurrently dropped",
178+
relation->relname)));
179+
180+
/*
181+
* If the current lookup found an index but a previous lookup either
182+
* did not find an index or found one with a different parent
183+
* relation, the relation was dropped and the OID was reused for an
184+
* index between lookups. RangeVarGetRelidExtended() will have
185+
* already locked the index at this point, so we can't just lock the
186+
* newly discovered parent table OID without risking deadlock. As
187+
* above, we just error in this case.
188+
*/
189+
if (table_oid!=relId&&table_oid!=*locked_oid)
190+
ereport(ERROR,
191+
(errcode(ERRCODE_UNDEFINED_OBJECT),
192+
errmsg("index \"%s\" was concurrently created",
193+
relation->relname)));
163194
}
164195

165-
table=relation_open(table_oid,ShareUpdateExclusiveLock);
196+
tuple=SearchSysCache1(RELOID,ObjectIdGetDatum(table_oid));
197+
if (!HeapTupleIsValid(tuple))
198+
elog(ERROR,"cache lookup failed for OID %u",table_oid);
199+
form= (Form_pg_class)GETSTRUCT(tuple);
166200

167201
/* the relkinds that can be used with ANALYZE */
168-
switch (table->rd_rel->relkind)
202+
switch (form->relkind)
169203
{
170204
caseRELKIND_RELATION:
171205
caseRELKIND_MATVIEW:
@@ -176,62 +210,36 @@ stats_lock_check_privileges(Oid reloid)
176210
ereport(ERROR,
177211
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
178212
errmsg("cannot modify statistics for relation \"%s\"",
179-
RelationGetRelationName(table)),
180-
errdetail_relkind_not_supported(table->rd_rel->relkind)));
213+
NameStr(form->relname)),
214+
errdetail_relkind_not_supported(form->relkind)));
181215
}
182216

183-
if (OidIsValid(index_oid))
184-
{
185-
Relationindex;
186-
187-
Assert(index_lockmode!=NoLock);
188-
index=relation_open(index_oid,index_lockmode);
189-
190-
Assert(index->rd_index&&index->rd_index->indrelid==table_oid);
191-
192-
/* retain lock on index */
193-
relation_close(index,NoLock);
194-
}
195-
196-
if (table->rd_rel->relisshared)
217+
if (form->relisshared)
197218
ereport(ERROR,
198219
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
199220
errmsg("cannot modify statistics for shared relation")));
200221

222+
/* Check permissions */
201223
if (!object_ownercheck(DatabaseRelationId,MyDatabaseId,GetUserId()))
202224
{
203-
AclResultaclresult=pg_class_aclcheck(RelationGetRelid(table),
225+
AclResultaclresult=pg_class_aclcheck(table_oid,
204226
GetUserId(),
205227
ACL_MAINTAIN);
206228

207229
if (aclresult!=ACLCHECK_OK)
208230
aclcheck_error(aclresult,
209-
get_relkind_objtype(table->rd_rel->relkind),
210-
NameStr(table->rd_rel->relname));
231+
get_relkind_objtype(form->relkind),
232+
NameStr(form->relname));
211233
}
212234

213-
/* retain lock on table */
214-
relation_close(table,NoLock);
215-
}
235+
ReleaseSysCache(tuple);
216236

217-
/*
218-
* Lookup relation oid from schema and relation name.
219-
*/
220-
Oid
221-
stats_lookup_relid(constchar*nspname,constchar*relname)
222-
{
223-
Oidnspoid;
224-
Oidreloid;
225-
226-
nspoid=LookupExplicitNamespace(nspname, false);
227-
reloid=get_relname_relid(relname,nspoid);
228-
if (!OidIsValid(reloid))
229-
ereport(ERROR,
230-
(errcode(ERRCODE_UNDEFINED_TABLE),
231-
errmsg("relation \"%s.%s\" does not exist",
232-
nspname,relname)));
233-
234-
returnreloid;
237+
/* Lock heap before index to avoid deadlock. */
238+
if (relId!=oldRelId&&table_oid!=relId)
239+
{
240+
LockRelationOid(table_oid,ShareUpdateExclusiveLock);
241+
*locked_oid=table_oid;
242+
}
235243
}
236244

237245

‎src/include/statistics/stat_utils.h‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515

1616
#include"fmgr.h"
1717

18+
/* avoid including primnodes.h here */
19+
typedefstructRangeVarRangeVar;
20+
1821
structStatsArgInfo
1922
{
2023
constchar*argname;
@@ -30,9 +33,8 @@ extern bool stats_check_arg_pair(FunctionCallInfo fcinfo,
3033
structStatsArgInfo*arginfo,
3134
intargnum1,intargnum2);
3235

33-
externvoidstats_lock_check_privileges(Oidreloid);
34-
35-
externOidstats_lookup_relid(constchar*nspname,constchar*relname);
36+
externvoidRangeVarCallbackForStats(constRangeVar*relation,
37+
OidrelId,OidoldRelid,void*arg);
3638

3739
externboolstats_fill_fcinfo_from_arg_pairs(FunctionCallInfopairs_fcinfo,
3840
FunctionCallInfopositional_fcinfo,

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -120,9 +120,9 @@ WHERE relation = 'stats_import.test'::regclass AND
120120
SELECT mode FROM pg_locks
121121
WHERE relation = 'stats_import.test_i'::regclass AND
122122
pid = pg_backend_pid() AND granted;
123-
mode
124-
-----------------
125-
AccessShareLock
123+
mode
124+
--------------------------
125+
ShareUpdateExclusiveLock
126126
(1 row)
127127

128128
COMMIT;

0 commit comments

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