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

Commite55f718

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 parent079d0ca commite55f718

File tree

1 file changed

+98
-118
lines changed

1 file changed

+98
-118
lines changed

‎src/backend/commands/policy.c

Lines changed: 98 additions & 118 deletions
Original file line numberDiff line numberDiff line change
@@ -187,159 +187,139 @@ policy_role_list_to_array(List *roles, int *num_roles)
187187
/*
188188
* Load row security policy from the catalog, and store it in
189189
* the relation's relcache entry.
190+
*
191+
* Note that caller should have verified that pg_class.relrowsecurity
192+
* is true for this relation.
190193
*/
191194
void
192195
RelationBuildRowSecurity(Relationrelation)
193196
{
194197
MemoryContextrscxt;
195198
MemoryContextoldcxt=CurrentMemoryContext;
196-
RowSecurityDesc*volatilersdesc=NULL;
199+
RowSecurityDesc*rsdesc;
200+
Relationcatalog;
201+
ScanKeyDataskey;
202+
SysScanDescsscan;
203+
HeapTupletuple;
197204

198205
/*
199206
* Create a memory context to hold everything associated with this
200207
* relation's row security policy. This makes it easy to clean up during
201-
* a relcache flush.
208+
* a relcache flush. However, to cover the possibility of an error
209+
* partway through, we don't make the context long-lived till we're done.
202210
*/
203-
rscxt=AllocSetContextCreate(CacheMemoryContext,
211+
rscxt=AllocSetContextCreate(CurrentMemoryContext,
204212
"row security descriptor",
205213
ALLOCSET_SMALL_SIZES);
214+
MemoryContextCopyAndSetIdentifier(rscxt,
215+
RelationGetRelationName(relation));
216+
217+
rsdesc=MemoryContextAllocZero(rscxt,sizeof(RowSecurityDesc));
218+
rsdesc->rscxt=rscxt;
206219

207220
/*
208-
* Since rscxt lives under CacheMemoryContext, it is long-lived. Use a
209-
* PG_TRY block to ensure it'll get freed if we fail partway through.
221+
* Now scan pg_policy for RLS policies associated with this relation.
222+
* Because we use the index on (polrelid, polname), we should consistently
223+
* visit the rel's policies in name order, at least when system indexes
224+
* aren't disabled. This simplifies equalRSDesc().
210225
*/
211-
PG_TRY();
212-
{
213-
Relationcatalog;
214-
ScanKeyDataskey;
215-
SysScanDescsscan;
216-
HeapTupletuple;
217-
218-
MemoryContextCopyAndSetIdentifier(rscxt,
219-
RelationGetRelationName(relation));
226+
catalog=table_open(PolicyRelationId,AccessShareLock);
220227

221-
rsdesc=MemoryContextAllocZero(rscxt,sizeof(RowSecurityDesc));
222-
rsdesc->rscxt=rscxt;
228+
ScanKeyInit(&skey,
229+
Anum_pg_policy_polrelid,
230+
BTEqualStrategyNumber,F_OIDEQ,
231+
ObjectIdGetDatum(RelationGetRelid(relation)));
223232

224-
catalog=table_open(PolicyRelationId,AccessShareLock);
233+
sscan=systable_beginscan(catalog,PolicyPolrelidPolnameIndexId, true,
234+
NULL,1,&skey);
225235

226-
ScanKeyInit(&skey,
227-
Anum_pg_policy_polrelid,
228-
BTEqualStrategyNumber,F_OIDEQ,
229-
ObjectIdGetDatum(RelationGetRelid(relation)));
236+
while (HeapTupleIsValid(tuple=systable_getnext(sscan)))
237+
{
238+
Form_pg_policypolicy_form= (Form_pg_policy)GETSTRUCT(tuple);
239+
RowSecurityPolicy*policy;
240+
Datumdatum;
241+
boolisnull;
242+
char*str_value;
230243

231-
sscan=systable_beginscan(catalog,PolicyPolrelidPolnameIndexId, true,
232-
NULL,1,&skey);
244+
policy=MemoryContextAllocZero(rscxt,sizeof(RowSecurityPolicy));
233245

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

295-
/* Get WITH CHECK qual */
296-
value_datum=heap_getattr(tuple,Anum_pg_policy_polwithcheck,
297-
RelationGetDescr(catalog),&isnull);
298-
if (!isnull)
299-
{
300-
with_check_value=TextDatumGetCString(value_datum);
301-
with_check_qual= (Expr*)stringToNode(with_check_value);
302-
}
303-
else
304-
with_check_qual=NULL;
252+
/* Get policy command */
253+
policy->polcmd=policy_form->polcmd;
305254

306-
/*Now copy everything into the cache context */
307-
MemoryContextSwitchTo(rscxt);
255+
/*Get policy, permissive or restrictive */
256+
policy->permissive=policy_form->polpermissive;
308257

309-
policy=palloc0(sizeof(RowSecurityPolicy));
310-
policy->policy_name=pstrdup(policy_name_value);
311-
policy->polcmd=cmd_value;
312-
policy->permissive=permissive_value;
313-
policy->roles=DatumGetArrayTypePCopy(roles_datum);
314-
policy->qual=copyObject(qual_expr);
315-
policy->with_check_qual=copyObject(with_check_qual);
316-
policy->hassublinks=checkExprHasSubLink((Node*)qual_expr)||
317-
checkExprHasSubLink((Node*)with_check_qual);
258+
/* Get policy name */
259+
policy->policy_name=
260+
MemoryContextStrdup(rscxt,NameStr(policy_form->polname));
318261

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

272+
/* Get policy qual */
273+
datum=heap_getattr(tuple,Anum_pg_policy_polqual,
274+
RelationGetDescr(catalog),&isnull);
275+
if (!isnull)
276+
{
277+
str_value=TextDatumGetCString(datum);
278+
MemoryContextSwitchTo(rscxt);
279+
policy->qual= (Expr*)stringToNode(str_value);
321280
MemoryContextSwitchTo(oldcxt);
281+
pfree(str_value);
282+
}
283+
else
284+
policy->qual=NULL;
322285

323-
/* clean up some (not all) of the junk ... */
324-
if (qual_expr!=NULL)
325-
pfree(qual_expr);
326-
if (with_check_qual!=NULL)
327-
pfree(with_check_qual);
286+
/* Get WITH CHECK qual */
287+
datum=heap_getattr(tuple,Anum_pg_policy_polwithcheck,
288+
RelationGetDescr(catalog),&isnull);
289+
if (!isnull)
290+
{
291+
str_value=TextDatumGetCString(datum);
292+
MemoryContextSwitchTo(rscxt);
293+
policy->with_check_qual= (Expr*)stringToNode(str_value);
294+
MemoryContextSwitchTo(oldcxt);
295+
pfree(str_value);
328296
}
297+
else
298+
policy->with_check_qual=NULL;
329299

330-
systable_endscan(sscan);
331-
table_close(catalog,AccessShareLock);
332-
}
333-
PG_CATCH();
334-
{
335-
/* Delete rscxt, first making sure it isn't active */
300+
/* We want to cache whether there are SubLinks in these expressions */
301+
policy->hassublinks=checkExprHasSubLink((Node*)policy->qual)||
302+
checkExprHasSubLink((Node*)policy->with_check_qual);
303+
304+
/*
305+
* Add this object to list. For historical reasons, the list is built
306+
* in reverse order.
307+
*/
308+
MemoryContextSwitchTo(rscxt);
309+
rsdesc->policies=lcons(policy,rsdesc->policies);
336310
MemoryContextSwitchTo(oldcxt);
337-
MemoryContextDelete(rscxt);
338-
PG_RE_THROW();
339311
}
340-
PG_END_TRY();
341312

342-
/* Success --- attach the policy descriptor to the relcache entry */
313+
systable_endscan(sscan);
314+
table_close(catalog,AccessShareLock);
315+
316+
/*
317+
* Success. Reparent the descriptor's memory context under
318+
* CacheMemoryContext so that it will live indefinitely, then attach the
319+
* policy descriptor to the relcache entry.
320+
*/
321+
MemoryContextSetParent(rscxt,CacheMemoryContext);
322+
343323
relation->rd_rsdesc=rsdesc;
344324
}
345325

0 commit comments

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