Linux-Security-Module Archive mirror
 help / color / mirror / Atom feed
From: Amir Goldstein <amir73il@gmail.com>
To: "Seth Forshee (DigitalOcean)" <sforshee@kernel.org>
Cc: Christian Brauner <brauner@kernel.org>,
	Serge Hallyn <serge@hallyn.com>,
	 Paul Moore <paul@paul-moore.com>, Eric Paris <eparis@redhat.com>,
	 James Morris <jmorris@namei.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>, Jan Kara <jack@suse.cz>,
	 Stephen Smalley <stephen.smalley.work@gmail.com>,
	Ondrej Mosnacek <omosnace@redhat.com>,
	 Casey Schaufler <casey@schaufler-ca.com>,
	Mimi Zohar <zohar@linux.ibm.com>,
	 Roberto Sassu <roberto.sassu@huawei.com>,
	Dmitry Kasatkin <dmitry.kasatkin@gmail.com>,
	 Eric Snowberg <eric.snowberg@oracle.com>,
	"Matthew Wilcox (Oracle)" <willy@infradead.org>,
	 Jonathan Corbet <corbet@lwn.net>,
	Miklos Szeredi <miklos@szeredi.hu>,
	linux-kernel@vger.kernel.org,  linux-fsdevel@vger.kernel.org,
	linux-security-module@vger.kernel.org,  audit@vger.kernel.org,
	selinux@vger.kernel.org,  linux-integrity@vger.kernel.org,
	linux-doc@vger.kernel.org,  linux-unionfs@vger.kernel.org
Subject: Re: [PATCH v2 20/25] ovl: add fscaps handlers
Date: Tue, 27 Feb 2024 15:28:18 +0200	[thread overview]
Message-ID: <CAOQ4uxjvrFuz2iCiO9dsOnear+qN=M+GFW-eEOZU5uCzBkTwLQ@mail.gmail.com> (raw)
In-Reply-To: <20240221-idmap-fscap-refactor-v2-20-3039364623bd@kernel.org>

On Wed, Feb 21, 2024 at 11:25 PM Seth Forshee (DigitalOcean)
<sforshee@kernel.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@kernel.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.

  parent reply	other threads:[~2024-02-27 13:28 UTC|newest]

