Skip to content

Commit abfcf55

Browse files
committed
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 0c5fd88 ("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/[email protected] [1] Fixes: 0c5fd88 ("acl: move idmapped mount fixup into vfs_{g,s}etxattr()") Cc: Seth Forshee <[email protected]> Signed-off-by: Christian Brauner (Microsoft) <[email protected]> Reviewed-by: Seth Forshee <[email protected]> Link: https://lore.kernel.org/r/[email protected]
1 parent 568035b commit abfcf55

File tree

2 files changed

+16
-10
lines changed

2 files changed

+16
-10
lines changed

fs/overlayfs/inode.c

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -460,21 +460,24 @@ ssize_t ovl_listxattr(struct dentry *dentry, char *list, size_t size)
460460
* of the POSIX ACLs retrieved from the lower layer to this function to not
461461
* alter the POSIX ACLs for the underlying filesystem.
462462
*/
463-
static void ovl_idmap_posix_acl(struct user_namespace *mnt_userns,
463+
static void ovl_idmap_posix_acl(struct inode *realinode,
464+
struct user_namespace *mnt_userns,
464465
struct posix_acl *acl)
465466
{
467+
struct user_namespace *fs_userns = i_user_ns(realinode);
468+
466469
for (unsigned int i = 0; i < acl->a_count; i++) {
467470
vfsuid_t vfsuid;
468471
vfsgid_t vfsgid;
469472

470473
struct posix_acl_entry *e = &acl->a_entries[i];
471474
switch (e->e_tag) {
472475
case ACL_USER:
473-
vfsuid = make_vfsuid(mnt_userns, &init_user_ns, e->e_uid);
476+
vfsuid = make_vfsuid(mnt_userns, fs_userns, e->e_uid);
474477
e->e_uid = vfsuid_into_kuid(vfsuid);
475478
break;
476479
case ACL_GROUP:
477-
vfsgid = make_vfsgid(mnt_userns, &init_user_ns, e->e_gid);
480+
vfsgid = make_vfsgid(mnt_userns, fs_userns, e->e_gid);
478481
e->e_gid = vfsgid_into_kgid(vfsgid);
479482
break;
480483
}
@@ -536,7 +539,7 @@ struct posix_acl *ovl_get_acl(struct inode *inode, int type, bool rcu)
536539
if (!clone)
537540
clone = ERR_PTR(-ENOMEM);
538541
else
539-
ovl_idmap_posix_acl(mnt_user_ns(realpath.mnt), clone);
542+
ovl_idmap_posix_acl(realinode, mnt_user_ns(realpath.mnt), clone);
540543
/*
541544
* Since we're not in RCU path walk we always need to release the
542545
* original ACLs.

fs/posix_acl.c

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,7 @@ posix_acl_permission(struct user_namespace *mnt_userns, struct inode *inode,
361361
const struct posix_acl *acl, int want)
362362
{
363363
const struct posix_acl_entry *pa, *pe, *mask_obj;
364+
struct user_namespace *fs_userns = i_user_ns(inode);
364365
int found = 0;
365366
vfsuid_t vfsuid;
366367
vfsgid_t vfsgid;
@@ -376,7 +377,7 @@ posix_acl_permission(struct user_namespace *mnt_userns, struct inode *inode,
376377
goto check_perm;
377378
break;
378379
case ACL_USER:
379-
vfsuid = make_vfsuid(mnt_userns, &init_user_ns,
380+
vfsuid = make_vfsuid(mnt_userns, fs_userns,
380381
pa->e_uid);
381382
if (vfsuid_eq_kuid(vfsuid, current_fsuid()))
382383
goto mask;
@@ -390,7 +391,7 @@ posix_acl_permission(struct user_namespace *mnt_userns, struct inode *inode,
390391
}
391392
break;
392393
case ACL_GROUP:
393-
vfsgid = make_vfsgid(mnt_userns, &init_user_ns,
394+
vfsgid = make_vfsgid(mnt_userns, fs_userns,
394395
pa->e_gid);
395396
if (vfsgid_in_group_p(vfsgid)) {
396397
found = 1;
@@ -736,6 +737,7 @@ void posix_acl_getxattr_idmapped_mnt(struct user_namespace *mnt_userns,
736737
{
737738
struct posix_acl_xattr_header *header = value;
738739
struct posix_acl_xattr_entry *entry = (void *)(header + 1), *end;
740+
struct user_namespace *fs_userns = i_user_ns(inode);
739741
int count;
740742
vfsuid_t vfsuid;
741743
vfsgid_t vfsgid;
@@ -753,13 +755,13 @@ void posix_acl_getxattr_idmapped_mnt(struct user_namespace *mnt_userns,
753755
switch (le16_to_cpu(entry->e_tag)) {
754756
case ACL_USER:
755757
uid = make_kuid(&init_user_ns, le32_to_cpu(entry->e_id));
756-
vfsuid = make_vfsuid(mnt_userns, &init_user_ns, uid);
758+
vfsuid = make_vfsuid(mnt_userns, fs_userns, uid);
757759
entry->e_id = cpu_to_le32(from_kuid(&init_user_ns,
758760
vfsuid_into_kuid(vfsuid)));
759761
break;
760762
case ACL_GROUP:
761763
gid = make_kgid(&init_user_ns, le32_to_cpu(entry->e_id));
762-
vfsgid = make_vfsgid(mnt_userns, &init_user_ns, gid);
764+
vfsgid = make_vfsgid(mnt_userns, fs_userns, gid);
763765
entry->e_id = cpu_to_le32(from_kgid(&init_user_ns,
764766
vfsgid_into_kgid(vfsgid)));
765767
break;
@@ -775,6 +777,7 @@ void posix_acl_setxattr_idmapped_mnt(struct user_namespace *mnt_userns,
775777
{
776778
struct posix_acl_xattr_header *header = value;
777779
struct posix_acl_xattr_entry *entry = (void *)(header + 1), *end;
780+
struct user_namespace *fs_userns = i_user_ns(inode);
778781
int count;
779782
vfsuid_t vfsuid;
780783
vfsgid_t vfsgid;
@@ -793,13 +796,13 @@ void posix_acl_setxattr_idmapped_mnt(struct user_namespace *mnt_userns,
793796
case ACL_USER:
794797
uid = make_kuid(&init_user_ns, le32_to_cpu(entry->e_id));
795798
vfsuid = VFSUIDT_INIT(uid);
796-
uid = from_vfsuid(mnt_userns, &init_user_ns, vfsuid);
799+
uid = from_vfsuid(mnt_userns, fs_userns, vfsuid);
797800
entry->e_id = cpu_to_le32(from_kuid(&init_user_ns, uid));
798801
break;
799802
case ACL_GROUP:
800803
gid = make_kgid(&init_user_ns, le32_to_cpu(entry->e_id));
801804
vfsgid = VFSGIDT_INIT(gid);
802-
gid = from_vfsgid(mnt_userns, &init_user_ns, vfsgid);
805+
gid = from_vfsgid(mnt_userns, fs_userns, vfsgid);
803806
entry->e_id = cpu_to_le32(from_kgid(&init_user_ns, gid));
804807
break;
805808
default:

0 commit comments

Comments
 (0)