Merge tag 'fs.idmapped.overlay.acl.v5.20' of git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux

Pull acl updates from Christian Brauner:
 "Last cycle we introduced support for mounting overlayfs on top of
  idmapped mounts. While looking into additional testing we realized
  that posix acls don't really work correctly with stacking filesystems
  on top of idmapped layers.

  We already knew what the fix were but it would require work that is
  more suitable for the merge window so we turned off posix acls for
  v5.19 for overlayfs on top of idmapped layers with Miklos routing my
  patch upstream in 72a8e05 ("Merge tag 'ovl-fixes-5.19-rc7' [..]").

  This contains the work to support posix acls for overlayfs on top of
  idmapped layers. Since the posix acl fixes should use the new
  vfs{g,u}id_t work the associated branch has been merged in. (We sent a
  pull request for this earlier.)

  We've also pulled in Miklos pull request containing my patch to turn
  of posix acls on top of idmapped layers. This allowed us to avoid
  rebasing the branch which we didn't like because we were already at
  rc7 by then. Merging it in allows this branch to first fix posix acls
  and then to cleanly revert the temporary fix it brought in by commit
  4a47c63 ("ovl: turn of SB_POSIXACL with idmapped layers
  temporarily").

  The last patch in this series adds Seth Forshee as a co-maintainer for
  idmapped mounts. Seth has been integral to all of this work and is
  also the main architect behind the filesystem idmapping work which
  ultimately made filesystems such as FUSE and overlayfs available in
  containers. He continues to be active in both development and review.
  I'm very happy he decided to help and he has my full trust. This
  increases the bus factor which is always great for work like this. I'm
  honestly very excited about this because I think in general we don't
  do great in the bringing on new maintainers department"

For more explanations of the ACL issues, see

  https://lore.kernel.org/all/20220801145520.1532837-1-brauner@kernel.org/

* tag 'fs.idmapped.overlay.acl.v5.20' of git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux:
  Add Seth Forshee as co-maintainer for idmapped mounts
  Revert "ovl: turn of SB_POSIXACL with idmapped layers temporarily"
  ovl: handle idmappings in ovl_get_acl()
  acl: make posix_acl_clone() available to overlayfs
  acl: port to vfs{g,u}id_t
  acl: move idmapped mount fixup into vfs_{g,s}etxattr()
  mnt_idmapping: add vfs[g,u]id_into_k[g,u]id()

Author: torvalds

Documentation/filesystems/overlayfs.rst

@@ -466,10 +466,6 @@ overlay filesystem and the value of st_ino for filesystem objects may not be
 persistent and could change even while the overlay filesystem is mounted, as
 summarized in the `Inode properties`_ table above.
 
-4) "idmapped mounts"
-When the upper or lower layers are idmapped mounts overlayfs will be mounted
-without support for POSIX Access Control Lists (ACLs). This limitation will
-eventually be lifted.
 
 Changes to underlying filesystems
 ---------------------------------

MAINTAINERS

@@ -9620,6 +9620,7 @@ F:	drivers/input/misc/ideapad_slidebar.c
 
 IDMAPPED MOUNTS
 M:	Christian Brauner <brauner@kernel.org>
+M:	Seth Forshee <sforshee@kernel.org>
 L:	linux-fsdevel@vger.kernel.org
 S:	Maintained
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git

fs/ksmbd/vfs.c

@@ -963,7 +963,7 @@ ssize_t ksmbd_vfs_getxattr(struct user_namespace *user_ns,
  */
 int ksmbd_vfs_setxattr(struct user_namespace *user_ns,
 		       struct dentry *dentry, const char *attr_name,
-		       const void *attr_value, size_t attr_size, int flags)
+		       void *attr_value, size_t attr_size, int flags)
 {
 	int err;
 

fs/ksmbd/vfs.h

@@ -109,7 +109,7 @@ ssize_t ksmbd_vfs_casexattr_len(struct user_namespace *user_ns,
 				int attr_name_len);
 int ksmbd_vfs_setxattr(struct user_namespace *user_ns,
 		       struct dentry *dentry, const char *attr_name,
-		       const void *attr_value, size_t attr_size, int flags);
+		       void *attr_value, size_t attr_size, int flags);
 int ksmbd_vfs_xattr_stream_name(char *stream_name, char **xattr_stream_name,
 				size_t *xattr_stream_name_size, int s_type);
 int ksmbd_vfs_remove_xattr(struct user_namespace *user_ns,

fs/overlayfs/inode.c

@@ -454,23 +454,94 @@ ssize_t ovl_listxattr(struct dentry *dentry, char *list, size_t size)
 	return res;
 }
 
