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