[systemd-devel] [PATCH 07/12] policy: use the db->entries_hash to access the policy db entries

[Djalal Harouni]

Hi,

On Fri, Jun 20, 2014 at 07:12:13PM +0100, Djalal Harouni wrote:
> On Fri, Jun 20, 2014 at 08:01:04PM +0200, Daniel Mack wrote:
> > On 06/20/2014 07:28 PM, Daniel Mack wrote:
> > > On 06/20/2014 06:50 PM, Djalal Harouni wrote:
> > >> Use the db->entries_hash to access the policy db entries instead of the
> > >> db->send_access_hash which is just a cache for send entries.
> > 
> > Ah, you're purging the other entries in patch #12. Alright then, now it
> > makes sense.
> Indeed, I've tested it and the cache is cleared.
> 
> > Applied both #7 and #9 now.
> Thank you Daniel, There is a still another series related to the cache I
> just need to test it.
> 
> When we update the TALK POLICY of a connection that is already
> referenced as a *destination* in the cache we must purge all its entries
> from the cache, since permission have been changed! we need to redo the
> permission checks.
> 
> This should be done in kdbus_policy_set() when we update the TALK policy
> but since kdbus_policy_set() can be called by an endpoint as an owner,
> not only a connection, I did split the code in a new function...
> 
> Anyway I'll test it send it tomorrow.
Just an update:

I'm still working on the TALK cache issue, I've added the infrastracture
to clear the TALK cache (db->send_access_hash) when we update the TALK
policy of the destination connection, yes we need to clear the
previously cached entries where it is listed as destination since its
TALK permission have been changed (issue #1). However we don't want to
clear *all* the cache, only entries that apply to the newly updated
policies!

Since policy holders connections might have several policies and multiple
names, the best solution would be naturaly to clear only cached entries
related to the updated policy and name! and keep other cached entries
related to other policies and names... while working on it, I hit
another two bugs:


Issue #2: currently it seems we can't have a policy holder connection
with multiple policies and names!

This check:
https://code.google.com/p/d-bus/source/browse/connection.c#1908

Will prevent policy holders to register multiple names and policies,
it should prevent activators, since we have a one name per activator!
but not policy holders.

So I've removed this check, I've added the necessary test infrastructure
to be able to register multiple policies and names, but I'm stuck in
which I think a kdbus item alignment problem! I'll try to fix and start
with this series!


Issue #3 (not confirmed) related to issue #2 and issue #1 of course is
that in function:
__kdbus_policy_check_talk_access()
https://code.google.com/p/d-bus/source/browse/policy.c#254

That function do not receive the name of the policy to check TALK access
against! the other OWN and SEE check access functions do, but not the
'TALK' one.

The current behaviour will walk the 'conn_dst->names_list' and perform
the TALK access against all names, if one name allows the TALK then we
have access! but in that case perhaps the source connection was aiming
to send to a different name which restrcit access, not this one!

So IMHO that function and all its callers should be updated to receive
the name of the destination as a third parameter, and the 'name' should
also be saved in the TALK cache, this way we are sure that the cached
entries belong to the name being checked! So we have two issues here.

So what do you think Daniel, Kay ?

(Hope I'm not getting it wrong)

I'll try to have tests for issue #2 ASAP, and will probably handle
issue #1 since I've already added the infrastructure, and if you confirm
issue #3 I'll perhaps also work on it later too (if you don't beat me
:-)

Thank you!


-- 
Djalal Harouni
http://opendz.org