• Christian Brauner's avatar
    acl: handle idmapped mounts for idmapped filesystems · abfcf55d
    Christian Brauner authored
    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
    abfcf55d
inode.c 34.3 KB