Skip to content

Commit 0c5fd88

Browse files
committed
acl: move idmapped mount fixup into vfs_{g,s}etxattr()
This cycle we added support for mounting overlayfs on top of idmapped mounts. Recently I've started looking into potential corner cases when trying to add additional tests and I noticed that reporting for POSIX ACLs is currently wrong when using idmapped layers with overlayfs mounted on top of it. I'm going to give a rather detailed explanation to both the origin of the problem and the solution. Let's assume the user creates the following directory layout and they have a rootfs /var/lib/lxc/c1/rootfs. The files in this rootfs are owned as you would expect files on your host system to be owned. For example, ~/.bashrc for your regular user would be owned by 1000:1000 and /root/.bashrc would be owned by 0:0. IOW, this is just regular boring filesystem tree on an ext4 or xfs filesystem. The user chooses to set POSIX ACLs using the setfacl binary granting the user with uid 4 read, write, and execute permissions for their .bashrc file: setfacl -m u:4:rwx /var/lib/lxc/c2/rootfs/home/ubuntu/.bashrc Now they to expose the whole rootfs to a container using an idmapped mount. So they first create: mkdir -pv /vol/contpool/{ctrover,merge,lowermap,overmap} mkdir -pv /vol/contpool/ctrover/{over,work} chown 10000000:10000000 /vol/contpool/ctrover/{over,work} The user now creates an idmapped mount for the rootfs: mount-idmapped/mount-idmapped --map-mount=b:0:10000000:65536 \ /var/lib/lxc/c2/rootfs \ /vol/contpool/lowermap This for example makes it so that /var/lib/lxc/c2/rootfs/home/ubuntu/.bashrc which is owned by uid and gid 1000 as being owned by uid and gid 10001000 at /vol/contpool/lowermap/home/ubuntu/.bashrc. Assume the user wants to expose these idmapped mounts through an overlayfs mount to a container. mount -t overlay overlay \ -o lowerdir=/vol/contpool/lowermap, \ upperdir=/vol/contpool/overmap/over, \ workdir=/vol/contpool/overmap/work \ /vol/contpool/merge The user can do this in two ways: (1) Mount overlayfs in the initial user namespace and expose it to the container. (2) Mount overlayfs on top of the idmapped mounts inside of the container's user namespace. Let's assume the user chooses the (1) option and mounts overlayfs on the host and then changes into a container which uses the idmapping 0:10000000:65536 which is the same used for the two idmapped mounts. Now the user tries to retrieve the POSIX ACLs using the getfacl command getfacl -n /vol/contpool/lowermap/home/ubuntu/.bashrc and to their surprise they see: # file: vol/contpool/merge/home/ubuntu/.bashrc # owner: 1000 # group: 1000 user::rw- user:4294967295:rwx group::r-- mask::rwx other::r-- indicating the the uid wasn't correctly translated according to the idmapped mount. The problem is how we currently translate POSIX ACLs. Let's inspect the callchain in this example: idmapped mount /vol/contpool/merge: 0:10000000:65536 caller's idmapping: 0:10000000:65536 overlayfs idmapping (ofs->creator_cred): 0:0:4k /* initial idmapping */ sys_getxattr() -> path_getxattr() -> getxattr() -> do_getxattr() |> vfs_getxattr() | -> __vfs_getxattr() | -> handler->get == ovl_posix_acl_xattr_get() | -> ovl_xattr_get() | -> vfs_getxattr() | -> __vfs_getxattr() | -> handler->get() /* lower filesystem callback */ |> posix_acl_fix_xattr_to_user() { 4 = make_kuid(&init_user_ns, 4); 4 = mapped_kuid_fs(&init_user_ns /* no idmapped mount */, 4); /* FAILURE */ -1 = from_kuid(0:10000000:65536 /* caller's idmapping */, 4); } If the user chooses to use option (2) and mounts overlayfs on top of idmapped mounts inside the container things don't look that much better: idmapped mount /vol/contpool/merge: 0:10000000:65536 caller's idmapping: 0:10000000:65536 overlayfs idmapping (ofs->creator_cred): 0:10000000:65536 sys_getxattr() -> path_getxattr() -> getxattr() -> do_getxattr() |> vfs_getxattr() | -> __vfs_getxattr() | -> handler->get == ovl_posix_acl_xattr_get() | -> ovl_xattr_get() | -> vfs_getxattr() | -> __vfs_getxattr() | -> handler->get() /* lower filesystem callback */ |> posix_acl_fix_xattr_to_user() { 4 = make_kuid(&init_user_ns, 4); 4 = mapped_kuid_fs(&init_user_ns, 4); /* FAILURE */ -1 = from_kuid(0:10000000:65536 /* caller's idmapping */, 4); } As is easily seen the problem arises because the idmapping of the lower mount isn't taken into account as all of this happens in do_gexattr(). But do_getxattr() is always called on an overlayfs mount and inode and thus cannot possible take the idmapping of the lower layers into account. This problem is similar for fscaps but there the translation happens as part of vfs_getxattr() already. Let's walk through an fscaps overlayfs callchain: setcap 'cap_net_raw+ep' /var/lib/lxc/c2/rootfs/home/ubuntu/.bashrc The expected outcome here is that we'll receive the cap_net_raw capability as we are able to map the uid associated with the fscap to 0 within our container. IOW, we want to see 0 as the result of the idmapping translations. If the user chooses option (1) we get the following callchain for fscaps: idmapped mount /vol/contpool/merge: 0:10000000:65536 caller's idmapping: 0:10000000:65536 overlayfs idmapping (ofs->creator_cred): 0:0:4k /* initial idmapping */ sys_getxattr() -> path_getxattr() -> getxattr() -> do_getxattr() -> vfs_getxattr() -> xattr_getsecurity() -> security_inode_getsecurity() ________________________________ -> cap_inode_getsecurity() | | { V | 10000000 = make_kuid(0:0:4k /* overlayfs idmapping */, 10000000); | 10000000 = mapped_kuid_fs(0:0:4k /* no idmapped mount */, 10000000); | /* Expected result is 0 and thus that we own the fscap. */ | 0 = from_kuid(0:10000000:65536 /* caller's idmapping */, 10000000); | } | -> vfs_getxattr_alloc() | -> handler->get == ovl_other_xattr_get() | -> vfs_getxattr() | -> xattr_getsecurity() | -> security_inode_getsecurity() | -> cap_inode_getsecurity() | { | 0 = make_kuid(0:0:4k /* lower s_user_ns */, 0); | 10000000 = mapped_kuid_fs(0:10000000:65536 /* idmapped mount */, 0); | 10000000 = from_kuid(0:0:4k /* overlayfs idmapping */, 10000000); | |____________________________________________________________________| } -> vfs_getxattr_alloc() -> handler->get == /* lower filesystem callback */ And if the user chooses option (2) we get: idmapped mount /vol/contpool/merge: 0:10000000:65536 caller's idmapping: 0:10000000:65536 overlayfs idmapping (ofs->creator_cred): 0:10000000:65536 sys_getxattr() -> path_getxattr() -> getxattr() -> do_getxattr() -> vfs_getxattr() -> xattr_getsecurity() -> security_inode_getsecurity() _______________________________ -> cap_inode_getsecurity() | | { V | 10000000 = make_kuid(0:10000000:65536 /* overlayfs idmapping */, 0); | 10000000 = mapped_kuid_fs(0:0:4k /* no idmapped mount */, 10000000); | /* Expected result is 0 and thus that we own the fscap. */ | 0 = from_kuid(0:10000000:65536 /* caller's idmapping */, 10000000); | } | -> vfs_getxattr_alloc() | -> handler->get == ovl_other_xattr_get() | |-> vfs_getxattr() | -> xattr_getsecurity() | -> security_inode_getsecurity() | -> cap_inode_getsecurity() | { | 0 = make_kuid(0:0:4k /* lower s_user_ns */, 0); | 10000000 = mapped_kuid_fs(0:10000000:65536 /* idmapped mount */, 0); | 0 = from_kuid(0:10000000:65536 /* overlayfs idmapping */, 10000000); | |____________________________________________________________________| } -> vfs_getxattr_alloc() -> handler->get == /* lower filesystem callback */ We can see how the translation happens correctly in those cases as the conversion happens within the vfs_getxattr() helper. For POSIX ACLs we need to do something similar. However, in contrast to fscaps we cannot apply the fix directly to the kernel internal posix acl data structure as this would alter the cached values and would also require a rework of how we currently deal with POSIX ACLs in general which almost never take the filesystem idmapping into account (the noteable exception being FUSE but even there the implementation is special) and instead retrieve the raw values based on the initial idmapping. The correct values are then generated right before returning to userspace. The fix for this is to move taking the mount's idmapping into account directly in vfs_getxattr() instead of having it be part of posix_acl_fix_xattr_to_user(). To this end we split out two small and unexported helpers posix_acl_getxattr_idmapped_mnt() and posix_acl_setxattr_idmapped_mnt(). The former to be called in vfs_getxattr() and the latter to be called in vfs_setxattr(). Let's go back to the original example. Assume the user chose option (1) and mounted overlayfs on top of idmapped mounts on the host: idmapped mount /vol/contpool/merge: 0:10000000:65536 caller's idmapping: 0:10000000:65536 overlayfs idmapping (ofs->creator_cred): 0:0:4k /* initial idmapping */ sys_getxattr() -> path_getxattr() -> getxattr() -> do_getxattr() |> vfs_getxattr() | |> __vfs_getxattr() | | -> handler->get == ovl_posix_acl_xattr_get() | | -> ovl_xattr_get() | | -> vfs_getxattr() | | |> __vfs_getxattr() | | | -> handler->get() /* lower filesystem callback */ | | |> posix_acl_getxattr_idmapped_mnt() | | { | | 4 = make_kuid(&init_user_ns, 4); | | 10000004 = mapped_kuid_fs(0:10000000:65536 /* lower idmapped mount */, 4); | | 10000004 = from_kuid(&init_user_ns, 10000004); | | |_______________________ | | } | | | | | |> posix_acl_getxattr_idmapped_mnt() | | { | | V | 10000004 = make_kuid(&init_user_ns, 10000004); | 10000004 = mapped_kuid_fs(&init_user_ns /* no idmapped mount */, 10000004); | 10000004 = from_kuid(&init_user_ns, 10000004); | } |_________________________________________________ | | | | |> posix_acl_fix_xattr_to_user() | { V 10000004 = make_kuid(0:0:4k /* init_user_ns */, 10000004); /* SUCCESS */ 4 = from_kuid(0:10000000:65536 /* caller's idmapping */, 10000004); } And similarly if the user chooses option (1) and mounted overayfs on top of idmapped mounts inside the container: idmapped mount /vol/contpool/merge: 0:10000000:65536 caller's idmapping: 0:10000000:65536 overlayfs idmapping (ofs->creator_cred): 0:10000000:65536 sys_getxattr() -> path_getxattr() -> getxattr() -> do_getxattr() |> vfs_getxattr() | |> __vfs_getxattr() | | -> handler->get == ovl_posix_acl_xattr_get() | | -> ovl_xattr_get() | | -> vfs_getxattr() | | |> __vfs_getxattr() | | | -> handler->get() /* lower filesystem callback */ | | |> posix_acl_getxattr_idmapped_mnt() | | { | | 4 = make_kuid(&init_user_ns, 4); | | 10000004 = mapped_kuid_fs(0:10000000:65536 /* lower idmapped mount */, 4); | | 10000004 = from_kuid(&init_user_ns, 10000004); | | |_______________________ | | } | | | | | |> posix_acl_getxattr_idmapped_mnt() | | { V | 10000004 = make_kuid(&init_user_ns, 10000004); | 10000004 = mapped_kuid_fs(&init_user_ns /* no idmapped mount */, 10000004); | 10000004 = from_kuid(0(&init_user_ns, 10000004); | |_________________________________________________ | } | | | |> posix_acl_fix_xattr_to_user() | { V 10000004 = make_kuid(0:0:4k /* init_user_ns */, 10000004); /* SUCCESS */ 4 = from_kuid(0:10000000:65536 /* caller's idmappings */, 10000004); } The last remaining problem we need to fix here is ovl_get_acl(). During ovl_permission() overlayfs will call: ovl_permission() -> generic_permission() -> acl_permission_check() -> check_acl() -> get_acl() -> inode->i_op->get_acl() == ovl_get_acl() > get_acl() /* on the underlying filesystem) ->inode->i_op->get_acl() == /*lower filesystem callback */ -> posix_acl_permission() passing through the get_acl request to the underlying filesystem. This will retrieve the acls stored in the lower filesystem without taking the idmapping of the underlying mount into account as this would mean altering the cached values for the lower filesystem. So we block using ACLs for now until we decided on a nice way to fix this. Note this limitation both in the documentation and in the code. The most straightforward solution would be to have ovl_get_acl() simply duplicate the ACLs, update the values according to the idmapped mount and return it to acl_permission_check() so it can be used in posix_acl_permission() forgetting them afterwards. This is a bit heavy handed but fairly straightforward otherwise. Link: brauner/mount-idmapped#9 Link: https://lore.kernel.org/r/[email protected] Cc: Seth Forshee <[email protected]> Cc: Amir Goldstein <[email protected]> Cc: Vivek Goyal <[email protected]> Cc: Christoph Hellwig <[email protected]> Cc: Aleksa Sarai <[email protected]> Cc: Miklos Szeredi <[email protected]> Cc: [email protected] Cc: [email protected] Reviewed-by: Seth Forshee <[email protected]> Signed-off-by: Christian Brauner (Microsoft) <[email protected]>
1 parent c9fa2b0 commit 0c5fd88

