Re: [PATCH 3/4] xfs: SGI ACLs: Map uid/gid namespaces

[Dave Chinner <david@fromorbit.com>]

On Tue, Oct 27, 2015 at 04:55:22PM +0100, Andreas Gruenbacher wrote:
> On Mon, Oct 26, 2015 at 10:46 PM, Dave Chinner <david@fromorbit.com> wrote:
> > On Sat, Oct 24, 2015 at 11:16:08PM +0200, Andreas Gruenbacher wrote:
> >> @@ -71,10 +72,10 @@ xfs_acl_from_disk(
> >>
> >>               switch (acl_e->e_tag) {
> >>               case ACL_USER:
> >> -                     acl_e->e_uid = xfs_uid_to_kuid(be32_to_cpu(ace->ae_id));
> >> +                     acl_e->e_uid = make_kuid(ns, be32_to_cpu(ace->ae_id));
> >
> > Please don't replace the xfs wrappers with the horribly named
> > generic functions. Pass the namespace to xfs_uid_to_kuid(), and
> > modify them, please. That way people who don't deal with namespaces
> > every day can tell exactly what format conversion is taking place
> > just by reading the code...
> 
> We would effectively end up with:
> 
>   #define xfs_kuid_to_uid from_kuid
>   #define xfs_kgid_to_gid from_kgid
>   #define xfs_uid_to_kuid make_kuid
>   #define xfs_gid_to_kgid make_kgid

No, you'd just add the namespace pointer to the static inline
functions we already have, and add a comment stating when the caller
should pass the init_ns vs user_ns.

But now that I've thought about this a bit more, I don't think that
the changes you've made are correct - we shouldn't be doing uid/gid
namespace conversion in disk formating functions. That is, the
conversion of user/group types should be done at the incoming layers
(i.e. at VFS/ioctl layers), not deep in the guts of the XFS code.

i.e. the disk formating functions should be passed uids/gids that
are already mapped to the init namespace, because there is no
guarantee that the formatting occurs in the same user context as the
syscall (e.g. low level disk formatting work gets deferred to a work
queue).

> Are you sure you really want that?

Why do you think we added the wrappers in the first place? It was
because the ns maintainer refused to follow standard, self
documenting type conversion naming conventions of <type>_to_<type>
so we added them ourselves. The user namespace code is horrible
enough without adding confusing type change functions everywhere...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs