[PATCH 0/3] ovl: corner cases of upper with no xattr support

[PATCH 0/3] ovl: corner cases of upper with no xattr support

[Amir Goldstein]

Miklos,

Found one more bug in ovl_set_impure() patch besides the noxattr case.

Added the mount time check and piggy backed the noxattr checks for
setting opaque and redirect.

Checked that all cases work as expected with and without CONFIG_TMPFS_XATTR.

Amir.

Amir Goldstein (3):
  ovl: fix wrong error handling of ovl_set_impure()
  ovl: check on mount time if upper fs supports setting xattr
  ovl: handle rename when upper doesn't supports xattr

 fs/overlayfs/copy_up.c   | 34 ++++++++++++++++++++++++----------
 fs/overlayfs/dir.c       | 33 +++++++++++++++++++++++++--------
 fs/overlayfs/overlayfs.h |  5 +++++
 fs/overlayfs/ovl_entry.h |  2 +-
 fs/overlayfs/super.c     |  6 ++++++
 fs/overlayfs/util.c      |  9 ++++++++-
 6 files changed, 69 insertions(+), 20 deletions(-)

-- 
2.7.4

[PATCH 1/3] ovl: fix wrong error handling of ovl_set_impure()

[Amir Goldstein]

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/dir.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 5ad6ccf..9b221cf 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -973,13 +973,13 @@ static int ovl_rename(struct inode *olddir, struct dentry *old,
 		if (ovl_type_origin(old) && !ovl_type_merge(new->d_parent)) {
 			err = ovl_set_impure(new->d_parent, new_upperdir);
 			if (err)
-				goto out_dput;
+				goto out_revert_creds;
 		}
 		if (!overwrite && ovl_type_origin(new) &&
 		    !ovl_type_merge(old->d_parent)) {
 			err = ovl_set_impure(old->d_parent, old_upperdir);
 			if (err)
-				goto out_dput;
+				goto out_revert_creds;
 		}
 	}
 
-- 
2.7.4

[PATCH 2/3] ovl: check on mount time if upper fs supports setting xattr

[Amir Goldstein]

xattr are needed by overlayfs for setting opaque dir, redirect dir
and copy up origin.

Check at boot time by trying to set the overlay.opaque xattr on the
workdir and if that fails issue a warning message.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/copy_up.c   | 34 ++++++++++++++++++++++++----------
 fs/overlayfs/overlayfs.h |  5 +++++
 fs/overlayfs/ovl_entry.h |  2 +-
 fs/overlayfs/super.c     |  6 ++++++
 fs/overlayfs/util.c      |  7 +++++++
 5 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 051323d..071b547 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -280,6 +280,26 @@ static struct ovl_fh *ovl_encode_fh(struct dentry *lower, uuid_be *uuid)
 	return fh;
 }
 
