Re: [systemd-devel] [PATCH 2/2] policy: make policy checks work across user namespaces

2014-09-09 Thread Djalal Harouni
On Tue, Sep 09, 2014 at 10:40:57AM +0200, Daniel Mack wrote:
> On 09/08/2014 03:50 PM, Djalal Harouni wrote:
> > Yes there are compile time checks, and it is perhaps easier/consistent
> > to read this way! but yes a union is also good. OK I'll update it.
> 
> Nevermind - I amended the your patch accordingly and pushed it.
Oh sorry, and thank you for fixing that :-)

-- 
Djalal Harouni
http://opendz.org
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH 2/2] policy: make policy checks work across user namespaces

2014-09-09 Thread Daniel Mack
On 09/08/2014 03:50 PM, Djalal Harouni wrote:
> Yes there are compile time checks, and it is perhaps easier/consistent
> to read this way! but yes a union is also good. OK I'll update it.

Nevermind - I amended the your patch accordingly and pushed it.


Thanks!

Daniel

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH 2/2] policy: make policy checks work across user namespaces

2014-09-08 Thread Djalal Harouni
On Mon, Sep 08, 2014 at 03:27:42PM +0200, Daniel Mack wrote:
> On 09/08/2014 03:18 PM, Djalal Harouni wrote:
> >   * This is the internal version of struct kdbus_policy_db_access.
> > @@ -51,7 +52,8 @@ struct kdbus_policy_db_cache_entry {
> >  struct kdbus_policy_db_entry_access {
> > u8 type;/* USER, GROUP, WORLD */
> > u8 access;  /* OWN, TALK, SEE */
> > -   u64 id; /* uid, gid, 0 */
> > +   kuid_t uid; /* global uid */
> > +   kgid_t gid; /* global gid */
> 
> Such an entry can only either be referring to a user or group rule,
> determined by the 'type' field. Hence, having two members in the struct
> is overkill. I understand you did this to have the real kernel types in
> place, but we can put the two things in a union, right?
Yes there are compile time checks, and it is perhaps easier/consistent
to read this way! but yes a union is also good. OK I'll update it.

BTW there is a *small* optimization that we can also add later to the
"case KDBUS_POLICY_ACCESS_GROUP:" when we walk the additional group and
match, instead of linear we can do a binary search since groups are
sorted, there is kernel/groups.c:groups_search() which should do the job
but the symbol is not exported...

Ok will update/test for the union case and send it later, thank you!


-- 
Djalal Harouni
http://opendz.org
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH 2/2] policy: make policy checks work across user namespaces

2014-09-08 Thread Daniel Mack
On 09/08/2014 03:18 PM, Djalal Harouni wrote:
>   * This is the internal version of struct kdbus_policy_db_access.
> @@ -51,7 +52,8 @@ struct kdbus_policy_db_cache_entry {
>  struct kdbus_policy_db_entry_access {
>   u8 type;/* USER, GROUP, WORLD */
>   u8 access;  /* OWN, TALK, SEE */
> - u64 id; /* uid, gid, 0 */
> + kuid_t uid; /* global uid */
> + kgid_t gid; /* global gid */

Such an entry can only either be referring to a user or group rule,
determined by the 'type' field. Hence, having two members in the struct
is overkill. I understand you did this to have the real kernel types in
place, but we can put the two things in a union, right?

The rest looks good!


Thanks,
Daniel

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel