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

Commitde6725d

Browse files
committed
Revise RelationBuildRowSecurity() to avoid memory leaks.
This function leaked some memory while loading qual clauses foran RLS policy. While ordinarily negligible, that could build upin some repeated-reload cases, as reported by Konstantin Knizhnik.We can improve matters by borrowing the coding long used inRelationBuildRuleLock: build stringToNode's result directly inthe target context, and remember to explicitly pfree theinput string.This patch by no means completely guarantees zero leaks withinthis function, since we have no real guarantee that the catalog-reading subroutines it calls don't leak anything. However,practical tests suggest that this is enough to resolve the issue.In any case, any remaining leaks are similar to those risked byRelationBuildRuleLock and other relcache-loading subroutines.If we need to fix them, we should adopt a more global approachsuch as that used by the RECOVER_RELATION_BUILD_MEMORY hack.While here, let's remove the need for an expensive PG_TRY block byusing MemoryContextSetParent to reparent an initially-short-livedcontext for the RLS data.Back-patch to all supported branches.Discussion:https://postgr.es/m/21356c12-8917-8249-b35f-1c447231922b@postgrespro.ru
1 parent1888ff8 commitde6725d

File tree

1 file changed

+96
-115
lines changed

1 file changed

+96
-115
lines changed

‎src/backend/commands/policy.c‎

