From 1bc02e2c2c4be81432b189b3019aa8c59f3d97ff Mon Sep 17 00:00:00 2001 From: Andrew Walker Date: Mon, 12 Aug 2024 12:41:55 -0700 Subject: [PATCH] Fix namespace handling in zpl_permission (#244) This commit fixes user / idmap namespaces in zpl_permission. ZFS updates to address kernel changes were subtly broken and passing the wrong namespace to generic_permission(). Since zpl_permission was initially written, zfs_zaccess() has become idmap-aware. This commit switches from using zfs_access to zfs_zaccess() and improves zfs_zaccess_aces_check() so that uids / gids in ACL entries are converted via idmap configuration prior to checking access. Signed-off-by: Andrew Walker --- module/os/linux/zfs/zfs_acl.c | 7 ++++++- module/os/linux/zfs/zpl_xattr.c | 34 ++++++++++++++++++++++++++------- 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/module/os/linux/zfs/zfs_acl.c b/module/os/linux/zfs/zfs_acl.c index b2a1d265998b..e83672c468fa 100644 --- a/module/os/linux/zfs/zfs_acl.c +++ b/module/os/linux/zfs/zfs_acl.c @@ -2442,8 +2442,11 @@ zfs_zaccess_aces_check(znode_t *zp, uint32_t *working_mode, break; case OWNING_GROUP: who = gowner; - zfs_fallthrough; + checkit = zfs_groupmember(zfsvfs, who, cr); + break; case ACE_IDENTIFIER_GROUP: + who = zfs_gid_to_vfsgid(mnt_ns, zfs_i_user_ns(ZTOI(zp)), + who); checkit = zfs_groupmember(zfsvfs, who, cr); break; case ACE_EVERYONE: @@ -2454,6 +2457,8 @@ zfs_zaccess_aces_check(znode_t *zp, uint32_t *working_mode, default: if (entry_type == 0) { uid_t newid; + who = zfs_uid_to_vfsuid(mnt_ns, + zfs_i_user_ns(ZTOI(zp)), who); newid = zfs_fuid_map_id(zfsvfs, who, cr, ZFS_ACE_USER); diff --git a/module/os/linux/zfs/zpl_xattr.c b/module/os/linux/zfs/zpl_xattr.c index 4d6ac1049500..f733d5e1ba39 100644 --- a/module/os/linux/zfs/zpl_xattr.c +++ b/module/os/linux/zfs/zpl_xattr.c @@ -1507,6 +1507,7 @@ zpl_permission(struct inode *ip, int mask) { int to_check = 0, i, ret; cred_t *cr = NULL; + zfsvfs_t *zfsvfs = NULL; /* * If NFSv4 ACLs are not being used, go back to @@ -1516,9 +1517,10 @@ zpl_permission(struct inode *ip, int mask) */ if ((ITOZSB(ip)->z_acl_type != ZFS_ACLTYPE_NFSV4) || ((ITOZ(ip)->z_pflags & ZFS_ACL_TRIVIAL && GENERIC_MASK(mask)))) { -#if (defined(HAVE_IOPS_PERMISSION_USERNS) || \ - defined(HAVE_IOPS_PERMISSION_IDMAP)) - return (generic_permission(zfs_init_idmap, ip, mask)); +#if defined(HAVE_IOPS_PERMISSION_USERNS) + return (generic_permission(userns, ip, mask)); +#elif defined(HAVE_IOPS_PERMISSION_IDMAP) + return (generic_permission(idmap, ip, mask)); #else return (generic_permission(ip, mask)); #endif @@ -1535,9 +1537,10 @@ zpl_permission(struct inode *ip, int mask) * NFSv4 ACE. Pass back to default kernel permissions check. */ if (to_check == 0) { -#if (defined(HAVE_IOPS_PERMISSION_USERNS) || \ - defined(HAVE_IOPS_PERMISSION_IDMAP)) - return (generic_permission(zfs_init_idmap, ip, mask)); +#if defined(HAVE_IOPS_PERMISSION_USERNS) + return (generic_permission(userns, ip, mask)); +#elif defined(HAVE_IOPS_PERMISSION_IDMAP) + return (generic_permission(idmap, ip, mask)); #else return (generic_permission(ip, mask)); #endif @@ -1614,7 +1617,24 @@ zpl_permission(struct inode *ip, int mask) return (-ECHILD); } - ret = -zfs_access(ITOZ(ip), to_check, V_ACE_MASK, cr); + zfsvfs = ZTOZSB(ITOZ(ip)); + + if ((ret = -zfs_enter_verify_zp(zfsvfs, ITOZ(ip), FTAG)) != 0) + return (ret); + +#if defined(HAVE_IOPS_PERMISSION_USERNS) + ret = -zfs_zaccess(ITOZ(ip), to_check, V_ACE_MASK, B_FALSE, cr, + userns); +#elif defined(HAVE_IOPS_PERMISSION_IDMAP) + ret = -zfs_zaccess(ITOZ(ip), to_check, V_ACE_MASK, B_FALSE, cr, + idmap); +#else + ret = -zfs_zaccess(ITOZ(ip), to_check, V_ACE_MASK, B_FALSE, cr, + zfs_init_idmap); +#endif + + zfs_exit(zfsvfs, FTAG); + crfree(cr); return (ret); }