* Dean Rasheed (dean.a.rash...@gmail.com) wrote: > On 27 August 2015 at 13:49, Andres Freund <and...@anarazel.de> wrote: > > The locking around rowsecurity policy expressions seems to be > > insufficient: > > SELECT * FROM document WHERE f_leak(dtitle) ORDER BY did; > > WARNING: RelationIdGetRelation(247984) without holding lock on the relation > > WARNING: relation_open(247984, NoLock) of relation "uaccount" without > > previously held lock [...] > > Istmt that something like > > context.for_execute = true; > > acquireLocksOnSubLinks((Node *) > > securityQuals, &context); > > acquireLocksOnSubLinks((Node *) > > withCheckOptions, &context); > > needs to be added to that code. > > Yes, I think you're right. It needs to happen before fireRIRonSubLink, > and only if hasSubLinks is true.
Attached appears to fix this for the RLS case from my testing. Any comments? Barring concerns, I'll push this later today and back-patch to 9.5. Thanks! Stephen
From 7ac58a62338103338b6907fc7ea89f9afb9a0e53 Mon Sep 17 00:00:00 2001 From: Stephen Frost <sfr...@snowman.net> Date: Fri, 28 Aug 2015 08:10:22 -0400 Subject: [PATCH] Ensure locks are acquired on RLS-added relations During fireRIRrules(), get_row_security_policies can add to securityQuals and withCheckOptions. Make sure to lock any relations added at that point and before firing RIR rules on those expressions. Back-patch to 9.5 where RLS was added. --- src/backend/rewrite/rewriteHandler.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index 1734e48..fbc0c57 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -1787,6 +1787,8 @@ fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown) { if (hasSubLinks) { + acquireLocksOnSubLinks_context context; + /* * Recursively process the new quals, checking for infinite * recursion. @@ -1799,6 +1801,20 @@ fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown) activeRIRs = lcons_oid(RelationGetRelid(rel), activeRIRs); + /* + * get_row_security_policies just added to securityQuals and/or + * withCheckOptions, and there were SubLinks, so make sure + * we lock any relations which were added as a result. + */ + context.for_execute = true; + (void) acquireLocksOnSubLinks((Node *) securityQuals, &context); + (void) acquireLocksOnSubLinks((Node *) withCheckOptions, + &context); + + /* + * Now that we have the locks on anything added by + * get_row_security_policies, fire any RIR rules for them. + */ expression_tree_walker((Node *) securityQuals, fireRIRonSubLink, (void *) activeRIRs); -- 1.9.1
signature.asc
Description: Digital signature