[PATCH 0/4] ovl: xattr fixes / improvements

[PATCH 0/4] ovl: xattr fixes / improvements

[Andreas Gruenbacher]

Here are some overlayfs fixes.  The first and third should definitely go
in for 4.8; the second is a mere cleanup; the fourth could go in later
but shouldn't hurt now, either.

Thanks,
Andreas

Andreas Gruenbacher (4):
  ovl: Fix OVL_XATTR_PREFIX
  ovl: Get rid of ovl_xattr_noacl_handlers array
  ovl: Switch to generic_removexattr
  ovl: Switch to generic_getxattr

 fs/overlayfs/dir.c       |  4 +--
 fs/overlayfs/inode.c     | 90 ++++++++++++++----------------------------------
 fs/overlayfs/overlayfs.h | 12 +++----
 fs/overlayfs/super.c     | 65 ++++++++++++++++++++++------------
 4 files changed, 74 insertions(+), 97 deletions(-)

-- 
2.7.4

[PATCH 1/4] ovl: Fix OVL_XATTR_PREFIX

[Andreas Gruenbacher]

Make sure ovl_own_xattr_handler only matches attribute names starting
with "overlay.", not "overlayXXX".

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/overlayfs/inode.c     | 5 ++---
 fs/overlayfs/overlayfs.h | 4 ++--
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 1b885c1..859e42c 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -193,9 +193,8 @@ static int ovl_readlink(struct dentry *dentry, char __user *buf, int bufsiz)
 
 static bool ovl_is_private_xattr(const char *name)
 {
-#define OVL_XATTR_PRE_NAME OVL_XATTR_PREFIX "."
-	return strncmp(name, OVL_XATTR_PRE_NAME,
-		       sizeof(OVL_XATTR_PRE_NAME) - 1) == 0;
+	return strncmp(name, OVL_XATTR_PREFIX,
+		       sizeof(OVL_XATTR_PREFIX) - 1) == 0;
 }
 
 int ovl_setxattr(struct dentry *dentry, struct inode *inode,
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index e4f5c95..77fd667 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -24,8 +24,8 @@ enum ovl_path_type {
 	(OVL_TYPE_MERGE(type) || !OVL_TYPE_UPPER(type))
 
 
-#define OVL_XATTR_PREFIX XATTR_TRUSTED_PREFIX "overlay"
-#define OVL_XATTR_OPAQUE OVL_XATTR_PREFIX ".opaque"
+#define OVL_XATTR_PREFIX XATTR_TRUSTED_PREFIX "overlay."
+#define OVL_XATTR_OPAQUE OVL_XATTR_PREFIX "opaque"
 
 #define OVL_ISUPPER_MASK 1UL
 
-- 
2.7.4

[PATCH 2/4] ovl: Get rid of ovl_xattr_noacl_handlers array

[Andreas Gruenbacher]

Use an ordinary #ifdef to conditionally include the POSIX ACL handlers
in ovl_xattr_handlers, like the other filesystems do.  Flag the code
that is now only used conditionally with __maybe_unused.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/overlayfs/super.c | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 4036132..e2b555d 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -967,10 +967,11 @@ static unsigned int ovl_split_lowerdirs(char *str)
 	return ctr;
 }
 
-static int ovl_posix_acl_xattr_set(const struct xattr_handler *handler,
-				   struct dentry *dentry, struct inode *inode,
-				   const char *name, const void *value,
-				   size_t size, int flags)
+static int __maybe_unused
+ovl_posix_acl_xattr_set(const struct xattr_handler *handler,
+			struct dentry *dentry, struct inode *inode,
+			const char *name, const void *value,
+			size_t size, int flags)
 {
 	struct dentry *workdir = ovl_workdir(dentry);
 	struct inode *realinode = ovl_inode_real(inode, NULL);
@@ -1021,13 +1022,15 @@ static int ovl_own_xattr_set(const struct xattr_handler *handler,
 	return -EPERM;
 }
 
-static const struct xattr_handler ovl_posix_acl_access_xattr_handler = {
+static const struct xattr_handler __maybe_unused
+ovl_posix_acl_access_xattr_handler = {
 	.name = XATTR_NAME_POSIX_ACL_ACCESS,
 	.flags = ACL_TYPE_ACCESS,
 	.set = ovl_posix_acl_xattr_set,
 };
 
-static const struct xattr_handler ovl_posix_acl_default_xattr_handler = {
+static const struct xattr_handler __maybe_unused
+ovl_posix_acl_default_xattr_handler = {
 	.name = XATTR_NAME_POSIX_ACL_DEFAULT,
 	.flags = ACL_TYPE_DEFAULT,
 	.set = ovl_posix_acl_xattr_set,
@@ -1044,19 +1047,15 @@ static const struct xattr_handler ovl_other_xattr_handler = {
 };
 
 static const struct xattr_handler *ovl_xattr_handlers[] = {
+#ifdef CONFIG_FS_POSIX_ACL
 	&ovl_posix_acl_access_xattr_handler,
 	&ovl_posix_acl_default_xattr_handler,
+#endif
 	&ovl_own_xattr_handler,
 	&ovl_other_xattr_handler,
 	NULL
 };
 
-static const struct xattr_handler *ovl_xattr_noacl_handlers[] = {
-	&ovl_own_xattr_handler,
-	&ovl_other_xattr_handler,
-	NULL,
-};
-
 static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 {
 	struct path upperpath = { NULL, NULL };
@@ -1269,10 +1268,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 
 	sb->s_magic = OVERLAYFS_SUPER_MAGIC;
 	sb->s_op = &ovl_super_operations;
-	if (IS_ENABLED(CONFIG_FS_POSIX_ACL))
-		sb->s_xattr = ovl_xattr_handlers;
-	else
-		sb->s_xattr = ovl_xattr_noacl_handlers;
+	sb->s_xattr = ovl_xattr_handlers;
 	sb->s_root = root_dentry;
 	sb->s_fs_info = ufs;
 	sb->s_flags |= MS_POSIXACL;
-- 
2.7.4

[PATCH 3/4] ovl: Switch to generic_removexattr

[Andreas Gruenbacher]

Commit d837a49b switches from iop->setxattr from ovl_setxattr to
generic_setxattr, so switch from ovl_removexattr to generic_removexattr
as well.  As far as permission checking goes, the same rules should
apply in either case.

While doing that, rename ovl_setxattr to ovl_xattr_set to indicate that
this is not an iop->setxattr implementation and remove the unused inode
argument.

Move ovl_other_xattr_set above ovl_own_xattr_set so that they match the
order of handlers in ovl_xattr_handlers.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/overlayfs/dir.c       |  2 +-
 fs/overlayfs/inode.c     | 65 ++++++++++++++++--------------------------------
 fs/overlayfs/overlayfs.h |  6 ++---
 fs/overlayfs/super.c     | 18 +++++++-------
 4 files changed, 33 insertions(+), 58 deletions(-)

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 12bcd07..7823480 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -952,7 +952,7 @@ const struct inode_operations ovl_dir_inode_operations = {
 	.setxattr	= generic_setxattr,
 	.getxattr	= ovl_getxattr,
 	.listxattr	= ovl_listxattr,
-	.removexattr	= ovl_removexattr,
+	.removexattr	= generic_removexattr,
 	.get_acl	= ovl_get_acl,
 	.update_time	= ovl_update_time,
 };
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 859e42c..4f348ca 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -197,25 +197,38 @@ static bool ovl_is_private_xattr(const char *name)
 		       sizeof(OVL_XATTR_PREFIX) - 1) == 0;
 }
 
-int ovl_setxattr(struct dentry *dentry, struct inode *inode,
-		 const char *name, const void *value,
-		 size_t size, int flags)
+int ovl_xattr_set(struct dentry *dentry, const char *name, const void *value,
+		  size_t size, int flags)
 {
 	int err;
-	struct dentry *upperdentry;
+	struct path realpath;
+	enum ovl_path_type type = ovl_path_real(dentry, &realpath);
 	const struct cred *old_cred;
 
 	err = ovl_want_write(dentry);
 	if (err)
 		goto out;
 
+	if (!value && !OVL_TYPE_UPPER(type)) {
+		err = vfs_getxattr(realpath.dentry, name, NULL, 0);
+		if (err < 0)
+			goto out_drop_write;
+	}
+
 	err = ovl_copy_up(dentry);
 	if (err)
 		goto out_drop_write;
 
-	upperdentry = ovl_dentry_upper(dentry);
+	if (!OVL_TYPE_UPPER(type))
+		ovl_path_upper(dentry, &realpath);
+
 	old_cred = ovl_override_creds(dentry->d_sb);
-	err = vfs_setxattr(upperdentry, name, value, size, flags);
+	if (value)
+		err = vfs_setxattr(realpath.dentry, name, value, size, flags);
+	else {
+		BUG_ON(flags != XATTR_REPLACE);
+		err = vfs_removexattr(realpath.dentry, name);
+	}
 	revert_creds(old_cred);
 
 out_drop_write:
@@ -271,42 +284,6 @@ ssize_t ovl_listxattr(struct dentry *dentry, char *list, size_t size)
 	return res;
 }
 
-int ovl_removexattr(struct dentry *dentry, const char *name)
-{
-	int err;
-	struct path realpath;
-	enum ovl_path_type type = ovl_path_real(dentry, &realpath);
-	const struct cred *old_cred;
-
-	err = ovl_want_write(dentry);
-	if (err)
-		goto out;
-
-	err = -ENODATA;
-	if (ovl_is_private_xattr(name))
-		goto out_drop_write;
-
-	if (!OVL_TYPE_UPPER(type)) {
-		err = vfs_getxattr(realpath.dentry, name, NULL, 0);
-		if (err < 0)
-			goto out_drop_write;
-
-		err = ovl_copy_up(dentry);
-		if (err)
-			goto out_drop_write;
-
-		ovl_path_upper(dentry, &realpath);
-	}
-
-	old_cred = ovl_override_creds(dentry->d_sb);
-	err = vfs_removexattr(realpath.dentry, name);
-	revert_creds(old_cred);
-out_drop_write:
-	ovl_drop_write(dentry);
-out:
-	return err;
-}
-
 struct posix_acl *ovl_get_acl(struct inode *inode, int type)
 {
 	struct inode *realinode = ovl_inode_real(inode, NULL);
@@ -392,7 +369,7 @@ static const struct inode_operations ovl_file_inode_operations = {
 	.setxattr	= generic_setxattr,
 	.getxattr	= ovl_getxattr,
 	.listxattr	= ovl_listxattr,
-	.removexattr	= ovl_removexattr,
+	.removexattr	= generic_removexattr,
 	.get_acl	= ovl_get_acl,
 	.update_time	= ovl_update_time,
 };
@@ -405,7 +382,7 @@ static const struct inode_operations ovl_symlink_inode_operations = {
 	.setxattr	= generic_setxattr,
 	.getxattr	= ovl_getxattr,
 	.listxattr	= ovl_listxattr,
-	.removexattr	= ovl_removexattr,
+	.removexattr	= generic_removexattr,
 	.update_time	= ovl_update_time,
 };
 
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 77fd667..951c8f5 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -183,13 +183,11 @@ int ovl_check_d_type_supported(struct path *realpath);
 /* inode.c */
 int ovl_setattr(struct dentry *dentry, struct iattr *attr);
 int ovl_permission(struct inode *inode, int mask);
-int ovl_setxattr(struct dentry *dentry, struct inode *inode,
-		 const char *name, const void *value,
-		 size_t size, int flags);
+int ovl_xattr_set(struct dentry *dentry, const char *name, const void *value,
+		  size_t size, int flags);
 ssize_t ovl_getxattr(struct dentry *dentry, struct inode *inode,
 		     const char *name, void *value, size_t size);
 ssize_t ovl_listxattr(struct dentry *dentry, char *list, size_t size);
-int ovl_removexattr(struct dentry *dentry, const char *name);
 struct posix_acl *ovl_get_acl(struct inode *inode, int type);
 int ovl_open_maybe_copy_up(struct dentry *dentry, unsigned int file_flags);
 int ovl_update_time(struct inode *inode, struct timespec *ts, int flags);
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index e2b555d..9eb3f82 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -999,21 +999,13 @@ ovl_posix_acl_xattr_set(const struct xattr_handler *handler,
 
 	posix_acl_release(acl);
 
-	return ovl_setxattr(dentry, inode, handler->name, value, size, flags);
+	return ovl_xattr_set(dentry, handler->name, value, size, flags);
 
 out_acl_release:
 	posix_acl_release(acl);
 	return err;
 }
 
-static int ovl_other_xattr_set(const struct xattr_handler *handler,
-			       struct dentry *dentry, struct inode *inode,
-			       const char *name, const void *value,
-			       size_t size, int flags)
-{
-	return ovl_setxattr(dentry, inode, name, value, size, flags);
-}
-
 static int ovl_own_xattr_set(const struct xattr_handler *handler,
 			     struct dentry *dentry, struct inode *inode,
 			     const char *name, const void *value,
@@ -1022,6 +1014,14 @@ static int ovl_own_xattr_set(const struct xattr_handler *handler,
 	return -EPERM;
 }
 
+static int ovl_other_xattr_set(const struct xattr_handler *handler,
+			       struct dentry *dentry, struct inode *inode,
+			       const char *name, const void *value,
+			       size_t size, int flags)
+{
+	return ovl_xattr_set(dentry, name, value, size, flags);
+}
+
 static const struct xattr_handler __maybe_unused
 ovl_posix_acl_access_xattr_handler = {
 	.name = XATTR_NAME_POSIX_ACL_ACCESS,
-- 
2.7.4

[PATCH 4/4] ovl: Switch to generic_getxattr

[Andreas Gruenbacher]

Now that overlayfs has xattr handlers for iop->{set,remove}xattr, use
those same handlers for iop->getxattr as well.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/overlayfs/dir.c       |  2 +-
 fs/overlayfs/inode.c     | 20 ++------------------
 fs/overlayfs/overlayfs.h |  2 --
 fs/overlayfs/super.c     | 25 +++++++++++++++++++++++++
 4 files changed, 28 insertions(+), 21 deletions(-)

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 7823480..a5ade8a 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -950,7 +950,7 @@ const struct inode_operations ovl_dir_inode_operations = {
 	.permission	= ovl_permission,
 	.getattr	= ovl_dir_getattr,
 	.setxattr	= generic_setxattr,
-	.getxattr	= ovl_getxattr,
+	.getxattr	= generic_getxattr,
 	.listxattr	= ovl_listxattr,
 	.removexattr	= generic_removexattr,
 	.get_acl	= ovl_get_acl,
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 4f348ca..a699ff9 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -237,22 +237,6 @@ out:
 	return err;
 }
 
-ssize_t ovl_getxattr(struct dentry *dentry, struct inode *inode,
-		     const char *name, void *value, size_t size)
-{
-	struct dentry *realdentry = ovl_dentry_real(dentry);
-	ssize_t res;
-	const struct cred *old_cred;
-
-	if (ovl_is_private_xattr(name))
-		return -ENODATA;
-
-	old_cred = ovl_override_creds(dentry->d_sb);
-	res = vfs_getxattr(realdentry, name, value, size);
-	revert_creds(old_cred);
-	return res;
-}
-
 ssize_t ovl_listxattr(struct dentry *dentry, char *list, size_t size)
 {
 	struct dentry *realdentry = ovl_dentry_real(dentry);
@@ -367,7 +351,7 @@ static const struct inode_operations ovl_file_inode_operations = {
 	.permission	= ovl_permission,
 	.getattr	= ovl_getattr,
 	.setxattr	= generic_setxattr,
-	.getxattr	= ovl_getxattr,
+	.getxattr	= generic_getxattr,
 	.listxattr	= ovl_listxattr,
 	.removexattr	= generic_removexattr,
 	.get_acl	= ovl_get_acl,
@@ -380,7 +364,7 @@ static const struct inode_operations ovl_symlink_inode_operations = {
 	.readlink	= ovl_readlink,
 	.getattr	= ovl_getattr,
 	.setxattr	= generic_setxattr,
-	.getxattr	= ovl_getxattr,
+	.getxattr	= generic_getxattr,
 	.listxattr	= ovl_listxattr,
 	.removexattr	= generic_removexattr,
 	.update_time	= ovl_update_time,
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 951c8f5..5df6fd1 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -185,8 +185,6 @@ int ovl_setattr(struct dentry *dentry, struct iattr *attr);
 int ovl_permission(struct inode *inode, int mask);
 int ovl_xattr_set(struct dentry *dentry, const char *name, const void *value,
 		  size_t size, int flags);
-ssize_t ovl_getxattr(struct dentry *dentry, struct inode *inode,
-		     const char *name, void *value, size_t size);
 ssize_t ovl_listxattr(struct dentry *dentry, char *list, size_t size);
 struct posix_acl *ovl_get_acl(struct inode *inode, int type);
 int ovl_open_maybe_copy_up(struct dentry *dentry, unsigned int file_flags);
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 9eb3f82..762f3ad 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -967,6 +967,20 @@ static unsigned int ovl_split_lowerdirs(char *str)
 	return ctr;
 }
 
+static int ovl_xattr_get(const struct xattr_handler *handler,
+			 struct dentry *dentry, struct inode *inode,
+			 const char *name, void *buffer, size_t size)
+{
+	struct dentry *realdentry = ovl_dentry_real(dentry);
+	const struct cred *old_cred;
+	ssize_t err;
+
+	old_cred = ovl_override_creds(dentry->d_sb);
+	err = vfs_getxattr(realdentry, name, buffer, size);
+	revert_creds(old_cred);
+	return err;
+}
+
 static int __maybe_unused
 ovl_posix_acl_xattr_set(const struct xattr_handler *handler,
 			struct dentry *dentry, struct inode *inode,
@@ -1006,6 +1020,13 @@ out_acl_release:
 	return err;
 }
 
+static int ovl_own_xattr_get(const struct xattr_handler *handler,
+			     struct dentry *dentry, struct inode *inode,
+			     const char *name, void *buffer, size_t size)
+{
+	return -EPERM;
+}
+
 static int ovl_own_xattr_set(const struct xattr_handler *handler,
 			     struct dentry *dentry, struct inode *inode,
 			     const char *name, const void *value,
@@ -1026,6 +1047,7 @@ static const struct xattr_handler __maybe_unused
 ovl_posix_acl_access_xattr_handler = {
 	.name = XATTR_NAME_POSIX_ACL_ACCESS,
 	.flags = ACL_TYPE_ACCESS,
+	.get = ovl_xattr_get,
 	.set = ovl_posix_acl_xattr_set,
 };
 
@@ -1033,16 +1055,19 @@ static const struct xattr_handler __maybe_unused
 ovl_posix_acl_default_xattr_handler = {
 	.name = XATTR_NAME_POSIX_ACL_DEFAULT,
 	.flags = ACL_TYPE_DEFAULT,
+	.get = ovl_xattr_get,
 	.set = ovl_posix_acl_xattr_set,
 };
 
 static const struct xattr_handler ovl_own_xattr_handler = {
 	.prefix	= OVL_XATTR_PREFIX,
+	.get = ovl_own_xattr_get,
 	.set = ovl_own_xattr_set,
 };
 
 static const struct xattr_handler ovl_other_xattr_handler = {
 	.prefix	= "", /* catch all */
+	.get = ovl_xattr_get,
 	.set = ovl_other_xattr_set,
 };
 
-- 
2.7.4

Re: [PATCH 3/4] ovl: Switch to generic_removexattr

[Miklos Szeredi]

On Mon, Aug 22, 2016 at 8:06 PM, Andreas Gruenbacher
<agruenba@redhat.com> wrote:
> Commit d837a49b switches from iop->setxattr from ovl_setxattr to
> generic_setxattr, so switch from ovl_removexattr to generic_removexattr
> as well.  As far as permission checking goes, the same rules should
> apply in either case.
>
> While doing that, rename ovl_setxattr to ovl_xattr_set to indicate that
> this is not an iop->setxattr implementation and remove the unused inode
> argument.
>
> Move ovl_other_xattr_set above ovl_own_xattr_set so that they match the
> order of handlers in ovl_xattr_handlers.
>
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> ---
>  fs/overlayfs/dir.c       |  2 +-
>  fs/overlayfs/inode.c     | 65 ++++++++++++++++--------------------------------
>  fs/overlayfs/overlayfs.h |  6 ++---
>  fs/overlayfs/super.c     | 18 +++++++-------
>  4 files changed, 33 insertions(+), 58 deletions(-)
>
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index 12bcd07..7823480 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -952,7 +952,7 @@ const struct inode_operations ovl_dir_inode_operations = {
>         .setxattr       = generic_setxattr,
>         .getxattr       = ovl_getxattr,
>         .listxattr      = ovl_listxattr,
> -       .removexattr    = ovl_removexattr,
> +       .removexattr    = generic_removexattr,
>         .get_acl        = ovl_get_acl,
>         .update_time    = ovl_update_time,
>  };
> diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
> index 859e42c..4f348ca 100644
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -197,25 +197,38 @@ static bool ovl_is_private_xattr(const char *name)
>                        sizeof(OVL_XATTR_PREFIX) - 1) == 0;
>  }
>
> -int ovl_setxattr(struct dentry *dentry, struct inode *inode,
> -                const char *name, const void *value,
> -                size_t size, int flags)
> +int ovl_xattr_set(struct dentry *dentry, const char *name, const void *value,
> +                 size_t size, int flags)
>  {
>         int err;
> -       struct dentry *upperdentry;
> +       struct path realpath;
> +       enum ovl_path_type type = ovl_path_real(dentry, &realpath);
>         const struct cred *old_cred;
>
>         err = ovl_want_write(dentry);
>         if (err)
>                 goto out;
>
> +       if (!value && !OVL_TYPE_UPPER(type)) {
> +               err = vfs_getxattr(realpath.dentry, name, NULL, 0);
> +               if (err < 0)
> +                       goto out_drop_write;
> +       }
> +
>         err = ovl_copy_up(dentry);
>         if (err)
>                 goto out_drop_write;
>
> -       upperdentry = ovl_dentry_upper(dentry);
> +       if (!OVL_TYPE_UPPER(type))
> +               ovl_path_upper(dentry, &realpath);
> +
>         old_cred = ovl_override_creds(dentry->d_sb);
> -       err = vfs_setxattr(upperdentry, name, value, size, flags);
> +       if (value)
> +               err = vfs_setxattr(realpath.dentry, name, value, size, flags);
> +       else {
> +               BUG_ON(flags != XATTR_REPLACE);
> +               err = vfs_removexattr(realpath.dentry, name);
> +       }

Hmm, setxattr(8) says that a zero length value is permitted and
indeed, setxatt() in fs/xattr.c handles that case, but passes a NULL
value.   At that point it becomes impossible to differentiate the two
cases (and the BUG_ON could trigger too, AFAICS).

Am I missing something?

Thanks,
Miklos