File tree

7 files changed

+151
-64
lines changed

7 files changed

+151
-64
lines changed

fs/ksmbd/vfs.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -963,7 +963,7 @@ ssize_t ksmbd_vfs_getxattr(struct user_namespace *user_ns,
963963
*/
964964
int ksmbd_vfs_setxattr(struct user_namespace *user_ns,
965965
struct dentry *dentry, const char *attr_name,
966-
const void *attr_value, size_t attr_size, int flags)
966+
void *attr_value, size_t attr_size, int flags)
967967
{
968968
int err;
969969

fs/ksmbd/vfs.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ ssize_t ksmbd_vfs_casexattr_len(struct user_namespace *user_ns,
109109
int attr_name_len);
110110
int ksmbd_vfs_setxattr(struct user_namespace *user_ns,
111111
struct dentry *dentry, const char *attr_name,
112-
const void *attr_value, size_t attr_size, int flags);
112+
void *attr_value, size_t attr_size, int flags);
113113
int ksmbd_vfs_xattr_stream_name(char *stream_name, char **xattr_stream_name,
114114
size_t *xattr_stream_name_size, int s_type);
115115
int ksmbd_vfs_remove_xattr(struct user_namespace *user_ns,

fs/overlayfs/overlayfs.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,8 @@ static inline int ovl_do_setxattr(struct ovl_fs *ofs, struct dentry *dentry,
249249
const char *name, const void *value,
250250
size_t size, int flags)
251251
{
252-
int err = vfs_setxattr(ovl_upper_mnt_userns(ofs), dentry, name, value, size, flags);
252+
int err = vfs_setxattr(ovl_upper_mnt_userns(ofs), dentry, name,
253+
(void *)value, size, flags);
253254

254255
pr_debug("setxattr(%pd2, \"%s\", \"%*pE\", %zu, %d) = %i\n",
255256
dentry, name, min((int)size, 48), value, size, flags, err);

fs/posix_acl.c

Lines changed: 106 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -375,8 +375,7 @@ posix_acl_permission(struct user_namespace *mnt_userns, struct inode *inode,
375375
goto check_perm;
376376
break;
377377
case ACL_USER:
378-
uid = mapped_kuid_fs(mnt_userns,
379-
i_user_ns(inode),
378+
uid = mapped_kuid_fs(mnt_userns, &init_user_ns,
380379
pa->e_uid);
381380
if (uid_eq(uid, current_fsuid()))
382381
goto mask;
@@ -390,8 +389,7 @@ posix_acl_permission(struct user_namespace *mnt_userns, struct inode *inode,
390389
}
391390
break;
392391
case ACL_GROUP:
393-
gid = mapped_kgid_fs(mnt_userns,
394-
i_user_ns(inode),
392+
gid = mapped_kgid_fs(mnt_userns, &init_user_ns,
395393
pa->e_gid);
396394
if (in_group_p(gid)) {
397395
found = 1;
@@ -710,46 +708,127 @@ EXPORT_SYMBOL(posix_acl_update_mode);
710708
/*
711709
* Fix up the uids and gids in posix acl extended attributes in place.
712710
*/
713-
static void posix_acl_fix_xattr_userns(
714-
struct user_namespace *to, struct user_namespace *from,
715-
struct user_namespace *mnt_userns,
716-
void *value, size_t size, bool from_user)
711+
static int posix_acl_fix_xattr_common(void *value, size_t size)
712+
{
713+
struct posix_acl_xattr_header *header = value;
714+
int count;
715+
716+
if (!header)
717+
return -EINVAL;
718+
if (size < sizeof(struct posix_acl_xattr_header))
719+
return -EINVAL;
720+
if (header->a_version != cpu_to_le32(POSIX_ACL_XATTR_VERSION))
721+
return -EINVAL;
722+
723+
count = posix_acl_xattr_count(size);
724+
if (count < 0)
725+
return -EINVAL;
726+
if (count == 0)
727+
return -EINVAL;
728+
729+
return count;
730+
}
731+
732+
void posix_acl_getxattr_idmapped_mnt(struct user_namespace *mnt_userns,
733+
const struct inode *inode,
734+
void *value, size_t size)
717735
{
718736
struct posix_acl_xattr_header *header = value;
719737
struct posix_acl_xattr_entry *entry = (void *)(header + 1), *end;
720738
int count;
739+
vfsuid_t vfsuid;
740+
vfsgid_t vfsgid;
721741
kuid_t uid;
722742
kgid_t gid;
723743

724-
if (!value)
744+
if (no_idmapping(mnt_userns, i_user_ns(inode)))
725745
return;
726-
if (size < sizeof(struct posix_acl_xattr_header))
746+
747+
count = posix_acl_fix_xattr_common(value, size);
748+
if (count < 0)
727749
return;
728-
if (header->a_version != cpu_to_le32(POSIX_ACL_XATTR_VERSION))
750+
751+
for (end = entry + count; entry != end; entry++) {
752+
switch (le16_to_cpu(entry->e_tag)) {
753+
case ACL_USER:
754+
uid = make_kuid(&init_user_ns, le32_to_cpu(entry->e_id));
755+
vfsuid = make_vfsuid(mnt_userns, &init_user_ns, uid);
756+
entry->e_id = cpu_to_le32(from_kuid(&init_user_ns,
757+
vfsuid_into_kuid(vfsuid)));
758+
break;
759+
case ACL_GROUP:
760+
gid = make_kgid(&init_user_ns, le32_to_cpu(entry->e_id));
761+
vfsgid = make_vfsgid(mnt_userns, &init_user_ns, gid);
762+
entry->e_id = cpu_to_le32(from_kgid(&init_user_ns,
763+
vfsgid_into_kgid(vfsgid)));
764+
break;
765+
default:
766+
break;
767+
}
768+
}
769+
}
770+
771+
void posix_acl_setxattr_idmapped_mnt(struct user_namespace *mnt_userns,
772+
const struct inode *inode,
773+
void *value, size_t size)
774+
{
775+
struct posix_acl_xattr_header *header = value;
776+
struct posix_acl_xattr_entry *entry = (void *)(header + 1), *end;
777+
int count;
778+
vfsuid_t vfsuid;
779+
vfsgid_t vfsgid;
780+
kuid_t uid;
781+
kgid_t gid;
782+
783+
if (no_idmapping(mnt_userns, i_user_ns(inode)))
729784
return;
730785

731-
count = posix_acl_xattr_count(size);
786+
count = posix_acl_fix_xattr_common(value, size);
732787
if (count < 0)
733788
return;
734-
if (count == 0)
789+
790+
for (end = entry + count; entry != end; entry++) {
791+
switch (le16_to_cpu(entry->e_tag)) {
792+
case ACL_USER:
793+
uid = make_kuid(&init_user_ns, le32_to_cpu(entry->e_id));
794+
vfsuid = VFSUIDT_INIT(uid);
795+
uid = from_vfsuid(mnt_userns, &init_user_ns, vfsuid);
796+
entry->e_id = cpu_to_le32(from_kuid(&init_user_ns, uid));
797+
break;
798+
case ACL_GROUP:
799+
gid = make_kgid(&init_user_ns, le32_to_cpu(entry->e_id));
800+
vfsgid = VFSGIDT_INIT(gid);
801+
gid = from_vfsgid(mnt_userns, &init_user_ns, vfsgid);
802+
entry->e_id = cpu_to_le32(from_kgid(&init_user_ns, gid));
803+
break;
804+
default:
805+
break;
806+
}
807+
}
808+
}
809+
810+
static void posix_acl_fix_xattr_userns(
811+
struct user_namespace *to, struct user_namespace *from,
812+
void *value, size_t size)
813+
{
814+
struct posix_acl_xattr_header *header = value;
815+
struct posix_acl_xattr_entry *entry = (void *)(header + 1), *end;
816+
int count;
817+
kuid_t uid;
818+
kgid_t gid;
819+
820+
count = posix_acl_fix_xattr_common(value, size);
821+
if (count < 0)
735822
return;
736823

737824
for (end = entry + count; entry != end; entry++) {
738825
switch(le16_to_cpu(entry->e_tag)) {
739826
case ACL_USER:
740827
uid = make_kuid(from, le32_to_cpu(entry->e_id));
741-
if (from_user)
742-
uid = mapped_kuid_user(mnt_userns, &init_user_ns, uid);
743-
else
744-
uid = mapped_kuid_fs(mnt_userns, &init_user_ns, uid);
745828
entry->e_id = cpu_to_le32(from_kuid(to, uid));
746829
break;
747830
case ACL_GROUP:
748831
gid = make_kgid(from, le32_to_cpu(entry->e_id));
749-
if (from_user)
750-
gid = mapped_kgid_user(mnt_userns, &init_user_ns, gid);
751-
else
752-
gid = mapped_kgid_fs(mnt_userns, &init_user_ns, gid);
753832
entry->e_id = cpu_to_le32(from_kgid(to, gid));
754833
break;
755834
default:
@@ -758,34 +837,20 @@ static void posix_acl_fix_xattr_userns(
758837
}
759838
}
760839

761-
void posix_acl_fix_xattr_from_user(struct user_namespace *mnt_userns,
762-
struct inode *inode,
763-
void *value, size_t size)
840+
void posix_acl_fix_xattr_from_user(void *value, size_t size)
764841
{
765842
struct user_namespace *user_ns = current_user_ns();
766-
767-
/* Leave ids untouched on non-idmapped mounts. */
768-
if (no_idmapping(mnt_userns, i_user_ns(inode)))
769-
mnt_userns = &init_user_ns;
770-
if ((user_ns == &init_user_ns) && (mnt_userns == &init_user_ns))
843+
if (user_ns == &init_user_ns)
771844
return;
772-
posix_acl_fix_xattr_userns(&init_user_ns, user_ns, mnt_userns, value,
773-
size, true);
845+
posix_acl_fix_xattr_userns(&init_user_ns, user_ns, value, size);
774846
}
775847

776-
void posix_acl_fix_xattr_to_user(struct user_namespace *mnt_userns,
777-
struct inode *inode,
778-
void *value, size_t size)
848+
void posix_acl_fix_xattr_to_user(void *value, size_t size)
779849
{
780850
struct user_namespace *user_ns = current_user_ns();
781-
782-
/* Leave ids untouched on non-idmapped mounts. */
783-
if (no_idmapping(mnt_userns, i_user_ns(inode)))
784-
mnt_userns = &init_user_ns;
785-
if ((user_ns == &init_user_ns) && (mnt_userns == &init_user_ns))
851+
if (user_ns == &init_user_ns)
786852
return;
787-
posix_acl_fix_xattr_userns(user_ns, &init_user_ns, mnt_userns, value,
788-
size, false);
853+
posix_acl_fix_xattr_userns(user_ns, &init_user_ns, value, size);
789854
}
790855

791856
/*

fs/xattr.c

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -282,22 +282,32 @@ __vfs_setxattr_locked(struct user_namespace *mnt_userns, struct dentry *dentry,
282282
}
283283
EXPORT_SYMBOL_GPL(__vfs_setxattr_locked);
284284

285+
static inline bool is_posix_acl_xattr(const char *name)
286+
{
287+
return (strcmp(name, XATTR_NAME_POSIX_ACL_ACCESS) == 0) ||
288+
(strcmp(name, XATTR_NAME_POSIX_ACL_DEFAULT) == 0);
289+
}
290+
285291
int
286292
vfs_setxattr(struct user_namespace *mnt_userns, struct dentry *dentry,
287-
const char *name, const void *value, size_t size, int flags)
293+
const char *name, void *value, size_t size, int flags)
288294
{
289295
struct inode *inode = dentry->d_inode;
290296
struct inode *delegated_inode = NULL;
291297
const void *orig_value = value;
292298
int error;
293299

294300
if (size && strcmp(name, XATTR_NAME_CAPS) == 0) {
295-
error = cap_convert_nscap(mnt_userns, dentry, &value, size);
301+
error = cap_convert_nscap(mnt_userns, dentry,
302+
(const void **)&value, size);
296303
if (error < 0)
297304
return error;
298305
size = error;
299306
}
300307

308+
if (size && is_posix_acl_xattr(name))
309+
posix_acl_setxattr_idmapped_mnt(mnt_userns, inode, value, size);
310+
301311
retry_deleg:
302312
inode_lock(inode);
303313
error = __vfs_setxattr_locked(mnt_userns, dentry, name, value, size,
@@ -431,7 +441,10 @@ vfs_getxattr(struct user_namespace *mnt_userns, struct dentry *dentry,
431441
return ret;
432442
}
433443
nolsm:
434-
return __vfs_getxattr(dentry, inode, name, value, size);
444+
error = __vfs_getxattr(dentry, inode, name, value, size);
445+
if (error > 0 && is_posix_acl_xattr(name))
446+
posix_acl_getxattr_idmapped_mnt(mnt_userns, inode, value, size);
447+
return error;
435448
}
436449
EXPORT_SYMBOL_GPL(vfs_getxattr);
437450

@@ -577,8 +590,7 @@ static void setxattr_convert(struct user_namespace *mnt_userns,
577590
if (ctx->size &&
578591
((strcmp(ctx->kname->name, XATTR_NAME_POSIX_ACL_ACCESS) == 0) ||
579592
(strcmp(ctx->kname->name, XATTR_NAME_POSIX_ACL_DEFAULT) == 0)))
580-
posix_acl_fix_xattr_from_user(mnt_userns, d_inode(d),
581-
ctx->kvalue, ctx->size);
593+
posix_acl_fix_xattr_from_user(ctx->kvalue, ctx->size);
582594
}
583595

584596
int do_setxattr(struct user_namespace *mnt_userns, struct dentry *dentry,
@@ -695,8 +707,7 @@ do_getxattr(struct user_namespace *mnt_userns, struct dentry *d,
695707
if (error > 0) {
696708
if ((strcmp(kname, XATTR_NAME_POSIX_ACL_ACCESS) == 0) ||
697709
(strcmp(kname, XATTR_NAME_POSIX_ACL_DEFAULT) == 0))
698-
posix_acl_fix_xattr_to_user(mnt_userns, d_inode(d),
699-
ctx->kvalue, error);
710+
posix_acl_fix_xattr_to_user(ctx->kvalue, error);
700711
if (ctx->size && copy_to_user(ctx->value, ctx->kvalue, error))
701712
error = -EFAULT;
702713
} else if (error == -ERANGE && ctx->size >= XATTR_SIZE_MAX) {

include/linux/posix_acl_xattr.h

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -33,21 +33,31 @@ posix_acl_xattr_count(size_t size)
3333
}
3434

3535
#ifdef CONFIG_FS_POSIX_ACL
36-
void posix_acl_fix_xattr_from_user(struct user_namespace *mnt_userns,
37-
struct inode *inode,
38-
void *value, size_t size);
39-
void posix_acl_fix_xattr_to_user(struct user_namespace *mnt_userns,
40-
struct inode *inode,
41-
void *value, size_t size);
36+
void posix_acl_fix_xattr_from_user(void *value, size_t size);
37+
void posix_acl_fix_xattr_to_user(void *value, size_t size);
38+
void posix_acl_getxattr_idmapped_mnt(struct user_namespace *mnt_userns,
39+
const struct inode *inode,
40+
void *value, size_t size);
41+
void posix_acl_setxattr_idmapped_mnt(struct user_namespace *mnt_userns,
42+
const struct inode *inode,
43+
void *value, size_t size);
4244
#else
43-
static inline void posix_acl_fix_xattr_from_user(struct user_namespace *mnt_userns,
44-
struct inode *inode,
45-
void *value, size_t size)
45+
static inline void posix_acl_fix_xattr_from_user(void *value, size_t size)
4646
{
4747
}
48-
static inline void posix_acl_fix_xattr_to_user(struct user_namespace *mnt_userns,
49-
struct inode *inode,
50-
void *value, size_t size)
48+
static inline void posix_acl_fix_xattr_to_user(void *value, size_t size)
49+
{
50+
}
51+
static inline void
52+
posix_acl_getxattr_idmapped_mnt(struct user_namespace *mnt_userns,
53+
const struct inode *inode, void *value,
54+
size_t size)
55+
{
56+
}
57+
static inline void
58+
posix_acl_setxattr_idmapped_mnt(struct user_namespace *mnt_userns,
59+
const struct inode *inode, void *value,
60+
size_t size)
5161
{
5262
}
5363
#endif

include/linux/xattr.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ int __vfs_setxattr_locked(struct user_namespace *, struct dentry *,
6161
const char *, const void *, size_t, int,
6262
struct inode **);
6363
int vfs_setxattr(struct user_namespace *, struct dentry *, const char *,
64-
const void *, size_t, int);
64+
void *, size_t, int);
6565
int __vfs_removexattr(struct user_namespace *, struct dentry *, const char *);
6666
int __vfs_removexattr_locked(struct user_namespace *, struct dentry *,
6767
const char *, struct inode **);

0 commit comments

Comments
 (0)