AcquireRewriteLocks/acquireLocksOnSubLinks vs. rowsecurity

From: Andres Freund <andres(at)anarazel(dot)de>
To: pgsql-hackers(at)postgresql(dot)org, Stephen Frost <sfrost(at)snowman(dot)net>
Subject: AcquireRewriteLocks/acquireLocksOnSubLinks vs. rowsecurity
Date: 2015-08-27 12:49:31
Message-ID: 20150827124931.GD15922@awork2.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

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

I don't know the relevant code well. But as far as I can see that's
because normally the expectation is that relevant locks have either been
taken by the parser or by AcquireRewriteLocks(). But before

static Query *
fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown)
{
...
/*
* Fetch any new security quals that must be applied to this RTE.
*/
get_row_security_policies(parsetree, parsetree->commandType, rte,
rt_index, &securityQuals, &withCheckOptions,
&hasRowSecurity, &hasSubLinks);

if (securityQuals != NIL || withCheckOptions != NIL)
{
...
if (hasSubLinks)
{
...
expression_tree_walker((Node *) securityQuals,
fireRIRonSubLink, (void *) activeRIRs);
...
}

rte->securityQuals = list_concat(securityQuals,
rte->securityQuals);

neither will have acquired relevant locks. The parser because it doesn't
know about rowsecurity, AcquireRewriteLocks/acquireLocksOnSubLinks
because rte->securityQuals wan't even set and range_table_walker() uses
that.

Istmt that something like
context.for_execute = true;
acquireLocksOnSubLinks((Node *) securityQuals, &context);
acquireLocksOnSubLinks((Node *) withCheckOptions, &context);
needs to be added to that code.

Greetings,

Andres Freund

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2015-08-27 13:13:52 What does RIR as in fireRIRrules stand for?
Previous Message Andres Freund 2015-08-27 12:41:06 Re: checkpointer continuous flushing