+int ovl_check_setxattr(struct ovl_fs *ofs, struct dentry *upperdentry,
+		       const char *name, const void *value, size_t size,
+		       int xerr)
+{
+	int err;
+
+	if (ofs->noxattr)
+		return xerr;
+
+	err = ovl_do_setxattr(upperdentry, name, value, size, 0);
+
+	if (err == -EOPNOTSUPP) {
+		pr_warn("overlayfs: cannot set %s xattr on upper\n", name);
+		ofs->noxattr = true;
+		return xerr;
+	}
+
+	return err;
+}
+
 static int ovl_set_origin(struct dentry *dentry, struct dentry *lower,
 			  struct dentry *upper)
 {
@@ -289,7 +309,7 @@ static int ovl_set_origin(struct dentry *dentry, struct dentry *lower,
 	struct ovl_fs *ofs = dentry->d_sb->s_fs_info;
 	int err;
 
-	if (ofs->nooriginxattr)
+	if (ofs->noxattr)
 		return 0;
 
 	/*
@@ -304,18 +324,12 @@ static int ovl_set_origin(struct dentry *dentry, struct dentry *lower,
 			return PTR_ERR(fh);
 	}
 
-	err = ovl_do_setxattr(upper, OVL_XATTR_ORIGIN, fh, fh ? fh->len : 0, 0);
-	kfree(fh);
-
 	/*
 	 * Do not fail when upper doesn't support xattrs.
 	 */
-	if (err == -EOPNOTSUPP) {
-		pr_warn("overlayfs: cannot set " OVL_XATTR_ORIGIN " xattr on upper\n");
-		ofs->nooriginxattr = true;
-		return 0;
-	}
-
+	err = ovl_check_setxattr(ofs, upper, OVL_XATTR_ORIGIN, fh,
+				 fh ? fh->len : 0, 0);
+	kfree(fh);
 	return err;
 }
 
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 8b86bf7..c723895 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -208,6 +208,7 @@ bool ovl_dentry_is_impure(struct dentry *dentry);
 bool ovl_dentry_is_whiteout(struct dentry *dentry);
 void ovl_dentry_set_opaque(struct dentry *dentry);
 void ovl_dentry_set_impure(struct dentry *dentry);
+bool ovl_noxattr(struct super_block *sb);
 bool ovl_redirect_dir(struct super_block *sb);
 void ovl_clear_redirect_dir(struct super_block *sb);
 const char *ovl_dentry_get_redirect(struct dentry *dentry);
@@ -282,3 +283,7 @@ int ovl_copy_up(struct dentry *dentry);
 int ovl_copy_up_flags(struct dentry *dentry, int flags);
 int ovl_copy_xattr(struct dentry *old, struct dentry *new);
 int ovl_set_attr(struct dentry *upper, struct kstat *stat);
+struct ovl_fs;
+int ovl_check_setxattr(struct ovl_fs *ofs, struct dentry *upperdentry,
+		       const char *name, const void *value, size_t size,
+		       int xerr);
diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index b3d9cd6..34bc4a9 100644
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -28,7 +28,7 @@ struct ovl_fs {
 	/* creds of process who forced instantiation of super block */
 	const struct cred *creator_cred;
 	bool tmpfile;
-	bool nooriginxattr;
+	bool noxattr;
 	wait_queue_head_t copyup_wq;
 	/* sb common to all layers */
 	struct super_block *same_sb;
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 9828b7d..d4cc189 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -891,6 +891,12 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 				dput(temp);
 			else
 				pr_warn("overlayfs: upper fs does not support tmpfile.\n");
+
+			/* Check if upper/work fs supports trusted xattr */
+			err = ovl_check_setxattr(ufs, ufs->workdir,
+						 OVL_XATTR_OPAQUE, "y", 1, 1);
+			if (err)
+				pr_warn("overlayfs: upper fs does not support xattr.\n");
 		}
 	}
 
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 6c3a7aa..ce37cf4 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -201,6 +201,13 @@ void ovl_dentry_set_impure(struct dentry *dentry)
 	oe->impure = true;
 }
 