+/*
+ * Apply the idmapping of the layer to POSIX ACLs. The caller must pass a clone
+ * of the POSIX ACLs retrieved from the lower layer to this function to not
+ * alter the POSIX ACLs for the underlying filesystem.
+ */
+static void ovl_idmap_posix_acl(struct user_namespace *mnt_userns,
+				struct posix_acl *acl)
+{
+	for (unsigned int i = 0; i < acl->a_count; i++) {
+		vfsuid_t vfsuid;
+		vfsgid_t vfsgid;
+
+		struct posix_acl_entry *e = &acl->a_entries[i];
+		switch (e->e_tag) {
+		case ACL_USER:
+			vfsuid = make_vfsuid(mnt_userns, &init_user_ns, e->e_uid);
+			e->e_uid = vfsuid_into_kuid(vfsuid);
+			break;
+		case ACL_GROUP:
+			vfsgid = make_vfsgid(mnt_userns, &init_user_ns, e->e_gid);
+			e->e_gid = vfsgid_into_kgid(vfsgid);
+			break;
+		}
+	}
+}
+
+/*
+ * When the relevant layer is an idmapped mount we need to take the idmapping
+ * of the layer into account and translate any ACL_{GROUP,USER} values
+ * according to the idmapped mount.
+ *
+ * We cannot alter the ACLs returned from the relevant layer as that would
+ * alter the cached values filesystem wide for the lower filesystem. Instead we
+ * can clone the ACLs and then apply the relevant idmapping of the layer.
+ *
+ * This is obviously only relevant when idmapped layers are used.
+ */
 struct posix_acl *ovl_get_acl(struct inode *inode, int type, bool rcu)
 {
 	struct inode *realinode = ovl_inode_real(inode);
-	const struct cred *old_cred;
-	struct posix_acl *acl;
+	struct posix_acl *acl, *clone;
+	struct path realpath;
 
 	if (!IS_ENABLED(CONFIG_FS_POSIX_ACL) || !IS_POSIXACL(realinode))
 		return NULL;
 
-	if (rcu)
-		return get_cached_acl_rcu(realinode, type);
+	/* Careful in RCU walk mode */
+	ovl_i_path_real(inode, &realpath);
+	if (!realpath.dentry) {
+		WARN_ON(!rcu);
+		return ERR_PTR(-ECHILD);
+	}
 
-	old_cred = ovl_override_creds(inode->i_sb);
-	acl = get_acl(realinode, type);
-	revert_creds(old_cred);
+	if (rcu) {
+		acl = get_cached_acl_rcu(realinode, type);
+	} else {
+		const struct cred *old_cred;
+
+		old_cred = ovl_override_creds(inode->i_sb);
+		acl = get_acl(realinode, type);
+		revert_creds(old_cred);
+	}
+	/*
+	 * If there are no POSIX ACLs, or we encountered an error,
+	 * or the layer isn't idmapped we don't need to do anything.
+	 */
+	if (!is_idmapped_mnt(realpath.mnt) || IS_ERR_OR_NULL(acl))
+		return acl;
 
-	return acl;
+	/*
+	 * We only get here if the layer is idmapped. So drop out of RCU path
+	 * walk so we can clone the ACLs. There's no need to release the ACLs
+	 * since get_cached_acl_rcu() doesn't take a reference on the ACLs.
+	 */
+	if (rcu)
+		return ERR_PTR(-ECHILD);
+
+	clone = posix_acl_clone(acl, GFP_KERNEL);
+	if (!clone)
+		clone = ERR_PTR(-ENOMEM);
+	else
+		ovl_idmap_posix_acl(mnt_user_ns(realpath.mnt), clone);
+	/*
+	 * Since we're not in RCU path walk we always need to release the
+	 * original ACLs.
+	 */
+	posix_acl_release(acl);
+	return clone;
 }
 
 int ovl_update_time(struct inode *inode, struct timespec64 *ts, int flags)

fs/overlayfs/overlayfs.h

@@ -249,7 +249,8 @@ static inline int ovl_do_setxattr(struct ovl_fs *ofs, struct dentry *dentry,
 				  const char *name, const void *value,
 				  size_t size, int flags)
 {
-	int err = vfs_setxattr(ovl_upper_mnt_userns(ofs), dentry, name, value, size, flags);
+	int err = vfs_setxattr(ovl_upper_mnt_userns(ofs), dentry, name,
+			       (void *)value, size, flags);
 
 	pr_debug("setxattr(%pd2, \"%s\", \"%*pE\", %zu, %d) = %i\n",
 		 dentry, name, min((int)size, 48), value, size, flags, err);

fs/overlayfs/super.c

@@ -1003,9 +1003,6 @@ ovl_posix_acl_xattr_get(const struct xattr_handler *handler,
 			struct dentry *dentry, struct inode *inode,
 			const char *name, void *buffer, size_t size)
 {
-	if (!IS_POSIXACL(inode))
-		return -EOPNOTSUPP;
-
 	return ovl_xattr_get(dentry, inode, handler->name, buffer, size);
 }
 
@@ -1021,9 +1018,6 @@ ovl_posix_acl_xattr_set(const struct xattr_handler *handler,
 	struct posix_acl *acl = NULL;
 	int err;
 
-	if (!IS_POSIXACL(inode))
-		return -EOPNOTSUPP;
-
 	/* Check that everything is OK before copy-up */
 	if (value) {
 		acl = posix_acl_from_xattr(&init_user_ns, value, size);
@@ -1966,20 +1960,6 @@ static struct dentry *ovl_get_root(struct super_block *sb,
 	return root;
 }
 
-static bool ovl_has_idmapped_layers(struct ovl_fs *ofs)
-{
-
-	unsigned int i;
-	const struct vfsmount *mnt;
-
-	for (i = 0; i < ofs->numlayer; i++) {
-		mnt = ofs->layers[i].mnt;
-		if (mnt && is_idmapped_mnt(mnt))
-			return true;
-	}
-	return false;
-}
-
 static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 {
 	struct path upperpath = { };
@@ -2149,10 +2129,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_xattr = ofs->config.userxattr ? ovl_user_xattr_handlers :
 		ovl_trusted_xattr_handlers;
 	sb->s_fs_info = ofs;
-	if (ovl_has_idmapped_layers(ofs))
-		pr_warn("POSIX ACLs are not yet supported with idmapped layers, mounting without ACL support.\n");
-	else
-		sb->s_flags |= SB_POSIXACL;
+	sb->s_flags |= SB_POSIXACL;
 	sb->s_iflags |= SB_I_SKIP_SYNC;
 
 	err = -ENOMEM;