On Mon, Aug 22, 2016 at 8:06 PM, Andreas Gruenbacher
<agruenba@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