+bool ovl_noxattr(struct super_block *sb)
+{
+	struct ovl_fs *ofs = sb->s_fs_info;
+
+	return ofs->noxattr;
+}
+
 bool ovl_redirect_dir(struct super_block *sb)
 {
 	struct ovl_fs *ofs = sb->s_fs_info;
-- 
2.7.4

[PATCH 3/3] ovl: handle rename when upper doesn't supports xattr

[Amir Goldstein]

On failure to set opaque/redirect xattr on rename, skip setting xattr and
return -EXDEV. Do not fail rename on failure to set impure xattr.

On failure to set opaque xattr when creating a new directory, -EIO is
returned instead of -EOPNOTSUPP.

Any failure to set those xattr will be recorded in super block and
then setting any xattr on upper won't be attempted again.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/dir.c  | 29 +++++++++++++++++++++++------
 fs/overlayfs/util.c |  2 +-
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 9b221cf..3f40afc 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -127,22 +127,39 @@ int ovl_create_real(struct inode *dir, struct dentry *newdentry,
 	return err;
 }
 
-static int ovl_set_opaque(struct dentry *dentry, struct dentry *upperdentry)
+static int ovl_set_opaque_xerr(struct dentry *dentry, struct dentry *upper,
+			       int xerr)
 {
+	struct ovl_fs *ofs = dentry->d_sb->s_fs_info;
 	int err;
 
-	err = ovl_do_setxattr(upperdentry, OVL_XATTR_OPAQUE, "y", 1, 0);
+	err = ovl_check_setxattr(ofs, upper, OVL_XATTR_OPAQUE, "y", 1, xerr);
 	if (!err)
 		ovl_dentry_set_opaque(dentry);
 
 	return err;
 }
 
+static int ovl_set_opaque(struct dentry *dentry, struct dentry *upperdentry)
+{
+	/*
+	 * Fail with -EIO when trying to create opaque dir and upper doesn't
+	 * support xattrs. ovl_rename() calls ovl_set_opaque_xerr(-EXDEV) to
+	 * return a specific error for noxattr case.
+	 */
+	return ovl_set_opaque_xerr(dentry, upperdentry, -EIO);
+}
+
 static int ovl_set_impure(struct dentry *dentry, struct dentry *upperdentry)
 {
+	struct ovl_fs *ofs = dentry->d_sb->s_fs_info;
 	int err;
 
-	err = ovl_do_setxattr(upperdentry, OVL_XATTR_IMPURE, "y", 1, 0);
+	/*
+	 * Do not fail when upper doesn't support xattrs.
+	 * Upper inodes won't have origin nor redirect xattr anyway.
+	 */
+	err = ovl_check_setxattr(ofs, upperdentry, OVL_XATTR_IMPURE, "y", 1, 0);
 	if (!err)
 		ovl_dentry_set_impure(dentry);
 
@@ -959,7 +976,7 @@ static int ovl_rename(struct inode *olddir, struct dentry *old,
 	old_upperdir = ovl_dentry_upper(old->d_parent);
 	new_upperdir = ovl_dentry_upper(new->d_parent);
 
-	if (!samedir) {
+	if (!samedir && !ovl_noxattr(old->d_sb)) {
 		/*
 		 * When moving a merge dir or non-dir with copy up origin into
 		 * a non-merge upper dir (a.k.a pure upper dir), we are making
@@ -1032,7 +1049,7 @@ static int ovl_rename(struct inode *olddir, struct dentry *old,
 		if (ovl_type_merge_or_lower(old))
 			err = ovl_set_redirect(old, samedir);
 		else if (!old_opaque && ovl_type_merge(new->d_parent))
-			err = ovl_set_opaque(old, olddentry);
+			err = ovl_set_opaque_xerr(old, olddentry, -EXDEV);
 		if (err)
 			goto out_dput;
 	}
@@ -1040,7 +1057,7 @@ static int ovl_rename(struct inode *olddir, struct dentry *old,
 		if (ovl_type_merge_or_lower(new))
 			err = ovl_set_redirect(new, samedir);
 		else if (!new_opaque && ovl_type_merge(old->d_parent))
-			err = ovl_set_opaque(new, newdentry);
+			err = ovl_set_opaque_xerr(new, newdentry, -EXDEV);
 		if (err)
 			goto out_dput;
 	}
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index ce37cf4..c5136c5 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -212,7 +212,7 @@ bool ovl_redirect_dir(struct super_block *sb)
 {
 	struct ovl_fs *ofs = sb->s_fs_info;
 
-	return ofs->config.redirect_dir;
+	return ofs->config.redirect_dir && !ofs->noxattr;
 }
 
 void ovl_clear_redirect_dir(struct super_block *sb)
-- 
2.7.4