Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 2af07e2

Browse files
jeff-davisjeff-davis
committed
Fix search_path to a safe value during maintenance operations.
While executing maintenance operations (ANALYZE, CLUSTER, REFRESH MATERIALIZED VIEW, REINDEX, or VACUUM), set search_path to 'pg_catalog, pg_temp' to prevent inconsistent behavior. Functions that are used for functional indexes, in index expressions, or in materialized views and depend on a different search path must be declared with CREATE FUNCTION ... SET search_path='...'. This change was previously committed as 05e1737, then reverted in commit 2fcc7ee because it was too late in the cycle. Preparation for the MAINTAIN privilege, which was previously reverted due to search_path manipulation hazards. Discussion: https://postgr.es/m/d4ccaf3658cb3c281ec88c851a09733cd9482f22.camel@j-davis.com Discussion: https://postgr.es/m/E1q7j7Y-000z1H-Hr%40gemulon.postgresql.org Discussion: https://postgr.es/m/e44327179e5c9015c8dda67351c04da552066017.camel%40j-davis.com Reviewed-by: Greg Stark, Nathan Bossart, Noah Misch
1 parent 2c29e7f commit 2af07e2

30 files changed

+200
-32
lines changed

‎contrib/amcheck/t/004_verify_nbtree_unique.pl

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,11 @@
2020
'postgres', q(
2121
CREATE EXTENSION amcheck;
2222
23+
CREATE SCHEMA test_amcheck;
24+
SET search_path = test_amcheck;
25+
2326
CREATE FUNCTION ok_cmp (int4, int4)
24-
RETURNS int LANGUAGE sql AS
27+
RETURNS int LANGUAGE sql SET search_path = test_amcheck AS
2528
$$
2629
SELECT
2730
CASE WHEN $1 < $2 THEN -1
@@ -34,7 +37,7 @@
3437
--- Check 1: uniqueness violation.
3538
---
3639
CREATE FUNCTION ok_cmp1 (int4, int4)
37-
RETURNS int LANGUAGE sql AS
40+
RETURNS int LANGUAGE sql SET search_path = test_amcheck AS
3841
$$
3942
SELECT ok_cmp($1, $2);
4043
$$;
@@ -43,7 +46,7 @@
4346
--- Make values 768 and 769 look equal.
4447
---
4548
CREATE FUNCTION bad_cmp1 (int4, int4)
46-
RETURNS int LANGUAGE sql AS
49+
RETURNS int LANGUAGE sql SET search_path = test_amcheck AS
4750
$$
4851
SELECT
4952
CASE WHEN ($1 = 768 AND $2 = 769) OR
@@ -56,13 +59,13 @@
5659
--- Check 2: uniqueness violation without deduplication.
5760
---
5861
CREATE FUNCTION ok_cmp2 (int4, int4)
59-
RETURNS int LANGUAGE sql AS
62+
RETURNS int LANGUAGE sql SET search_path = test_amcheck AS
6063
$$
6164
SELECT ok_cmp($1, $2);
6265
$$;
6366
6467
CREATE FUNCTION bad_cmp2 (int4, int4)
65-
RETURNS int LANGUAGE sql AS
68+
RETURNS int LANGUAGE sql SET search_path = test_amcheck AS
6669
$$
6770
SELECT
6871
CASE WHEN $1 = $2 AND $1 = 400 THEN -1
@@ -74,13 +77,13 @@
7477
--- Check 3: uniqueness violation with deduplication.
7578
---
7679
CREATE FUNCTION ok_cmp3 (int4, int4)
77-
RETURNS int LANGUAGE sql AS
80+
RETURNS int LANGUAGE sql SET search_path = test_amcheck AS
7881
$$
7982
SELECT ok_cmp($1, $2);
8083
$$;
8184
8285
CREATE FUNCTION bad_cmp3 (int4, int4)
83-
RETURNS int LANGUAGE sql AS
86+
RETURNS int LANGUAGE sql SET search_path = test_amcheck AS
8487
$$
8588
SELECT bad_cmp2($1, $2);
8689
$$;
@@ -142,22 +145,23 @@
142145
# We have not yet broken the index, so we should get no corruption
143146
$result = $node->safe_psql(
144147
'postgres', q(
145-
SELECT bt_index_check('bttest_unique_idx1', true, true);
148+
SELECT bt_index_check('test_amcheck.bttest_unique_idx1', true, true);
146149
));
147150
is($result, '', 'run amcheck on non-broken bttest_unique_idx1');
148151

149152
# Change the operator class to use a function which considers certain different
150153
# values to be equal.
151154
$node->safe_psql(
152155
'postgres', q(
156+
SET search_path = test_amcheck;
153157
UPDATE pg_catalog.pg_amproc SET
154158
amproc = 'bad_cmp1'::regproc
155159
WHERE amproc = 'ok_cmp1'::regproc;
156160
));
157161

158162
($result, $stdout, $stderr) = $node->psql(
159163
'postgres', q(
160-
SELECT bt_index_check('bttest_unique_idx1', true, true);
164+
SELECT bt_index_check('test_amcheck.bttest_unique_idx1', true, true);
161165
));
162166
ok( $stderr =~ /index uniqueness is violated for index "bttest_unique_idx1"/,
163167
'detected uniqueness violation for index "bttest_unique_idx1"');
@@ -175,21 +179,22 @@
175179
# but no uniqueness violation.
176180
($result, $stdout, $stderr) = $node->psql(
177181
'postgres', q(
178-
SELECT bt_index_check('bttest_unique_idx2', true, true);
182+
SELECT bt_index_check('test_amcheck.bttest_unique_idx2', true, true);
179183
));
180184
ok( $stderr =~ /item order invariant violated for index "bttest_unique_idx2"/,
181185
'detected item order invariant violation for index "bttest_unique_idx2"');
182186

183187
$node->safe_psql(
184188
'postgres', q(
189+
SET search_path = test_amcheck;
185190
UPDATE pg_catalog.pg_amproc SET
186191
amproc = 'ok_cmp2'::regproc
187192
WHERE amproc = 'bad_cmp2'::regproc;
188193
));
189194

190195
($result, $stdout, $stderr) = $node->psql(
191196
'postgres', q(
192-
SELECT bt_index_check('bttest_unique_idx2', true, true);
197+
SELECT bt_index_check('test_amcheck.bttest_unique_idx2', true, true);
193198
));
194199
ok( $stderr =~ /index uniqueness is violated for index "bttest_unique_idx2"/,
195200
'detected uniqueness violation for index "bttest_unique_idx2"');
@@ -206,7 +211,7 @@
206211
# but no uniqueness violation.
207212
($result, $stdout, $stderr) = $node->psql(
208213
'postgres', q(
209-
SELECT bt_index_check('bttest_unique_idx3', true, true);
214+
SELECT bt_index_check('test_amcheck.bttest_unique_idx3', true, true);
210215
));
211216
ok( $stderr =~ /item order invariant violated for index "bttest_unique_idx3"/,
212217
'detected item order invariant violation for index "bttest_unique_idx3"');
@@ -215,6 +220,7 @@
215220
# with different visibility.
216221
$node->safe_psql(
217222
'postgres', q(
223+
SET search_path = test_amcheck;
218224
DELETE FROM bttest_unique3 WHERE 380 <= i AND i <= 420;
219225
INSERT INTO bttest_unique3 (SELECT * FROM generate_series(380, 420));
220226
INSERT INTO bttest_unique3 VALUES (400);
@@ -228,14 +234,15 @@
228234

229235
$node->safe_psql(
230236
'postgres', q(
237+
SET search_path = test_amcheck;
231238
UPDATE pg_catalog.pg_amproc SET
232239
amproc = 'ok_cmp3'::regproc
233240
WHERE amproc = 'bad_cmp3'::regproc;
234241
));
235242

236243
($result, $stdout, $stderr) = $node->psql(
237244
'postgres', q(
238-
SELECT bt_index_check('bttest_unique_idx3', true, true);
245+
SELECT bt_index_check('test_amcheck.bttest_unique_idx3', true, true);
239246
));
240247
ok( $stderr =~ /index uniqueness is violated for index "bttest_unique_idx3"/,
241248
'detected uniqueness violation for index "bttest_unique_idx3"');

‎contrib/amcheck/verify_nbtree.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,8 @@ bt_index_check_internal(Oid indrelid, bool parentcheck, bool heapallindexed,
313313
SetUserIdAndSecContext(heaprel->rd_rel->relowner,
314314
save_sec_context | SECURITY_RESTRICTED_OPERATION);
315315
save_nestlevel = NewGUCNestLevel();
316+
SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
317+
PGC_S_SESSION);
316318
}
317319
else
318320
{

‎doc/src/sgml/amcheck.sgml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@
2424
to hold, we can expect binary searches on the affected page to
2525
incorrectly guide index scans, resulting in wrong answers to SQL
2626
queries. If the structure appears to be valid, no error is raised.
27+
While these checking functions are run, the <xref
28+
linkend="guc-search-path"/> is temporarily changed to <literal>pg_catalog,
29+
pg_temp</literal>.
2730
</para>
2831
<para>
2932
Verification is performed using the same procedures as those used by

‎doc/src/sgml/brin.sgml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,9 @@
9595
</para>
9696

9797
<para>
98-
Lastly, the following functions can be used:
98+
Lastly, the following functions can be used (while these functions run,
99+
<xref linkend="guc-search-path"/> is temporarily changed to
100+
<literal>pg_catalog, pg_temp</literal>):
99101
<simplelist>
100102
<member>
101103
<function>brin_summarize_new_values(regclass)</function>

‎doc/src/sgml/ref/analyze.sgml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,12 @@ ANALYZE [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] [ <r
205205
(This will not be sufficient if there is heavy update activity.)
206206
</para>
207207

208+
<para>
209+
While <command>ANALYZE</command> is running, the <xref
210+
linkend="guc-search-path"/> is temporarily changed to <literal>pg_catalog,
211+
pg_temp</literal>.
212+
</para>
213+
208214
<para>
209215
<command>ANALYZE</command>
210216
requires only a read lock on the target table, so it can run in

‎doc/src/sgml/ref/cluster.sgml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,12 @@ CLUSTER [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] [ <r
153153
information.
154154
</para>
155155

156+
<para>
157+
While <command>CLUSTER</command> is running, the <xref
158+
linkend="guc-search-path"/> is temporarily changed to <literal>pg_catalog,
159+
pg_temp</literal>.
160+
</para>
161+
156162
<para>
157163
When an index scan is used, a temporary copy of the table is created that
158164
contains the table data in the index order. Temporary copies of each

‎doc/src/sgml/ref/create_index.sgml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -789,6 +789,12 @@ Indexes:
789789
the table to generate statistics for these indexes.
790790
</para>
791791

792+
<para>
793+
While <command>CREATE INDEX</command> is running, the <xref
794+
linkend="guc-search-path"/> is temporarily changed to <literal>pg_catalog,
795+
pg_temp</literal>.
796+
</para>
797+
792798
<para>
793799
For most index methods, the speed of creating an index is
794800
dependent on the setting of <xref linkend="guc-maintenance-work-mem"/>.

‎doc/src/sgml/ref/refresh_materialized_view.sgml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,12 @@ REFRESH MATERIALIZED VIEW [ CONCURRENTLY ] <replaceable class="parameter">name</
9898
will be ordered that way; but <command>REFRESH MATERIALIZED
9999
VIEW</command> does not guarantee to preserve that ordering.
100100
</para>
101+
102+
<para>
103+
While <command>REFRESH MATERIALIZED VIEW</command> is running, the <xref
104+
linkend="guc-search-path"/> is temporarily changed to <literal>pg_catalog,
105+
pg_temp</literal>.
106+
</para>
101107
</refsect1>
102108

103109
<refsect1>

‎doc/src/sgml/ref/reindex.sgml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,12 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { DA
291291
into expensive sequential scans.
292292
</para>
293293

294+
<para>
295+
While <command>REINDEX</command> is running, the <xref
296+
linkend="guc-search-path"/> is temporarily changed to <literal>pg_catalog,
297+
pg_temp</literal>.
298+
</para>
299+
294300
<para>
295301
Reindexing a single index or table requires being the owner of that
296302
index or table. Reindexing a schema or database requires being the

‎doc/src/sgml/ref/vacuum.sgml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -443,6 +443,12 @@ VACUUM [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] [ <re
443443
does not have permission to vacuum.
444444
</para>
445445

446+
<para>
447+
While <command>VACUUM</command> is running, the <xref
448+
linkend="guc-search-path"/> is temporarily changed to <literal>pg_catalog,
449+
pg_temp</literal>.
450+
</para>
451+
446452
<para>
447453
<command>VACUUM</command> cannot be executed inside a transaction block.
448454
</para>

‎src/backend/access/brin/brin.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1412,6 +1412,8 @@ brin_summarize_range(PG_FUNCTION_ARGS)
14121412
SetUserIdAndSecContext(heapRel->rd_rel->relowner,
14131413
save_sec_context | SECURITY_RESTRICTED_OPERATION);
14141414
save_nestlevel = NewGUCNestLevel();
1415+
SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
1416+
PGC_S_SESSION);
14151417
}
14161418
else
14171419
{

‎src/backend/catalog/index.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1464,6 +1464,8 @@ index_concurrently_build(Oid heapRelationId,
14641464
SetUserIdAndSecContext(heapRel->rd_rel->relowner,
14651465
save_sec_context | SECURITY_RESTRICTED_OPERATION);
14661466
save_nestlevel = NewGUCNestLevel();
1467+
SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
1468+
PGC_S_SESSION);
14671469

14681470
indexRelation = index_open(indexRelationId, RowExclusiveLock);
14691471

@@ -3016,6 +3018,9 @@ index_build(Relation heapRelation,
30163018
SetUserIdAndSecContext(heapRelation->rd_rel->relowner,
30173019
save_sec_context | SECURITY_RESTRICTED_OPERATION);
30183020
save_nestlevel = NewGUCNestLevel();
3021+
if (!IsBootstrapProcessingMode())
3022+
SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
3023+
PGC_S_SESSION);
30193024

30203025
/* Set up initial progress report status */
30213026
{
@@ -3351,6 +3356,8 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot)
33513356
SetUserIdAndSecContext(heapRelation->rd_rel->relowner,
33523357
save_sec_context | SECURITY_RESTRICTED_OPERATION);
33533358
save_nestlevel = NewGUCNestLevel();
3359+
SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
3360+
PGC_S_SESSION);
33543361

33553362
indexRelation = index_open(indexId, RowExclusiveLock);
33563363

@@ -3612,6 +3619,8 @@ reindex_index(const ReindexStmt *stmt, Oid indexId,
36123619
SetUserIdAndSecContext(heapRelation->rd_rel->relowner,
36133620
save_sec_context | SECURITY_RESTRICTED_OPERATION);
36143621
save_nestlevel = NewGUCNestLevel();
3622+
SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
3623+
PGC_S_SESSION);
36153624

36163625
if (progress)
36173626
{

‎src/backend/catalog/namespace.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4697,6 +4697,9 @@ check_search_path(char **newval, void **extra, GucSource source)
46974697
void
46984698
assign_search_path(const char *newval, void *extra)
46994699
{
4700+
/* don't access search_path during bootstrap */
4701+
Assert(!IsBootstrapProcessingMode());
4702+
47004703
/*
47014704
* We mark the path as needing recomputation, but don't do anything until
47024705
* it's needed. This avoids trying to do database access during GUC

‎src/backend/commands/analyze.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,8 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
339339
SetUserIdAndSecContext(onerel->rd_rel->relowner,
340340
save_sec_context | SECURITY_RESTRICTED_OPERATION);
341341
save_nestlevel = NewGUCNestLevel();
342+
SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
343+
PGC_S_SESSION);
342344

343345
/* measure elapsed time iff autovacuum logging requires it */
344346
if (AmAutoVacuumWorkerProcess() && params->log_min_duration >= 0)

‎src/backend/commands/cluster.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,8 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
350350
SetUserIdAndSecContext(OldHeap->rd_rel->relowner,
351351
save_sec_context | SECURITY_RESTRICTED_OPERATION);
352352
save_nestlevel = NewGUCNestLevel();
353+
SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
354+
PGC_S_SESSION);
353355

354356
/*
355357
* Since we may open a new transaction for each relation, we have to check

‎src/backend/commands/indexcmds.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -585,6 +585,10 @@ DefineIndex(Oid tableId,
585585

586586
root_save_nestlevel = NewGUCNestLevel();
587587

588+
if (!IsBootstrapProcessingMode())
589+
SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
590+
PGC_S_SESSION);
591+
588592
/*
589593
* Some callers need us to run with an empty default_tablespace; this is a
590594
* necessary hack to be able to reproduce catalog state accurately when
@@ -1340,6 +1344,8 @@ DefineIndex(Oid tableId,
13401344
SetUserIdAndSecContext(childrel->rd_rel->relowner,
13411345
child_save_sec_context | SECURITY_RESTRICTED_OPERATION);
13421346
child_save_nestlevel = NewGUCNestLevel();
1347+
SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
1348+
PGC_S_SESSION);
13431349

13441350
/*
13451351
* Don't try to create indexes on foreign tables, though. Skip
@@ -3881,6 +3887,8 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
38813887
SetUserIdAndSecContext(heapRel->rd_rel->relowner,
38823888
save_sec_context | SECURITY_RESTRICTED_OPERATION);
38833889
save_nestlevel = NewGUCNestLevel();
3890+
SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
3891+
PGC_S_SESSION);
38843892

38853893
/* determine safety of this index for set_indexsafe_procflags */
38863894
idx->safe = (indexRel->rd_indexprs == NIL &&

‎src/backend/commands/matview.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,8 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
173173
SetUserIdAndSecContext(relowner,
174174
save_sec_context | SECURITY_RESTRICTED_OPERATION);
175175
save_nestlevel = NewGUCNestLevel();
176+
SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
177+
PGC_S_SESSION);
176178

177179
/* Make sure it is a materialized view. */
178180
if (matviewRel->rd_rel->relkind != RELKIND_MATVIEW)

‎src/backend/commands/vacuum.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2166,6 +2166,8 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
21662166
SetUserIdAndSecContext(rel->rd_rel->relowner,
21672167
save_sec_context | SECURITY_RESTRICTED_OPERATION);
21682168
save_nestlevel = NewGUCNestLevel();
2169+
SetConfigOption("search_path", GUC_SAFE_SEARCH_PATH, PGC_USERSET,
2170+
PGC_S_SESSION);
21692171

21702172
/*
21712173
* If PROCESS_MAIN is set (the default), it's time to vacuum the main

‎src/bin/scripts/t/100_vacuumdb.pl

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,17 +109,13 @@
109109
CREATE FUNCTION f1(int) RETURNS int LANGUAGE SQL AS 'SELECT f0($1)';
110110
CREATE TABLE funcidx (x int);
111111
INSERT INTO funcidx VALUES (0),(1),(2),(3);
112-
CREATE INDEX i0 ON funcidx ((f1(x)));
113112
CREATE SCHEMA "Foo";
114113
CREATE TABLE "Foo".bar(id int);
115114
CREATE SCHEMA "Bar";
116115
CREATE TABLE "Bar".baz(id int);
117116
|);
118117
$node->command_ok([qw|vacuumdb -Z --table="need""q(uot"(")x") postgres|],
119118
'column list');
120-
$node->command_fails(
121-
[qw|vacuumdb -Zt funcidx postgres|],
122-
'unqualified name via functional index');
123119

124120
$node->command_fails(
125121
[ 'vacuumdb', '--analyze', '--table', 'vactable(c)', 'postgres' ],

0 commit comments

Comments
 (0)