Skip to content
Unverified Commit abfcf55d authored by Christian Brauner's avatar Christian Brauner Committed by Christian Brauner (Microsoft)
Browse files

acl: handle idmapped mounts for idmapped filesystems

Ensure that POSIX ACLs checking, getting, and setting works correctly
for filesystems mountable with a filesystem idmapping ("fs_idmapping")
that want to support idmapped mounts ("mnt_idmapping").

Note that no filesystems mountable with an fs_idmapping do yet support
idmapped mounts. This is required infrastructure work to unblock this.

As we explained in detail in [1] the fs_idmapping is irrelevant for
getxattr() and setxattr() when mapping the ACL_{GROUP,USER} {g,u}ids
stored in the uapi struct posix_acl_xattr_entry in
posix_acl_fix_xattr_{from,to}_user().

But for acl_permission_check() and posix_acl_{g,s}etxattr_idmapped_mnt()
the fs_idmapping matters.

acl_permission_check():
  During lookup POSIX ACLs are retrieved directly via i_op->get_acl() and
  are returned via the kernel internal struct posix_acl which contains
  e_{g,u}id members of type k{g,u}id_t that already take the
  fs_idmapping into acccount.

  For example, a POSIX ACL stored with u4 on the backing store is mapped
  to k10000004 in the fs_idmapping. The mnt_idmapping remaps the POSIX ACL
  to k20000004. In order to do that the fs_idmapping needs to be taken
  into account but that doesn't happen yet (Again, this is a
  counterfactual currently as fuse doesn't support idmapped mounts
  currently. It's just used as a convenient example.):

  fs_idmapping:  u0:k10000000:r65536
  mnt_idmapping: u0:v20000000:r65536
  ACL_USER:      k10000004

  acl_permission_check()
  -> check_acl()
     -> get_acl()
        -> i_op->get_acl() == fuse_get_acl()
           -> posix_acl_from_xattr(u0:k10000000:r65536 /* fs_idmapping */, ...)
              {
                      k10000004 = make_kuid(u0:k10000000:r65536 /* fs_idmapping */,
                                            u4 /* ACL_USER */);
              }
     -> posix_acl_permission()
        {
                -1 = make_vfsuid(u0:v20000000:r65536 /* mnt_idmapping */,
                                 &init_user_ns,
                                 k10000004);
                vfsuid_eq_kuid(-1, k10000004 /* caller_fsuid */)
        }

  In order to correctly map from the fs_idmapping into mnt_idmapping we
  require the relevant fs_idmaping to be passed:

  acl_permission_check()
  -> check_acl()
     -> get_acl()
        -> i_op->get_acl() == fuse_get_acl()
           -> posix_acl_from_xattr(u0:k10000000:r65536 /* fs_idmapping */, ...)
              {
                      k10000004 = make_kuid(u0:k10000000:r65536 /* fs_idmapping */,
                                            u4 /* ACL_USER */);
              }
     -> posix_acl_permission()
        {
                v20000004 = make_vfsuid(u0:v20000000:r65536 /* mnt_idmapping */,
                                        u0:k10000000:r65536 /* fs_idmapping */,
                                        k10000004);
                vfsuid_eq_kuid(v20000004, k10000004 /* caller_fsuid */)
        }

  The initial_idmapping is only correct for the current situation because
  all filesystems that currently support idmapped mounts do not support
  being mounted with an fs_idmapping.

  Note that ovl_get_acl() is used to retrieve the POSIX ACLs from the
  relevant lower layer and the lower layer's mnt_idmapping needs to be
  taken into account and so does the fs_idmapping. See 0c5fd887 ("acl:
  move idmapped mount fixup into vfs_{g,s}etxattr()") for more details.

For posix_acl_{g,s}etxattr_idmapped_mnt() it is not as obvious why the
fs_idmapping matters as it is for acl_permission_check(). Especially
because it doesn't matter for posix_acl_fix_xattr_{from,to}_user() (See
[1] for more context.).

Because posix_acl_{g,s}etxattr_idmapped_mnt() operate on the uapi
struct posix_acl_xattr_entry which contains {g,u}id_t values and thus
give the impression that the fs_idmapping is irrelevant as at this point
appropriate {g,u}id_t values have seemlingly been generated.

As we've stated multiple times this assumption is wrong and in fact the
uapi struct posix_acl_xattr_entry is taking idmappings into account
depending at what place it is operated on.

posix_acl_getxattr_idmapped_mnt()
  When posix_acl_getxattr_idmapped_mnt() is called the values stored in
  the uapi struct posix_acl_xattr_entry are mapped according to the
  fs_idmapping. This happened when they were read from the backing store
  and then translated from struct posix_acl into the uapi
  struct posix_acl_xattr_entry during posix_acl_to_xattr().

  In other words, the fs_idmapping matters as the values stored as
  {g,u}id_t in the uapi struct posix_acl_xattr_entry have been generated
  by it.

  So we need to take the fs_idmapping into account during make_vfsuid()
  in posix_acl_getxattr_idmapped_mnt().

posix_acl_setxattr_idmapped_mnt()
  When posix_acl_setxattr_idmapped_mnt() is called the values stored as
  {g,u}id_t in uapi struct posix_acl_xattr_entry are intended to be the
  values that ultimately get turned back into a k{g,u}id_t in
  posix_acl_from_xattr() (which turns the uapi
  struct posix_acl_xattr_entry into the kernel internal struct posix_acl).

  In other words, the fs_idmapping matters as the values stored as
  {g,u}id_t in the uapi struct posix_acl_xattr_entry are intended to be
  the values that will be undone in the fs_idmapping when writing to the
  backing store.

  So we need to take the fs_idmapping into account during from_vfsuid()
  in posix_acl_setxattr_idmapped_mnt().

Link: https://lore.kernel.org/all/20220801145520.1532837-1-brauner@kernel.org

 [1]
Fixes: 0c5fd887 ("acl: move idmapped mount fixup into vfs_{g,s}etxattr()")
Cc: Seth Forshee <sforshee@digitalocean.com>
Signed-off-by: default avatarChristian Brauner (Microsoft) <brauner@kernel.org>
Reviewed-by: default avatarSeth Forshee <sforshee@digitalocean.com>
Link: https://lore.kernel.org/r/20220816113514.43304-1-brauner@kernel.org
parent 568035b0
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment