Skip to content

Commit

Permalink
ovl: use wrappers to all vfs_*xattr() calls
Browse files Browse the repository at this point in the history
Use helpers ovl_*xattr() to access user/trusted.overlay.* xattrs
and use helpers ovl_do_*xattr() to access generic xattrs. This is a
preparatory patch for using idmapped base layers with overlay.

Note that a few of those places called vfs_*xattr() calls directly to
reduce the amount of debug output. But as Miklos pointed out since
overlayfs has been stable for quite some time the debug output isn't all
that relevant anymore and the additional debug in all locations was
actually quite helpful when developing this patch series.

Cc: <[email protected]>
Tested-by: Giuseppe Scrivano <[email protected]>
Reviewed-by: Christian Brauner (Microsoft) <[email protected]>
Signed-off-by: Amir Goldstein <[email protected]>
Signed-off-by: Christian Brauner (Microsoft) <[email protected]>
Signed-off-by: Miklos Szeredi <[email protected]>
  • Loading branch information
amir73il authored and Miklos Szeredi committed Apr 28, 2022
1 parent 3a761d7 commit c914c0e
Show file tree
Hide file tree
Showing 8 changed files with 75 additions and 54 deletions.
21 changes: 11 additions & 10 deletions fs/overlayfs/copy_up.c
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ int ovl_copy_xattr(struct super_block *sb, struct dentry *old,
goto retry;
}