Thread overview: 87+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-21 21:24 [PATCH v2 00/25] fs: use type-safe uid representation for filesystem capabilities Seth Forshee (DigitalOcean)
2024-02-21 21:24 ` [PATCH v2 01/25] mnt_idmapping: split out core vfs[ug]id_t definitions into vfsid.h Seth Forshee (DigitalOcean)
2024-02-22 14:09   ` Christian Brauner
2024-02-21 21:24 ` [PATCH v2 02/25] mnt_idmapping: include cred.h Seth Forshee (DigitalOcean)
2024-02-22 14:12   ` Christian Brauner
2024-02-21 21:24 ` [PATCH v2 03/25] capability: add static asserts for comapatibility of vfs_cap_data and vfs_ns_cap_data Seth Forshee (DigitalOcean)
2024-02-22 14:23   ` Christian Brauner
2024-02-21 21:24 ` [PATCH v2 04/25] capability: rename cpu_vfs_cap_data to vfs_caps Seth Forshee (DigitalOcean)
2024-02-21 21:24 ` [PATCH v2 05/25] capability: use vfsuid_t for vfs_caps rootids Seth Forshee (DigitalOcean)
2024-02-22 14:25   ` Christian Brauner
2024-02-21 21:24 ` [PATCH v2 06/25] capability: provide helpers for converting between xattrs and vfs_caps Seth Forshee (DigitalOcean)
2024-02-22 15:20   ` Christian Brauner
2024-02-22 15:38     ` Seth Forshee (DigitalOcean)
2024-02-23  8:08       ` Christian Brauner
2024-03-01 16:30   ` Roberto Sassu
2024-03-01 19:00     ` Seth Forshee (DigitalOcean)
2024-03-04  8:33       ` Roberto Sassu
2024-03-04 14:24         ` Seth Forshee (DigitalOcean)
2024-02-21 21:24 ` [PATCH v2 07/25] capability: provide a helper for converting vfs_caps to xattr for userspace Seth Forshee (DigitalOcean)
2024-02-22 15:22   ` Christian Brauner
2024-02-21 21:24 ` [PATCH v2 08/25] xattr: add is_fscaps_xattr() helper Seth Forshee (DigitalOcean)
2024-02-23  8:09   ` Christian Brauner
2024-02-21 21:24 ` [PATCH v2 09/25] commoncap: use is_fscaps_xattr() Seth Forshee (DigitalOcean)
2024-02-23  8:10   ` Christian Brauner
2024-02-21 21:24 ` [PATCH v2 10/25] xattr: " Seth Forshee (DigitalOcean)
2024-02-23  8:10   ` Christian Brauner
2024-02-21 21:24 ` [PATCH v2 11/25] security: add hooks for set/get/remove of fscaps Seth Forshee (DigitalOcean)
2024-02-21 23:31   ` Paul Moore
2024-02-22  0:07     ` Seth Forshee (DigitalOcean)
2024-02-23  8:23   ` Christian Brauner
2024-03-01 15:59   ` Roberto Sassu
2024-03-01 18:50     ` Seth Forshee (DigitalOcean)
2024-02-21 21:24 ` [PATCH v2 12/25] selinux: add hooks for fscaps operations Seth Forshee (DigitalOcean)
2024-02-21 23:38   ` Paul Moore
2024-02-22  0:10     ` Seth Forshee (DigitalOcean)
2024-02-22  0:19       ` Paul Moore
2024-02-22  0:28         ` Seth Forshee (DigitalOcean)
2024-02-21 21:24 ` [PATCH v2 13/25] smack: " Seth Forshee (DigitalOcean)
2024-02-21 22:52   ` Casey Schaufler
2024-02-22  0:11     ` Seth Forshee (DigitalOcean)
2024-02-21 21:24 ` [PATCH v2 14/25] evm: add support for fscaps security hooks Seth Forshee (DigitalOcean)
2024-03-01  9:19   ` Roberto Sassu
2024-03-01 12:54     ` Christian Brauner
2024-03-01 13:19       ` Roberto Sassu
2024-03-01 13:39         ` Christian Brauner
2024-03-01 14:39     ` Seth Forshee (DigitalOcean)
2024-03-01 15:04       ` Roberto Sassu
2024-03-04 15:01   ` Roberto Sassu
2024-02-21 21:24 ` [PATCH v2 15/25] security: call evm fscaps hooks from generic " Seth Forshee (DigitalOcean)
2024-02-21 23:43   ` Paul Moore
2024-02-22  0:20     ` Seth Forshee (DigitalOcean)
2024-02-22  0:37       ` Paul Moore
2024-02-21 21:24 ` [PATCH v2 16/25] fs: add inode operations to get/set/remove fscaps Seth Forshee (DigitalOcean)
2024-02-23  8:25   ` Christian Brauner
2024-02-21 21:24 ` [PATCH v2 17/25] fs: add vfs_get_fscaps() Seth Forshee (DigitalOcean)
2024-02-23  8:28   ` Christian Brauner
2024-02-21 21:24 ` [PATCH v2 18/25] fs: add vfs_set_fscaps() Seth Forshee (DigitalOcean)
2024-02-23  8:38   ` Christian Brauner
2024-02-21 21:24 ` [PATCH v2 19/25] fs: add vfs_remove_fscaps() Seth Forshee (DigitalOcean)
2024-02-23  8:40   ` Christian Brauner
2024-02-21 21:24 ` [PATCH v2 20/25] ovl: add fscaps handlers Seth Forshee (DigitalOcean)
2024-02-23  9:04   ` Christian Brauner
2024-02-27 13:28   ` Amir Goldstein [this message]
2024-02-27 14:57     ` Seth Forshee (DigitalOcean)
2024-02-21 21:24 ` [PATCH v2 21/25] ovl: use vfs_{get,set}_fscaps() for copy-up Seth Forshee (DigitalOcean)
2024-02-21 21:24 ` [PATCH v2 22/25] fs: use vfs interfaces for capabilities xattrs Seth Forshee (DigitalOcean)
2024-02-21 21:24 ` [PATCH v2 23/25] commoncap: remove cap_inode_getsecurity() Seth Forshee (DigitalOcean)
2024-02-21 21:24 ` [PATCH v2 24/25] commoncap: use vfs fscaps interfaces Seth Forshee (DigitalOcean)
2024-03-04 10:19   ` Roberto Sassu
2024-03-04 15:31     ` Seth Forshee (DigitalOcean)
2024-03-04 16:17       ` Roberto Sassu
2024-03-04 16:56         ` Seth Forshee (DigitalOcean)
2024-03-05  9:12           ` Christian Brauner
2024-03-05 12:46             ` Roberto Sassu
2024-03-05 16:26               ` Christian Brauner
2024-03-05 16:35                 ` Roberto Sassu
2024-03-05 17:03                   ` Seth Forshee (DigitalOcean)
2024-03-05 17:08                     ` Roberto Sassu
2024-03-05 17:11               ` Roberto Sassu
2024-03-05 20:17                 ` Seth Forshee (DigitalOcean)
2024-03-06  8:30                   ` Roberto Sassu
2024-03-06  2:17                 ` Mimi Zohar
2024-03-06  8:25                   ` Roberto Sassu
2024-03-06 12:56                     ` Mimi Zohar
2024-02-21 21:24 ` [PATCH v2 25/25] vfs: return -EOPNOTSUPP for fscaps from vfs_*xattr() Seth Forshee (DigitalOcean)
2024-02-22 15:27 ` [PATCH v2 00/25] fs: use type-safe uid representation for filesystem capabilities Christian Brauner
2024-02-22 16:28   ` Seth Forshee (DigitalOcean)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAOQ4uxjvrFuz2iCiO9dsOnear+qN=M+GFW-eEOZU5uCzBkTwLQ@mail.gmail.com' \
    --to=amir73il@gmail.com \
    --cc=audit@vger.kernel.org \
    --cc=brauner@kernel.org \
    --cc=casey@schaufler-ca.com \
    --cc=corbet@lwn.net \
    --cc=dmitry.kasatkin@gmail.com \
    --cc=eparis@redhat.com \
    --cc=eric.snowberg@oracle.com \
    --cc=jack@suse.cz \
    --cc=jmorris@namei.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=linux-unionfs@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=omosnace@redhat.com \
    --cc=paul@paul-moore.com \
    --cc=roberto.sassu@huawei.com \
    --cc=selinux@vger.kernel.org \
    --cc=serge@hallyn.com \
    --cc=sforshee@kernel.org \
    --cc=stephen.smalley.work@gmail.com \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@infradead.org \
    --cc=zohar@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).