| 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
| ||
|
Date: Tue, 27 Feb 2024 15:28:18 +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>, Jan Kara <jack@...e.cz>,
Stephen Smalley <stephen.smalley.work@...il.com>, Ondrej Mosnacek <omosnace@...hat.com>,
Casey Schaufler <casey@...aufler-ca.com>, Mimi Zohar <zohar@...ux.ibm.com>,
Roberto Sassu <roberto.sassu@...wei.com>, Dmitry Kasatkin <dmitry.kasatkin@...il.com>,
Eric Snowberg <eric.snowberg@...cle.com>, "Matthew Wilcox (Oracle)" <willy@...radead.org>,
Jonathan Corbet <corbet@....net>, 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, selinux@...r.kernel.org,
linux-integrity@...r.kernel.org, linux-doc@...r.kernel.org,
linux-unionfs@...r.kernel.org
Subject: Re: [PATCH v2 20/25] ovl: add fscaps handlers
On Wed, Feb 21, 2024 at 11:25 PM Seth Forshee (DigitalOcean)
<sforshee@...nel.org> wrote:
>
> Add handlers which read fs caps from the lower or upper filesystem and
> write/remove fs caps to the upper filesystem, performing copy-up as
> necessary.
>
> While fscaps only really make sense on regular files, the general policy
> is to allow most xattr namespaces on all different inode types, so
> fscaps handlers are installed in the inode operations for all types of
> inodes.
>
> Signed-off-by: Seth Forshee (DigitalOcean) <sforshee@...nel.org>
> ---
> fs/overlayfs/dir.c | 2 ++
> fs/overlayfs/inode.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++
> fs/overlayfs/overlayfs.h | 5 ++++
> 3 files changed, 79 insertions(+)
>
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index 0f8b4a719237..4ff360fe10c9 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -1307,6 +1307,8 @@ const struct inode_operations ovl_dir_inode_operations = {
> .get_inode_acl = ovl_get_inode_acl,
> .get_acl = ovl_get_acl,
> .set_acl = ovl_set_acl,
> + .get_fscaps = ovl_get_fscaps,
> + .set_fscaps = ovl_set_fscaps,
> .update_time = ovl_update_time,
> .fileattr_get = ovl_fileattr_get,
> .fileattr_set = ovl_fileattr_set,
> diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
> index c63b31a460be..7a8978ea6fe1 100644
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -568,6 +568,72 @@ int ovl_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
> }
> #endif
>
> +int ovl_get_fscaps(struct mnt_idmap *idmap, struct dentry *dentry,
> + struct vfs_caps *caps)
> +{
> + int err;
> + const struct cred *old_cred;
> + struct path realpath;
> +
> + ovl_path_real(dentry, &realpath);
> + old_cred = ovl_override_creds(dentry->d_sb);
> + err = vfs_get_fscaps(mnt_idmap(realpath.mnt), realpath.dentry, caps);
> + revert_creds(old_cred);
> + return err;
> +}
> +
> +int ovl_set_fscaps(struct mnt_idmap *idmap, struct dentry *dentry,
> + const struct vfs_caps *caps, int setxattr_flags)
> +{
> + int err;
> + struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
> + struct dentry *upperdentry = ovl_dentry_upper(dentry);
> + struct dentry *realdentry = upperdentry ?: ovl_dentry_lower(dentry);
> + const struct cred *old_cred;
> +
> + /*
> + * If the fscaps are to be remove from a lower file, check that they
> + * exist before copying up.
> + */
Don't you need to convert -ENODATA to 0 return value in this case?
> + if (!caps && !upperdentry) {
> + struct path realpath;
> + struct vfs_caps lower_caps;
> +
> + ovl_path_lower(dentry, &realpath);
> + old_cred = ovl_override_creds(dentry->d_sb);
> + err = vfs_get_fscaps(mnt_idmap(realpath.mnt), realdentry,
> + &lower_caps);
> + revert_creds(old_cred);
> + if (err)
> + goto out;
> + }
> +
> + err = ovl_want_write(dentry);
> + if (err)
> + goto out;
> +
ovl_want_write() should after ovl_copy_up(), see:
162d06444070 ("ovl: reorder ovl_want_write() after ovl_inode_lock()")
> + err = ovl_copy_up(dentry);
> + if (err)
> + goto out_drop_write;
> + upperdentry = ovl_dentry_upper(dentry);
> +
> + old_cred = ovl_override_creds(dentry->d_sb);
> + if (!caps)
> + err = vfs_remove_fscaps(ovl_upper_mnt_idmap(ofs), upperdentry);
> + else
> + err = vfs_set_fscaps(ovl_upper_mnt_idmap(ofs), upperdentry,
> + caps, setxattr_flags);
> + revert_creds(old_cred);
> +
> + /* copy c/mtime */
> + ovl_copyattr(d_inode(dentry));
> +
> +out_drop_write:
> + ovl_drop_write(dentry);
> +out:
> + return err;
> +}
> +
> int ovl_update_time(struct inode *inode, int flags)
> {
> if (flags & S_ATIME) {
> @@ -747,6 +813,8 @@ static const struct inode_operations ovl_file_inode_operations = {
> .get_inode_acl = ovl_get_inode_acl,
> .get_acl = ovl_get_acl,
> .set_acl = ovl_set_acl,
> + .get_fscaps = ovl_get_fscaps,
> + .set_fscaps = ovl_set_fscaps,
> .update_time = ovl_update_time,
> .fiemap = ovl_fiemap,
> .fileattr_get = ovl_fileattr_get,
> @@ -758,6 +826,8 @@ static const struct inode_operations ovl_symlink_inode_operations = {
> .get_link = ovl_get_link,
> .getattr = ovl_getattr,
> .listxattr = ovl_listxattr,
> + .get_fscaps = ovl_get_fscaps,
> + .set_fscaps = ovl_set_fscaps,
> .update_time = ovl_update_time,
> };
>
> @@ -769,6 +839,8 @@ static const struct inode_operations ovl_special_inode_operations = {
> .get_inode_acl = ovl_get_inode_acl,
> .get_acl = ovl_get_acl,
> .set_acl = ovl_set_acl,
> + .get_fscaps = ovl_get_fscaps,
> + .set_fscaps = ovl_set_fscaps,
> .update_time = ovl_update_time,
> };
>
Sorry, I did not understand the explanation why fscaps ops are needed
for non regular files. It does not look right to me.
Thanks,
Amir.
Powered by blists - more mailing lists