error = vfs_setxattr(&init_user_ns, new, name, value, size, 0);
error = ovl_do_setxattr(OVL_FS(sb), new, name, value, size, 0);
if (error) {
if (error != -EOPNOTSUPP || ovl_must_copy_xattr(name))
break;
Expand Down Expand Up @@ -433,7 +433,7 @@ static int ovl_set_upper_fh(struct ovl_fs *ofs, struct dentry *upper,
if (IS_ERR(fh))
return PTR_ERR(fh);

err = ovl_do_setxattr(ofs, index, OVL_XATTR_UPPER, fh->buf, fh->fb.len);
err = ovl_setxattr(ofs, index, OVL_XATTR_UPPER, fh->buf, fh->fb.len);

kfree(fh);
return err;
Expand Down Expand Up @@ -865,12 +865,13 @@ static bool ovl_need_meta_copy_up(struct dentry *dentry, umode_t mode,
return true;
}

static ssize_t ovl_getxattr(struct dentry *dentry, char *name, char **value)
static ssize_t ovl_getxattr_value(struct ovl_fs *ofs, struct dentry *dentry,
char *name, char **value)
{
ssize_t res;
char *buf;

res = vfs_getxattr(&init_user_ns, dentry, name, NULL, 0);
res = ovl_do_getxattr(ofs, dentry, name, NULL, 0);
if (res == -ENODATA || res == -EOPNOTSUPP)
res = 0;

Expand All @@ -879,7 +880,7 @@ static ssize_t ovl_getxattr(struct dentry *dentry, char *name, char **value)
if (!buf)
return -ENOMEM;

res = vfs_getxattr(&init_user_ns, dentry, name, buf, res);
res = ovl_do_getxattr(ofs, dentry, name, buf, res);
if (res < 0)
kfree(buf);
else
Expand All @@ -906,8 +907,8 @@ static int ovl_copy_up_meta_inode_data(struct ovl_copy_up_ctx *c)
return -EIO;

if (c->stat.size) {
err = cap_size = ovl_getxattr(upperpath.dentry, XATTR_NAME_CAPS,
&capability);
err = cap_size = ovl_getxattr_value(ofs, upperpath.dentry,
XATTR_NAME_CAPS, &capability);
if (cap_size < 0)
goto out;
}
Expand All @@ -921,14 +922,14 @@ static int ovl_copy_up_meta_inode_data(struct ovl_copy_up_ctx *c)
* don't want that to happen for normal copy-up operation.
*/
if (capability) {
err = vfs_setxattr(&init_user_ns, upperpath.dentry,
XATTR_NAME_CAPS, capability, cap_size, 0);
err = ovl_do_setxattr(ofs, upperpath.dentry, XATTR_NAME_CAPS,
capability, cap_size, 0);
if (err)
goto out_free;
}


err = ovl_do_removexattr(ofs, upperpath.dentry, OVL_XATTR_METACOPY);
err = ovl_removexattr(ofs, upperpath.dentry, OVL_XATTR_METACOPY);
if (err)
goto out_free;

Expand Down
15 changes: 8 additions & 7 deletions fs/overlayfs/dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -432,8 +432,8 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry,
return ERR_PTR(err);
}

static int ovl_set_upper_acl(struct dentry *upperdentry, const char *name,
const struct posix_acl *acl)
static int ovl_set_upper_acl(struct ovl_fs *ofs, struct dentry *upperdentry,
const char *name, const struct posix_acl *acl)
{
void *buffer;
size_t size;
Expand All @@ -451,7 +451,7 @@ static int ovl_set_upper_acl(struct dentry *upperdentry, const char *name,
if (err < 0)
goto out_free;

err = vfs_setxattr(&init_user_ns, upperdentry, name, buffer, size, XATTR_CREATE);
err = ovl_do_setxattr(ofs, upperdentry, name, buffer, size, XATTR_CREATE);
out_free:
kfree(buffer);
return err;
Expand All @@ -460,6 +460,7 @@ static int ovl_set_upper_acl(struct dentry *upperdentry, const char *name,
static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode,
struct ovl_cattr *cattr)
{
struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
struct dentry *workdir = ovl_workdir(dentry);
struct inode *wdir = workdir->d_inode;
struct dentry *upperdir = ovl_dentry_upper(dentry->d_parent);
Expand Down Expand Up @@ -516,13 +517,13 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode,
goto out_cleanup;
}
if (!hardlink) {
err = ovl_set_upper_acl(newdentry, XATTR_NAME_POSIX_ACL_ACCESS,
acl);
err = ovl_set_upper_acl(ofs, newdentry,
XATTR_NAME_POSIX_ACL_ACCESS, acl);
if (err)
goto out_cleanup;

err = ovl_set_upper_acl(newdentry, XATTR_NAME_POSIX_ACL_DEFAULT,
default_acl);
err = ovl_set_upper_acl(ofs, newdentry,
XATTR_NAME_POSIX_ACL_DEFAULT, default_acl);
if (err)
goto out_cleanup;
}
Expand Down
17 changes: 9 additions & 8 deletions fs/overlayfs/inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,7 @@ int ovl_xattr_set(struct dentry *dentry, struct inode *inode, const char *name,
const void *value, size_t size, int flags)
{
int err;
struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
struct dentry *upperdentry = ovl_i_dentry_upper(inode);
struct dentry *realdentry = upperdentry ?: ovl_dentry_lower(dentry);
const struct cred *old_cred;
Expand All @@ -367,12 +368,12 @@ int ovl_xattr_set(struct dentry *dentry, struct inode *inode, const char *name,
}

old_cred = ovl_override_creds(dentry->d_sb);
if (value)
err = vfs_setxattr(&init_user_ns, realdentry, name, value, size,
flags);
else {
if (value) {
err = ovl_do_setxattr(ofs, realdentry, name, value, size,
flags);
} else {
WARN_ON(flags != XATTR_REPLACE);
err = vfs_removexattr(&init_user_ns, realdentry, name);
err = ovl_do_removexattr(ofs, realdentry, name);
}
revert_creds(old_cred);

Expand Down Expand Up @@ -871,8 +872,8 @@ static int ovl_set_nlink_common(struct dentry *dentry,
if (WARN_ON(len >= sizeof(buf)))
return -EIO;

return ovl_do_setxattr(OVL_FS(inode->i_sb), ovl_dentry_upper(dentry),
OVL_XATTR_NLINK, buf, len);
return ovl_setxattr(OVL_FS(inode->i_sb), ovl_dentry_upper(dentry),
OVL_XATTR_NLINK, buf, len);
}

int ovl_set_nlink_upper(struct dentry *dentry)
Expand All @@ -897,7 +898,7 @@ unsigned int ovl_get_nlink(struct ovl_fs *ofs, struct dentry *lowerdentry,
if (!lowerdentry || !upperdentry || d_inode(lowerdentry)->i_nlink == 1)
return fallback;

err = ovl_do_getxattr(ofs, upperdentry, OVL_XATTR_NLINK,
err = ovl_getxattr(ofs, upperdentry, OVL_XATTR_NLINK,
&buf, sizeof(buf) - 1);
if (err < 0)
goto fail;
Expand Down
6 changes: 3 additions & 3 deletions fs/overlayfs/namei.c
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ static struct ovl_fh *ovl_get_fh(struct ovl_fs *ofs, struct dentry *dentry,
int res, err;
struct ovl_fh *fh = NULL;

res = ovl_do_getxattr(ofs, dentry, ox, NULL, 0);
res = ovl_getxattr(ofs, dentry, ox, NULL, 0);
if (res < 0) {
if (res == -ENODATA || res == -EOPNOTSUPP)
return NULL;
Expand All @@ -125,7 +125,7 @@ static struct ovl_fh *ovl_get_fh(struct ovl_fs *ofs, struct dentry *dentry,
if (!fh)
return ERR_PTR(-ENOMEM);

res = ovl_do_getxattr(ofs, dentry, ox, fh->buf, res);
res = ovl_getxattr(ofs, dentry, ox, fh->buf, res);
if (res < 0)
goto fail;

Expand Down Expand Up @@ -464,7 +464,7 @@ int ovl_verify_set_fh(struct ovl_fs *ofs, struct dentry *dentry,

err = ovl_verify_fh(ofs, dentry, ox, fh);
if (set && err == -ENODATA)
err = ovl_do_setxattr(ofs, dentry, ox, fh->buf, fh->fb.len);
err = ovl_setxattr(ofs, dentry, ox, fh->buf, fh->fb.len);
if (err)
goto fail;

Expand Down
38 changes: 28 additions & 10 deletions fs/overlayfs/overlayfs.h
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,9 @@ static inline int ovl_do_symlink(struct inode *dir, struct dentry *dentry,
}

static inline ssize_t ovl_do_getxattr(struct ovl_fs *ofs, struct dentry *dentry,
enum ovl_xattr ox, void *value,
const char *name, void *value,
size_t size)
{
const char *name = ovl_xattr(ofs, ox);
int err = vfs_getxattr(&init_user_ns, dentry, name, value, size);
int len = (value && err > 0) ? err : 0;

Expand All @@ -195,26 +194,45 @@ static inline ssize_t ovl_do_getxattr(struct ovl_fs *ofs, struct dentry *dentry,
return err;
}

static inline ssize_t ovl_getxattr(struct ovl_fs *ofs, struct dentry *dentry,
enum ovl_xattr ox, void *value,
size_t size)
{
return ovl_do_getxattr(ofs, dentry, ovl_xattr(ofs, ox), value, size);
}

static inline int ovl_do_setxattr(struct ovl_fs *ofs, struct dentry *dentry,
enum ovl_xattr ox, const void *value,
size_t size)
const char *name, const void *value,
size_t size, int flags)
{
const char *name = ovl_xattr(ofs, ox);
int err = vfs_setxattr(&init_user_ns, dentry, name, value, size, 0);
pr_debug("setxattr(%pd2, \"%s\", \"%*pE\", %zu, 0) = %i\n",
dentry, name, min((int)size, 48), value, size, err);
int err = vfs_setxattr(&init_user_ns, dentry, name, value, size, flags);

pr_debug("setxattr(%pd2, \"%s\", \"%*pE\", %zu, %d) = %i\n",
dentry, name, min((int)size, 48), value, size, flags, err);
return err;
}

static inline int ovl_setxattr(struct ovl_fs *ofs, struct dentry *dentry,
enum ovl_xattr ox, const void *value,
size_t size)
{
return ovl_do_setxattr(ofs, dentry, ovl_xattr(ofs, ox), value, size, 0);
}

static inline int ovl_do_removexattr(struct ovl_fs *ofs, struct dentry *dentry,
enum ovl_xattr ox)
const char *name)
{
const char *name = ovl_xattr(ofs, ox);
int err = vfs_removexattr(&init_user_ns, dentry, name);
pr_debug("removexattr(%pd2, \"%s\") = %i\n", dentry, name, err);
return err;
}

static inline int ovl_removexattr(struct ovl_fs *ofs, struct dentry *dentry,
enum ovl_xattr ox)
{
return ovl_do_removexattr(ofs, dentry, ovl_xattr(ofs, ox));
}

static inline int ovl_do_rename(struct inode *olddir, struct dentry *olddentry,
struct inode *newdir, struct dentry *newdentry,
unsigned int flags)
Expand Down
4 changes: 2 additions & 2 deletions fs/overlayfs/readdir.c
Original file line number Diff line number Diff line change
Expand Up @@ -623,8 +623,8 @@ static struct ovl_dir_cache *ovl_cache_get_impure(struct path *path)
* Removing the "impure" xattr is best effort.
*/
if (!ovl_want_write(dentry)) {
ovl_do_removexattr(ofs, ovl_dentry_upper(dentry),
OVL_XATTR_IMPURE);
ovl_removexattr(ofs, ovl_dentry_upper(dentry),
OVL_XATTR_IMPURE);
ovl_drop_write(dentry);
}
ovl_clear_flag(OVL_IMPURE, d_inode(dentry));
Expand Down
12 changes: 6 additions & 6 deletions fs/overlayfs/super.c
Original file line number Diff line number Diff line change
Expand Up @@ -809,13 +809,13 @@ static struct dentry *ovl_workdir_create(struct ovl_fs *ofs,
* allowed as upper are limited to "normal" ones, where checking
* for the above two errors is sufficient.
*/
err = vfs_removexattr(&init_user_ns, work,
XATTR_NAME_POSIX_ACL_DEFAULT);
err = ovl_do_removexattr(ofs, work,
XATTR_NAME_POSIX_ACL_DEFAULT);
if (err && err != -ENODATA && err != -EOPNOTSUPP)
goto out_dput;

err = vfs_removexattr(&init_user_ns, work,
XATTR_NAME_POSIX_ACL_ACCESS);
err = ovl_do_removexattr(ofs, work,
XATTR_NAME_POSIX_ACL_ACCESS);
if (err && err != -ENODATA && err != -EOPNOTSUPP)
goto out_dput;

Expand Down Expand Up @@ -1411,7 +1411,7 @@ static int ovl_make_workdir(struct super_block *sb, struct ovl_fs *ofs,
/*
* Check if upper/work fs supports (trusted|user).overlay.* xattr
*/
err = ovl_do_setxattr(ofs, ofs->workdir, OVL_XATTR_OPAQUE, "0", 1);
err = ovl_setxattr(ofs, ofs->workdir, OVL_XATTR_OPAQUE, "0", 1);
if (err) {
ofs->noxattr = true;
if (ofs->config.index || ofs->config.metacopy) {
Expand All @@ -1429,7 +1429,7 @@ static int ovl_make_workdir(struct super_block *sb, struct ovl_fs *ofs,
}
err = 0;
} else {
ovl_do_removexattr(ofs, ofs->workdir, OVL_XATTR_OPAQUE);
ovl_removexattr(ofs, ofs->workdir, OVL_XATTR_OPAQUE);
}

/*
Expand Down
16 changes: 8 additions & 8 deletions fs/overlayfs/util.c
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,7 @@ bool ovl_check_origin_xattr(struct ovl_fs *ofs, struct dentry *dentry)
{
int res;

res = ovl_do_getxattr(ofs, dentry, OVL_XATTR_ORIGIN, NULL, 0);
res = ovl_getxattr(ofs, dentry, OVL_XATTR_ORIGIN, NULL, 0);

/* Zero size value means "copied up but origin unknown" */
if (res >= 0)
Expand All @@ -572,7 +572,7 @@ bool ovl_check_dir_xattr(struct super_block *sb, struct dentry *dentry,
if (!d_is_dir(dentry))
return false;

res = ovl_do_getxattr(OVL_FS(sb), dentry, ox, &val, 1);
res = ovl_getxattr(OVL_FS(sb), dentry, ox, &val, 1);
if (res == 1 && val == 'y')
return true;

Expand Down Expand Up @@ -612,7 +612,7 @@ int ovl_check_setxattr(struct ovl_fs *ofs, struct dentry *upperdentry,
if (ofs->noxattr)
return xerr;

err = ovl_do_setxattr(ofs, upperdentry, ox, value, size);
err = ovl_setxattr(ofs, upperdentry, ox, value, size);

if (err == -EOPNOTSUPP) {
pr_warn("cannot set %s xattr on upper\n", ovl_xattr(ofs, ox));
Expand Down Expand Up @@ -652,7 +652,7 @@ void ovl_check_protattr(struct inode *inode, struct dentry *upper)
char buf[OVL_PROTATTR_MAX+1];
int res, n;

res = ovl_do_getxattr(ofs, upper, OVL_XATTR_PROTATTR, buf,
res = ovl_getxattr(ofs, upper, OVL_XATTR_PROTATTR, buf,
OVL_PROTATTR_MAX);
if (res < 0)
return;
Expand Down Expand Up @@ -708,7 +708,7 @@ int ovl_set_protattr(struct inode *inode, struct dentry *upper,
err = ovl_check_setxattr(ofs, upper, OVL_XATTR_PROTATTR,
buf, len, -EPERM);
} else if (inode->i_flags & OVL_PROT_I_FLAGS_MASK) {
err = ovl_do_removexattr(ofs, upper, OVL_XATTR_PROTATTR);
err = ovl_removexattr(ofs, upper, OVL_XATTR_PROTATTR);
if (err == -EOPNOTSUPP || err == -ENODATA)
err = 0;
}
Expand Down Expand Up @@ -951,7 +951,7 @@ int ovl_check_metacopy_xattr(struct ovl_fs *ofs, struct dentry *dentry)
if (!S_ISREG(d_inode(dentry)->i_mode))
return 0;

res = ovl_do_getxattr(ofs, dentry, OVL_XATTR_METACOPY, NULL, 0);
res = ovl_getxattr(ofs, dentry, OVL_XATTR_METACOPY, NULL, 0);
if (res < 0) {
if (res == -ENODATA || res == -EOPNOTSUPP)
return 0;
Expand Down Expand Up @@ -993,7 +993,7 @@ char *ovl_get_redirect_xattr(struct ovl_fs *ofs, struct dentry *dentry,
int res;
char *s, *next, *buf = NULL;

res = ovl_do_getxattr(ofs, dentry, OVL_XATTR_REDIRECT, NULL, 0);
res = ovl_getxattr(ofs, dentry, OVL_XATTR_REDIRECT, NULL, 0);
if (res == -ENODATA || res == -EOPNOTSUPP)
return NULL;
if (res < 0)
Expand All @@ -1005,7 +1005,7 @@ char *ovl_get_redirect_xattr(struct ovl_fs *ofs, struct dentry *dentry,
if (!buf)
return ERR_PTR(-ENOMEM);

res = ovl_do_getxattr(ofs, dentry, OVL_XATTR_REDIRECT, buf, res);
res = ovl_getxattr(ofs, dentry, OVL_XATTR_REDIRECT, buf, res);
if (res < 0)
goto fail;
if (res == 0)
Expand Down

0 comments on commit c914c0e

Please sign in to comment.