Lines changed: 96 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -186,156 +186,137 @@ policy_role_list_to_array(List *roles, int *num_roles)
186186
/*
187187
* Load row security policy from the catalog, and store it in
188188
* the relation's relcache entry.
189+
*
190+
* Note that caller should have verified that pg_class.relrowsecurity
191+
* is true for this relation.
189192
*/
190193
void
191194
RelationBuildRowSecurity(Relationrelation)
192195
{
193196
MemoryContextrscxt;
194197
MemoryContextoldcxt=CurrentMemoryContext;
195-
RowSecurityDesc*volatilersdesc=NULL;
198+
RowSecurityDesc*rsdesc;
199+
Relationcatalog;
200+
ScanKeyDataskey;
201+
SysScanDescsscan;
202+
HeapTupletuple;
196203

197204
/*
198205
* Create a memory context to hold everything associated with this
199206
* relation's row security policy. This makes it easy to clean up during
200-
* a relcache flush.
207+
* a relcache flush. However, to cover the possibility of an error
208+
* partway through, we don't make the context long-lived till we're done.
201209
*/
202-
rscxt=AllocSetContextCreate(CacheMemoryContext,
210+
rscxt=AllocSetContextCreate(CurrentMemoryContext,
203211
"row security descriptor",
204212
ALLOCSET_SMALL_SIZES);
205213

214+
rsdesc=MemoryContextAllocZero(rscxt,sizeof(RowSecurityDesc));
215+
rsdesc->rscxt=rscxt;
216+
206217
/*
207-
* Since rscxt lives under CacheMemoryContext, it is long-lived. Use a
208-
* PG_TRY block to ensure it'll get freed if we fail partway through.
218+
* Now scan pg_policy for RLS policies associated with this relation.
219+
* Because we use the index on (polrelid, polname), we should consistently
220+
* visit the rel's policies in name order, at least when system indexes
221+
* aren't disabled. This simplifies equalRSDesc().
209222
*/
210-
PG_TRY();
211-
{
212-
Relationcatalog;
213-
ScanKeyDataskey;
214-
SysScanDescsscan;
215-
HeapTupletuple;
223+
catalog=heap_open(PolicyRelationId,AccessShareLock);
216224

217-
rsdesc=MemoryContextAllocZero(rscxt,sizeof(RowSecurityDesc));
218-
rsdesc->rscxt=rscxt;
225+
ScanKeyInit(&skey,
226+
Anum_pg_policy_polrelid,
227+
BTEqualStrategyNumber,F_OIDEQ,
228+
ObjectIdGetDatum(RelationGetRelid(relation)));
219229

220-
catalog=heap_open(PolicyRelationId,AccessShareLock);
230+
sscan=systable_beginscan(catalog,PolicyPolrelidPolnameIndexId, true,
231+
NULL,1,&skey);
221232

222-
ScanKeyInit(&skey,
223-
Anum_pg_policy_polrelid,
224-
BTEqualStrategyNumber,F_OIDEQ,
225-
ObjectIdGetDatum(RelationGetRelid(relation)));
233+
while (HeapTupleIsValid(tuple=systable_getnext(sscan)))
234+
{
235+
Form_pg_policypolicy_form= (Form_pg_policy)GETSTRUCT(tuple);
236+
RowSecurityPolicy*policy;
237+
Datumdatum;
238+
boolisnull;
239+
char*str_value;
226240

227-
sscan=systable_beginscan(catalog,PolicyPolrelidPolnameIndexId, true,
228-
NULL,1,&skey);
241+
policy=MemoryContextAllocZero(rscxt,sizeof(RowSecurityPolicy));
229242

230243
/*
231-
* Loop through the row level security policies for this relation, if
232-
* any.
244+
* Note: we must be sure that pass-by-reference data gets copied into
245+
* rscxt. We avoid making that context current over wider spans than
246+
* we have to, though.
233247
*/
234-
while (HeapTupleIsValid(tuple=systable_getnext(sscan)))
235-
{
236-
Datumvalue_datum;
237-
charcmd_value;
238-
boolpermissive_value;
239-
Datumroles_datum;
240-
char*qual_value;
241-
Expr*qual_expr;
242-
char*with_check_value;
243-
Expr*with_check_qual;
244-
char*policy_name_value;
245-
boolisnull;
246-
RowSecurityPolicy*policy;
247-
248-
/*
249-
* Note: all the pass-by-reference data we collect here is either
250-
* still stored in the tuple, or constructed in the caller's
251-
* short-lived memory context. We must copy it into rscxt
252-
* explicitly below.
253-
*/
254-
255-
/* Get policy command */
256-
value_datum=heap_getattr(tuple,Anum_pg_policy_polcmd,
257-
RelationGetDescr(catalog),&isnull);
258-
Assert(!isnull);
259-
cmd_value=DatumGetChar(value_datum);
260-
261-
/* Get policy permissive or restrictive */
262-
value_datum=heap_getattr(tuple,Anum_pg_policy_polpermissive,
263-
RelationGetDescr(catalog),&isnull);
264-
Assert(!isnull);
265-
permissive_value=DatumGetBool(value_datum);
266-
267-
/* Get policy name */
268-
value_datum=heap_getattr(tuple,Anum_pg_policy_polname,
269-
RelationGetDescr(catalog),&isnull);
270-
Assert(!isnull);
271-
policy_name_value=NameStr(*(DatumGetName(value_datum)));
272-
273-
/* Get policy roles */
274-
roles_datum=heap_getattr(tuple,Anum_pg_policy_polroles,
275-
RelationGetDescr(catalog),&isnull);
276-
/* shouldn't be null, but initdb doesn't mark it so, so check */
277-
if (isnull)
278-
elog(ERROR,"unexpected null value in pg_policy.polroles");
279-
280-
/* Get policy qual */
281-
value_datum=heap_getattr(tuple,Anum_pg_policy_polqual,
282-
RelationGetDescr(catalog),&isnull);
283-
if (!isnull)
284-
{
285-
qual_value=TextDatumGetCString(value_datum);
286-
qual_expr= (Expr*)stringToNode(qual_value);
287-
}
288-
else
289-
qual_expr=NULL;
290248

291-
/* Get WITH CHECK qual */
292-
value_datum=heap_getattr(tuple,Anum_pg_policy_polwithcheck,
293-
RelationGetDescr(catalog),&isnull);
294-
if (!isnull)
295-
{
296-
with_check_value=TextDatumGetCString(value_datum);
297-
with_check_qual= (Expr*)stringToNode(with_check_value);
298-
}
299-
else
300-
with_check_qual=NULL;
249+
/* Get policy command */
250+
policy->polcmd=policy_form->polcmd;
301251

302-
/*Now copy everything into the cache context */
303-
MemoryContextSwitchTo(rscxt);
252+
/*Get policy, permissive or restrictive */
253+
policy->permissive=policy_form->polpermissive;
304254

305-
policy=palloc0(sizeof(RowSecurityPolicy));
306-
policy->policy_name=pstrdup(policy_name_value);
307-
policy->polcmd=cmd_value;
308-
policy->permissive=permissive_value;
309-
policy->roles=DatumGetArrayTypePCopy(roles_datum);
310-
policy->qual=copyObject(qual_expr);
311-
policy->with_check_qual=copyObject(with_check_qual);
312-
policy->hassublinks=checkExprHasSubLink((Node*)qual_expr)||
313-
checkExprHasSubLink((Node*)with_check_qual);
255+
/* Get policy name */
256+
policy->policy_name=
257+
MemoryContextStrdup(rscxt,NameStr(policy_form->polname));
314258

315-
rsdesc->policies=lcons(policy,rsdesc->policies);
259+
/* Get policy roles */
260+
datum=heap_getattr(tuple,Anum_pg_policy_polroles,
261+
RelationGetDescr(catalog),&isnull);
262+
/* shouldn't be null, but let's check for luck */
263+
if (isnull)
264+
elog(ERROR,"unexpected null value in pg_policy.polroles");
265+
MemoryContextSwitchTo(rscxt);
266+
policy->roles=DatumGetArrayTypePCopy(datum);
267+
MemoryContextSwitchTo(oldcxt);
316268

269+
/* Get policy qual */
270+
datum=heap_getattr(tuple,Anum_pg_policy_polqual,
271+
RelationGetDescr(catalog),&isnull);
272+
if (!isnull)
273+
{
274+
str_value=TextDatumGetCString(datum);
275+
MemoryContextSwitchTo(rscxt);
276+
policy->qual= (Expr*)stringToNode(str_value);
317277
MemoryContextSwitchTo(oldcxt);
278+
pfree(str_value);
279+
}
280+
else
281+
policy->qual=NULL;
318282

319-
/* clean up some (not all) of the junk ... */
320-
if (qual_expr!=NULL)
321-
pfree(qual_expr);
322-
if (with_check_qual!=NULL)
323-
pfree(with_check_qual);
283+
/* Get WITH CHECK qual */
284+
datum=heap_getattr(tuple,Anum_pg_policy_polwithcheck,
285+
RelationGetDescr(catalog),&isnull);
286+
if (!isnull)
287+
{
288+
str_value=TextDatumGetCString(datum);
289+
MemoryContextSwitchTo(rscxt);
290+
policy->with_check_qual= (Expr*)stringToNode(str_value);
291+
MemoryContextSwitchTo(oldcxt);
292+
pfree(str_value);
324293
}
294+
else
295+
policy->with_check_qual=NULL;
325296

326-
systable_endscan(sscan);
327-
heap_close(catalog,AccessShareLock);
328-
}
329-
PG_CATCH();
330-
{
331-
/* Delete rscxt, first making sure it isn't active */
297+
/* We want to cache whether there are SubLinks in these expressions */
298+
policy->hassublinks=checkExprHasSubLink((Node*)policy->qual)||
299+
checkExprHasSubLink((Node*)policy->with_check_qual);
300+
301+
/*
302+
* Add this object to list. For historical reasons, the list is built
303+
* in reverse order.
304+
*/
305+
MemoryContextSwitchTo(rscxt);
306+
rsdesc->policies=lcons(policy,rsdesc->policies);
332307
MemoryContextSwitchTo(oldcxt);
333-
MemoryContextDelete(rscxt);
334-
PG_RE_THROW();
335308
}
336-
PG_END_TRY();
337309

338-
/* Success --- attach the policy descriptor to the relcache entry */
310+
systable_endscan(sscan);
311+
heap_close(catalog,AccessShareLock);
312+
313+
/*
314+
* Success. Reparent the descriptor's memory context under
315+
* CacheMemoryContext so that it will live indefinitely, then attach the
316+
* policy descriptor to the relcache entry.
317+
*/
318+
MemoryContextSetParent(rscxt,CacheMemoryContext);
319+
339320
relation->rd_rsdesc=rsdesc;
340321
}
341322

0 commit comments

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