| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAOQ4uxj=oR+yj19rUm0E6cHTiStniqvebtZSDhV3XZC1qz6n6A@mail.gmail.com>
Date: Thu, 30 Nov 2023 08:23:28 +0200
From: Amir Goldstein <amir73il@...il.com>
To: "Seth Forshee (DigitalOcean)" <sforshee@...nel.org>
Cc: Christian Brauner <brauner@...nel.org>,
Serge Hallyn <serge@...lyn.com>,
Paul Moore <paul@...l-moore.com>,
Eric Paris <eparis@...hat.com>,
James Morris <jmorris@...ei.org>,
Alexander Viro <viro@...iv.linux.org.uk>,
Miklos Szeredi <miklos@...redi.hu>,
linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
linux-security-module@...r.kernel.org, audit@...r.kernel.org,
linux-unionfs@...r.kernel.org
Subject: Re: [PATCH 12/16] ovl: use vfs_{get,set}_fscaps() for copy-up
On Wed, Nov 29, 2023 at 11:50 PM Seth Forshee (DigitalOcean)
<sforshee@...nel.org> wrote:
>
> Using vfs_{get,set}xattr() for fscaps will be blocked in a future
> commit, so convert ovl to use the new interfaces. Also remove the now
> unused ovl_getxattr_value().
>
> Signed-off-by: Seth Forshee (DigitalOcean) <sforshee@...nel.org>
You may add:
Reviewed-by: Amir Goldstein <amir73il@...il.com>
I am assuming that this work is destined to be merged via the vfs tree?
Note that there is already a (non-conflicting) patch to copy_up.c on
Christian's vfs.rw branch.
I think it is best that all the overlayfs patches are tested together by
the vfs maintainer in preparation for the 6.8 merge window, so I have
a feeling that the 6.8 overlayfs PR is going to be merged via the vfs tree ;-)
Thanks,
Amir.
> ---
> fs/overlayfs/copy_up.c | 72 ++++++++++++++++++++++++++------------------------
> 1 file changed, 37 insertions(+), 35 deletions(-)
>
> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> index 4382881b0709..b43af5ce4b21 100644
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -73,6 +73,23 @@ static int ovl_copy_acl(struct ovl_fs *ofs, const struct path *path,
> return err;
> }
>
> +static int ovl_copy_fscaps(struct ovl_fs *ofs, const struct path *oldpath,
> + struct dentry *new)
> +{
> + struct vfs_caps capability;
> + int err;
> +
> + err = vfs_get_fscaps(mnt_idmap(oldpath->mnt), oldpath->dentry,
> + &capability);
> + if (err) {
> + if (err == -ENODATA || err == -EOPNOTSUPP)
> + return 0;
> + return err;
> + }
> +
> + return vfs_set_fscaps(ovl_upper_mnt_idmap(ofs), new, &capability, 0);
> +}
> +
> int ovl_copy_xattr(struct super_block *sb, const struct path *oldpath, struct dentry *new)
> {
> struct dentry *old = oldpath->dentry;
> @@ -130,6 +147,14 @@ int ovl_copy_xattr(struct super_block *sb, const struct path *oldpath, struct de
> break;
> }
>
> + if (!strcmp(name, XATTR_NAME_CAPS)) {
> + error = ovl_copy_fscaps(OVL_FS(sb), oldpath, new);
> + if (!error)
> + continue;
> + /* fs capabilities must be copied */
> + break;
> + }
> +
> retry:
> size = ovl_do_getxattr(oldpath, name, value, value_size);
> if (size == -ERANGE)
> @@ -1006,61 +1031,40 @@ static bool ovl_need_meta_copy_up(struct dentry *dentry, umode_t mode,
> return true;
> }
>
> -static ssize_t ovl_getxattr_value(const struct path *path, char *name, char **value)
> -{
> - ssize_t res;
> - char *buf;
> -
> - res = ovl_do_getxattr(path, name, NULL, 0);
> - if (res == -ENODATA || res == -EOPNOTSUPP)
> - res = 0;
> -
> - if (res > 0) {
> - buf = kzalloc(res, GFP_KERNEL);
> - if (!buf)
> - return -ENOMEM;
> -
> - res = ovl_do_getxattr(path, name, buf, res);
> - if (res < 0)
> - kfree(buf);
> - else
> - *value = buf;
> - }
> - return res;
> -}
> -
> /* Copy up data of an inode which was copied up metadata only in the past. */
> static int ovl_copy_up_meta_inode_data(struct ovl_copy_up_ctx *c)
> {
> struct ovl_fs *ofs = OVL_FS(c->dentry->d_sb);
> struct path upperpath;
> int err;
> - char *capability = NULL;
> - ssize_t cap_size;
> + struct vfs_caps capability;
> + bool has_capability = false;
>
> ovl_path_upper(c->dentry, &upperpath);
> if (WARN_ON(upperpath.dentry == NULL))
> return -EIO;
>
> if (c->stat.size) {
> - err = cap_size = ovl_getxattr_value(&upperpath, XATTR_NAME_CAPS,
> - &capability);
> - if (cap_size < 0)
> + err = vfs_get_fscaps(mnt_idmap(upperpath.mnt), upperpath.dentry,
> + &capability);
> + if (!err)
> + has_capability = 1;
> + else if (err != -ENODATA && err != EOPNOTSUPP)
> goto out;
> }
>
> err = ovl_copy_up_data(c, &upperpath);
> if (err)
> - goto out_free;
> + goto out;
>
> /*
> * Writing to upper file will clear security.capability xattr. We
> * don't want that to happen for normal copy-up operation.
> */
> ovl_start_write(c->dentry);
> - if (capability) {
> - err = ovl_do_setxattr(ofs, upperpath.dentry, XATTR_NAME_CAPS,
> - capability, cap_size, 0);
> + if (has_capability) {
> + err = vfs_set_fscaps(mnt_idmap(upperpath.mnt), upperpath.dentry,
> + &capability, 0);
> }
> if (!err) {
> err = ovl_removexattr(ofs, upperpath.dentry,
> @@ -1068,13 +1072,11 @@ static int ovl_copy_up_meta_inode_data(struct ovl_copy_up_ctx *c)
> }
> ovl_end_write(c->dentry);
> if (err)
> - goto out_free;
> + goto out;
>
> ovl_clear_flag(OVL_HAS_DIGEST, d_inode(c->dentry));
> ovl_clear_flag(OVL_VERIFIED_DIGEST, d_inode(c->dentry));
> ovl_set_upperdata(d_inode(c->dentry));
> -out_free:
> - kfree(capability);
> out:
> return err;
> }
>
> --
> 2.43.0
>
Powered by blists - more mailing lists