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 <[email protected]>
truenas · Aug 12, 2024 · 1bc02e2 · 1bc02e2
1 parent 072c9ab
commit 1bc02e2
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 8 deletions.
diff --git 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
@@ -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);
 }