* [PATCH 01/18] vfs: add miscattr ops
2021-02-03 12:40 [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR Miklos Szeredi
@ 2021-02-03 12:40 ` Miklos Szeredi
2021-02-03 15:04 ` Jan Kara
2021-03-23 0:23 ` Eric Biggers
2021-02-03 12:40 ` [PATCH 02/18] ecryptfs: stack " Miklos Szeredi
` (17 subsequent siblings)
18 siblings, 2 replies; 40+ messages in thread
From: Miklos Szeredi @ 2021-02-03 12:40 UTC (permalink / raw)
To: linux-fsdevel
Cc: linux-kernel, Al Viro, Andreas Dilger, Andreas Gruenbacher,
Christoph Hellwig, Darrick J . Wong, Dave Kleikamp, David Sterba,
Jaegeuk Kim, Jan Kara, Joel Becker, Matthew Garrett,
Mike Marshall, Richard Weinberger, Ryusuke Konishi,
Theodore Ts'o, Tyler Hicks
There's a substantial amount of boilerplate in filesystems handling
FS_IOC_[GS]ETFLAGS/ FS_IOC_FS[GS]ETXATTR ioctls.
Also due to userspace buffers being involved in the ioctl API this is
difficult to stack, as shown by overlayfs issues related to these ioctls.
Introduce a new internal API named "miscattr" (fsxattr can be confused with
xattr, xflags is inappropriate, since this is more than just flags).
There's significant overlap between flags and xflags and this API handles
the conversions automatically, so filesystems may choose which one to use.
In ->miscattr_get() a hint is provided to the filesystem whether flags or
xattr are being requested by userspace, but in this series this hint is
ignored by all filesystems, since generating all the attributes is cheap.
If a filesystem doesn't implemement the miscattr API, just fall back to
f_op->ioctl(). When all filesystems are converted, the fallback can be
removed.
32bit compat ioctls are now handled by the generic code as well.
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
Documentation/filesystems/locking.rst | 4 +
Documentation/filesystems/vfs.rst | 14 ++
fs/ioctl.c | 329 ++++++++++++++++++++++++++
include/linux/fs.h | 3 +
include/linux/miscattr.h | 52 ++++
5 files changed, 402 insertions(+)
create mode 100644 include/linux/miscattr.h
diff --git a/Documentation/filesystems/locking.rst b/Documentation/filesystems/locking.rst
index c0f2c7586531..5c0a30dd8336 100644
--- a/Documentation/filesystems/locking.rst
+++ b/Documentation/filesystems/locking.rst
@@ -80,6 +80,8 @@ prototypes::
struct file *, unsigned open_flag,
umode_t create_mode);
int (*tmpfile) (struct inode *, struct dentry *, umode_t);
+ int (*miscattr_set)(struct dentry *dentry, struct miscattr *ma);
+ int (*miscattr_get)(struct dentry *dentry, struct miscattr *ma);
locking rules:
all may block
@@ -107,6 +109,8 @@ fiemap: no
update_time: no
atomic_open: shared (exclusive if O_CREAT is set in open flags)
tmpfile: no
+miscattr_get: no or exclusive
+miscattr_set: exclusive
============ =============================================
diff --git a/Documentation/filesystems/vfs.rst b/Documentation/filesystems/vfs.rst
index ca52c82e5bb5..a0ee8fc85678 100644
--- a/Documentation/filesystems/vfs.rst
+++ b/Documentation/filesystems/vfs.rst
@@ -437,6 +437,8 @@ As of kernel 2.6.22, the following members are defined:
int (*atomic_open)(struct inode *, struct dentry *, struct file *,
unsigned open_flag, umode_t create_mode);
int (*tmpfile) (struct inode *, struct dentry *, umode_t);
+ int (*miscattr_set)(struct dentry *dentry, struct miscattr *ma);
+ int (*miscattr_get)(struct dentry *dentry, struct miscattr *ma);
};
Again, all methods are called without any locks being held, unless
@@ -584,6 +586,18 @@ otherwise noted.
atomically creating, opening and unlinking a file in given
directory.
+``miscattr_get``
+ called on ioctl(FS_IOC_GETFLAGS) and ioctl(FS_IOC_FSGETXATTR) to
+ retrieve miscellaneous filesystem flags and attributes. Also
+ called before the relevant SET operation to check what is being
+ changed (in this case with i_rwsem locked exclusive). If unset,
+ then fall back to f_op->ioctl().
+
+``miscattr_set``
+ called on ioctl(FS_IOC_SETFLAGS) and ioctl(FS_IOC_FSSETXATTR) to
+ change miscellaneous filesystem flags and attributes. Callers hold
+ i_rwsem exclusive. If unset, then fall back to f_op->ioctl().
+
The Address Space Object
========================
diff --git a/fs/ioctl.c b/fs/ioctl.c
index 4e6cc0a7d69c..f38f75c82e76 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -19,6 +19,9 @@
#include <linux/falloc.h>
#include <linux/sched/signal.h>
#include <linux/fiemap.h>
+#include <linux/mount.h>
+#include <linux/fscrypt.h>
+#include <linux/miscattr.h>
#include "internal.h"
@@ -657,6 +660,311 @@ static int ioctl_file_dedupe_range(struct file *file,
return ret;
}
+/**
+ * miscattr_fill_xflags - initialize miscattr with xflags
+ * @ma: miscattr pointer
+ * @xflags: FS_XFLAG_* flags
+ *
+ * Set ->fsx_xflags, ->xattr_valid and ->flags (translated xflags). All
+ * other fields are zeroed.
+ */
+void miscattr_fill_xflags(struct miscattr *ma, u32 xflags)
+{
+ memset(ma, 0, sizeof(*ma));
+ ma->xattr_valid = true;
+ ma->fsx_xflags = xflags;
+ if (ma->fsx_xflags & FS_XFLAG_IMMUTABLE)
+ ma->flags |= FS_IMMUTABLE_FL;
+ if (ma->fsx_xflags & FS_XFLAG_APPEND)
+ ma->flags |= FS_APPEND_FL;
+ if (ma->fsx_xflags & FS_XFLAG_SYNC)
+ ma->flags |= FS_SYNC_FL;
+ if (ma->fsx_xflags & FS_XFLAG_NOATIME)
+ ma->flags |= FS_NOATIME_FL;
+ if (ma->fsx_xflags & FS_XFLAG_NODUMP)
+ ma->flags |= FS_NODUMP_FL;
+ if (ma->fsx_xflags & FS_XFLAG_DAX)
+ ma->flags |= FS_DAX_FL;
+ if (ma->fsx_xflags & FS_XFLAG_PROJINHERIT)
+ ma->flags |= FS_PROJINHERIT_FL;
+}
+EXPORT_SYMBOL(miscattr_fill_xflags);
+
+/**
+ * miscattr_fill_flags - initialize miscattr with flags
+ * @ma: miscattr pointer
+ * @flags: FS_*_FL flags
+ *
+ * Set ->flags, ->flags_valid and ->fsx_xflags (translated flags).
+ * All other fields are zeroed.
+ */
+void miscattr_fill_flags(struct miscattr *ma, u32 flags)
+{
+ memset(ma, 0, sizeof(*ma));
+ ma->flags_valid = true;
+ ma->flags = flags;
+ if (ma->flags & FS_SYNC_FL)
+ ma->fsx_xflags |= FS_XFLAG_SYNC;
+ if (ma->flags & FS_IMMUTABLE_FL)
+ ma->fsx_xflags |= FS_XFLAG_IMMUTABLE;
+ if (ma->flags & FS_APPEND_FL)
+ ma->fsx_xflags |= FS_XFLAG_APPEND;
+ if (ma->flags & FS_NODUMP_FL)
+ ma->fsx_xflags |= FS_XFLAG_NODUMP;
+ if (ma->flags & FS_NOATIME_FL)
+ ma->fsx_xflags |= FS_XFLAG_NOATIME;
+ if (ma->flags & FS_DAX_FL)
+ ma->fsx_xflags |= FS_XFLAG_DAX;
+ if (ma->flags & FS_PROJINHERIT_FL)
+ ma->fsx_xflags |= FS_XFLAG_PROJINHERIT;
+}
+EXPORT_SYMBOL(miscattr_fill_flags);
+
+/**
+ * vfs_miscattr_get - retrieve miscellaneous inode attributes
+ * @dentry: the object to retrieve from
+ * @ma: miscattr pointer
+ *
+ * Call i_op->miscattr_get() callback, if exists.
+ *
+ * Returns 0 on success, or a negative error on failure.
+ */
+int vfs_miscattr_get(struct dentry *dentry, struct miscattr *ma)
+{
+ struct inode *inode = d_inode(dentry);
+
+ if (d_is_special(dentry))
+ return -ENOTTY;
+
+ if (!inode->i_op->miscattr_get)
+ return -ENOIOCTLCMD;
+
+ memset(ma, 0, sizeof(*ma));
+ return inode->i_op->miscattr_get(dentry, ma);
+}
+EXPORT_SYMBOL(vfs_miscattr_get);
+
+/**
+ * fsxattr_copy_to_user - copy fsxattr to userspace.
+ * @ma: miscattr pointer
+ * @ufa: fsxattr user pointer
+ *
+ * Returns 0 on success, or -EFAULT on failure.
+ */
+int fsxattr_copy_to_user(const struct miscattr *ma, struct fsxattr __user *ufa)
+{
+ struct fsxattr fa = {
+ .fsx_xflags = ma->fsx_xflags,
+ .fsx_extsize = ma->fsx_extsize,
+ .fsx_nextents = ma->fsx_nextents,
+ .fsx_projid = ma->fsx_projid,
+ .fsx_cowextsize = ma->fsx_cowextsize,
+ };
+
+ if (copy_to_user(ufa, &fa, sizeof(fa)))
+ return -EFAULT;
+
+ return 0;
+}
+EXPORT_SYMBOL(fsxattr_copy_to_user);
+
+static int fsxattr_copy_from_user(struct miscattr *ma,
+ struct fsxattr __user *ufa)
+{
+ struct fsxattr fa;
+
+ if (copy_from_user(&fa, ufa, sizeof(fa)))
+ return -EFAULT;
+
+ miscattr_fill_xflags(ma, fa.fsx_xflags);
+ ma->fsx_extsize = fa.fsx_extsize;
+ ma->fsx_nextents = fa.fsx_nextents;
+ ma->fsx_projid = fa.fsx_projid;
+ ma->fsx_cowextsize = fa.fsx_cowextsize;
+
+ return 0;
+}
+
+/*
+ * Generic function to check FS_IOC_FSSETXATTR/FS_IOC_SETFLAGS values and reject
+ * any invalid configurations.
+ *
+ * Note: must be called with inode lock held.
+ */
+static int miscattr_set_prepare(struct inode *inode,
+ const struct miscattr *old_ma,
+ struct miscattr *ma)
+{
+ int err;
+
+ /*
+ * The IMMUTABLE and APPEND_ONLY flags can only be changed by
+ * the relevant capability.
+ */
+ if ((ma->flags ^ old_ma->flags) & (FS_APPEND_FL | FS_IMMUTABLE_FL) &&
+ !capable(CAP_LINUX_IMMUTABLE))
+ return -EPERM;
+
+ err = fscrypt_prepare_setflags(inode, old_ma->flags, ma->flags);
+ if (err)
+ return err;
+
+ /*
+ * Project Quota ID state is only allowed to change from within the init
+ * namespace. Enforce that restriction only if we are trying to change
+ * the quota ID state. Everything else is allowed in user namespaces.
+ */
+ if (current_user_ns() != &init_user_ns) {
+ if (old_ma->fsx_projid != ma->fsx_projid)
+ return -EINVAL;
+ if ((old_ma->fsx_xflags ^ ma->fsx_xflags) &
+ FS_XFLAG_PROJINHERIT)
+ return -EINVAL;
+ }
+
+ /* Check extent size hints. */
+ if ((ma->fsx_xflags & FS_XFLAG_EXTSIZE) && !S_ISREG(inode->i_mode))
+ return -EINVAL;
+
+ if ((ma->fsx_xflags & FS_XFLAG_EXTSZINHERIT) &&
+ !S_ISDIR(inode->i_mode))
+ return -EINVAL;
+
+ if ((ma->fsx_xflags & FS_XFLAG_COWEXTSIZE) &&
+ !S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode))
+ return -EINVAL;
+
+ /*
+ * It is only valid to set the DAX flag on regular files and
+ * directories on filesystems.
+ */
+ if ((ma->fsx_xflags & FS_XFLAG_DAX) &&
+ !(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode)))
+ return -EINVAL;
+
+ /* Extent size hints of zero turn off the flags. */
+ if (ma->fsx_extsize == 0)
+ ma->fsx_xflags &= ~(FS_XFLAG_EXTSIZE | FS_XFLAG_EXTSZINHERIT);
+ if (ma->fsx_cowextsize == 0)
+ ma->fsx_xflags &= ~FS_XFLAG_COWEXTSIZE;
+
+ return 0;
+}
+
+/**
+ * vfs_miscattr_set - change miscellaneous inode attributes
+ * @dentry: the object to change
+ * @ma: miscattr pointer
+ *
+ * After verifying permissions, call i_op->miscattr_set() callback, if
+ * exists.
+ *
+ * Verifying attributes involves retrieving current attributes with
+ * i_op->miscattr_get(), this also allows initilaizing attributes that have
+ * not been set by the caller to current values. Inode lock is held
+ * thoughout to prevent racing with another instance.
+ *
+ * Returns 0 on success, or a negative error on failure.
+ */
+int vfs_miscattr_set(struct dentry *dentry, struct miscattr *ma)
+{
+ struct inode *inode = d_inode(dentry);
+ struct miscattr old_ma = {};
+ int err;
+
+ if (d_is_special(dentry))
+ return -ENOTTY;
+
+ if (!inode->i_op->miscattr_set)
+ return -ENOIOCTLCMD;
+
+ if (!inode_owner_or_capable(inode))
+ return -EPERM;
+
+ inode_lock(inode);
+ err = vfs_miscattr_get(dentry, &old_ma);
+ if (!err) {
+ /* initialize missing bits from old_ma */
+ if (ma->flags_valid) {
+ ma->fsx_xflags |= old_ma.fsx_xflags & ~FS_XFLAG_COMMON;
+ ma->fsx_extsize = old_ma.fsx_extsize;
+ ma->fsx_nextents = old_ma.fsx_nextents;
+ ma->fsx_projid = old_ma.fsx_projid;
+ ma->fsx_cowextsize = old_ma.fsx_cowextsize;
+ } else {
+ ma->flags |= old_ma.flags & ~FS_COMMON_FL;
+ }
+ err = miscattr_set_prepare(inode, &old_ma, ma);
+ if (!err)
+ err = inode->i_op->miscattr_set(dentry, ma);
+ }
+ inode_unlock(inode);
+
+ return err;
+}
+EXPORT_SYMBOL(vfs_miscattr_set);
+
+static int ioctl_getflags(struct file *file, void __user *argp)
+{
+ struct miscattr ma = { .flags_valid = true }; /* hint only */
+ unsigned int flags;
+ int err;
+
+ err = vfs_miscattr_get(file_dentry(file), &ma);
+ if (!err) {
+ flags = ma.flags;
+ if (copy_to_user(argp, &flags, sizeof(flags)))
+ err = -EFAULT;
+ }
+ return err;
+}
+
+static int ioctl_setflags(struct file *file, void __user *argp)
+{
+ struct miscattr ma;
+ unsigned int flags;
+ int err;
+
+ if (copy_from_user(&flags, argp, sizeof(flags)))
+ return -EFAULT;
+
+ err = mnt_want_write_file(file);
+ if (!err) {
+ miscattr_fill_flags(&ma, flags);
+ err = vfs_miscattr_set(file_dentry(file), &ma);
+ mnt_drop_write_file(file);
+ }
+ return err;
+}
+
+static int ioctl_fsgetxattr(struct file *file, void __user *argp)
+{
+ struct miscattr ma = { .xattr_valid = true }; /* hint only */
+ int err;
+
+ err = vfs_miscattr_get(file_dentry(file), &ma);
+ if (!err)
+ err = fsxattr_copy_to_user(&ma, argp);
+
+ return err;
+}
+
+static int ioctl_fssetxattr(struct file *file, void __user *argp)
+{
+ struct miscattr ma;
+ int err;
+
+ err = fsxattr_copy_from_user(&ma, argp);
+ if (!err) {
+ err = mnt_want_write_file(file);
+ if (!err) {
+ err = vfs_miscattr_set(file_dentry(file), &ma);
+ mnt_drop_write_file(file);
+ }
+ }
+ return err;
+}
+
/*
* do_vfs_ioctl() is not for drivers and not intended to be EXPORT_SYMBOL()'d.
* It's just a simple helper for sys_ioctl and compat_sys_ioctl.
@@ -727,6 +1035,18 @@ static int do_vfs_ioctl(struct file *filp, unsigned int fd,
return put_user(i_size_read(inode) - filp->f_pos,
(int __user *)argp);
+ case FS_IOC_GETFLAGS:
+ return ioctl_getflags(filp, argp);
+
+ case FS_IOC_SETFLAGS:
+ return ioctl_setflags(filp, argp);
+
+ case FS_IOC_FSGETXATTR:
+ return ioctl_fsgetxattr(filp, argp);
+
+ case FS_IOC_FSSETXATTR:
+ return ioctl_fssetxattr(filp, argp);
+
default:
if (S_ISREG(inode->i_mode))
return file_ioctl(filp, cmd, argp);
@@ -827,6 +1147,15 @@ COMPAT_SYSCALL_DEFINE3(ioctl, unsigned int, fd, unsigned int, cmd,
break;
#endif
+ /*
+ * These access 32-bit values anyway so no further handling is
+ * necessary.
+ */
+ case FS_IOC32_GETFLAGS:
+ case FS_IOC32_SETFLAGS:
+ cmd = (cmd == FS_IOC32_GETFLAGS) ?
+ FS_IOC_GETFLAGS : FS_IOC_SETFLAGS;
+ fallthrough;
/*
* everything else in do_vfs_ioctl() takes either a compatible
* pointer argument or no argument -- call it with a modified
diff --git a/include/linux/fs.h b/include/linux/fs.h
index fd47deea7c17..ca3bab7d5f6e 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -68,6 +68,7 @@ struct fsverity_info;
struct fsverity_operations;
struct fs_context;
struct fs_parameter_spec;
+struct miscattr;
extern void __init inode_init(void);
extern void __init inode_init_early(void);
@@ -1887,6 +1888,8 @@ struct inode_operations {
umode_t create_mode);
int (*tmpfile) (struct inode *, struct dentry *, umode_t);
int (*set_acl)(struct inode *, struct posix_acl *, int);
+ int (*miscattr_set)(struct dentry *dentry, struct miscattr *ma);
+ int (*miscattr_get)(struct dentry *dentry, struct miscattr *ma);
} ____cacheline_aligned;
static inline ssize_t call_read_iter(struct file *file, struct kiocb *kio,
diff --git a/include/linux/miscattr.h b/include/linux/miscattr.h
new file mode 100644
index 000000000000..1c531779dc9d
--- /dev/null
+++ b/include/linux/miscattr.h
@@ -0,0 +1,52 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _LINUX_MISCATTR_H
+#define _LINUX_MISCATTR_H
+
+/* Flags shared betwen flags/xflags */
+#define FS_COMMON_FL \
+ (FS_SYNC_FL | FS_IMMUTABLE_FL | FS_APPEND_FL | \
+ FS_NODUMP_FL | FS_NOATIME_FL | FS_DAX_FL | \
+ FS_PROJINHERIT_FL)
+
+#define FS_XFLAG_COMMON \
+ (FS_XFLAG_SYNC | FS_XFLAG_IMMUTABLE | FS_XFLAG_APPEND | \
+ FS_XFLAG_NODUMP | FS_XFLAG_NOATIME | FS_XFLAG_DAX | \
+ FS_XFLAG_PROJINHERIT)
+
+struct miscattr {
+ u32 flags; /* flags (FS_IOC_GETFLAGS/FS_IOC_SETFLAGS) */
+ /* struct fsxattr: */
+ u32 fsx_xflags; /* xflags field value (get/set) */
+ u32 fsx_extsize; /* extsize field value (get/set)*/
+ u32 fsx_nextents; /* nextents field value (get) */
+ u32 fsx_projid; /* project identifier (get/set) */
+ u32 fsx_cowextsize; /* CoW extsize field value (get/set)*/
+ /* selectors: */
+ bool flags_valid:1;
+ bool xattr_valid:1;
+};
+
+int fsxattr_copy_to_user(const struct miscattr *ma, struct fsxattr __user *ufa);
+
+void miscattr_fill_xflags(struct miscattr *ma, u32 xflags);
+void miscattr_fill_flags(struct miscattr *ma, u32 flags);
+
+/**
+ * miscattr_has_xattr - check for extentended flags/attributes
+ * @ma: miscattr pointer
+ *
+ * Returns true if any attributes are present that are not represented in
+ * ->flags.
+ */
+static inline bool miscattr_has_xattr(const struct miscattr *ma)
+{
+ return ma->xattr_valid &&
+ ((ma->fsx_xflags & ~FS_XFLAG_COMMON) || ma->fsx_extsize != 0 ||
+ ma->fsx_projid != 0 || ma->fsx_cowextsize != 0);
+}
+
+int vfs_miscattr_get(struct dentry *dentry, struct miscattr *ma);
+int vfs_miscattr_set(struct dentry *dentry, struct miscattr *ma);
+
+#endif /* _LINUX_MISCATTR_H */
--
2.26.2
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH 01/18] vfs: add miscattr ops
2021-02-03 12:40 ` [PATCH 01/18] vfs: add miscattr ops Miklos Szeredi
@ 2021-02-03 15:04 ` Jan Kara
2021-02-03 15:21 ` Miklos Szeredi
2021-03-23 0:23 ` Eric Biggers
1 sibling, 1 reply; 40+ messages in thread
From: Jan Kara @ 2021-02-03 15:04 UTC (permalink / raw)
To: Miklos Szeredi
Cc: linux-fsdevel, linux-kernel, Al Viro, Andreas Dilger,
Andreas Gruenbacher, Christoph Hellwig, Darrick J . Wong,
Dave Kleikamp, David Sterba, Jaegeuk Kim, Jan Kara, Joel Becker,
Matthew Garrett, Mike Marshall, Richard Weinberger,
Ryusuke Konishi, Theodore Ts'o, Tyler Hicks
Hi Miklos!
On Wed 03-02-21 13:40:55, Miklos Szeredi wrote:
> There's a substantial amount of boilerplate in filesystems handling
> FS_IOC_[GS]ETFLAGS/ FS_IOC_FS[GS]ETXATTR ioctls.
>
> Also due to userspace buffers being involved in the ioctl API this is
> difficult to stack, as shown by overlayfs issues related to these ioctls.
>
> Introduce a new internal API named "miscattr" (fsxattr can be confused with
> xattr, xflags is inappropriate, since this is more than just flags).
>
> There's significant overlap between flags and xflags and this API handles
> the conversions automatically, so filesystems may choose which one to use.
>
> In ->miscattr_get() a hint is provided to the filesystem whether flags or
> xattr are being requested by userspace, but in this series this hint is
> ignored by all filesystems, since generating all the attributes is cheap.
>
> If a filesystem doesn't implemement the miscattr API, just fall back to
> f_op->ioctl(). When all filesystems are converted, the fallback can be
> removed.
>
> 32bit compat ioctls are now handled by the generic code as well.
>
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Getting rid of the boilerplate code and improving stacking of these calls
look like a nice goal to me :) Some technical comments below.
> +/**
> + * miscattr_fill_xflags - initialize miscattr with xflags
> + * @ma: miscattr pointer
> + * @xflags: FS_XFLAG_* flags
> + *
> + * Set ->fsx_xflags, ->xattr_valid and ->flags (translated xflags). All
> + * other fields are zeroed.
> + */
> +void miscattr_fill_xflags(struct miscattr *ma, u32 xflags)
Maybe call this miscattr_fill_from_xflags() and the next function
miscattr_fill_from_flags()? At least to me it would be clearer when I want
to use which function just by looking at the name...
> +{
> + memset(ma, 0, sizeof(*ma));
> + ma->xattr_valid = true;
> + ma->fsx_xflags = xflags;
> + if (ma->fsx_xflags & FS_XFLAG_IMMUTABLE)
> + ma->flags |= FS_IMMUTABLE_FL;
> + if (ma->fsx_xflags & FS_XFLAG_APPEND)
> + ma->flags |= FS_APPEND_FL;
> + if (ma->fsx_xflags & FS_XFLAG_SYNC)
> + ma->flags |= FS_SYNC_FL;
> + if (ma->fsx_xflags & FS_XFLAG_NOATIME)
> + ma->flags |= FS_NOATIME_FL;
> + if (ma->fsx_xflags & FS_XFLAG_NODUMP)
> + ma->flags |= FS_NODUMP_FL;
> + if (ma->fsx_xflags & FS_XFLAG_DAX)
> + ma->flags |= FS_DAX_FL;
> + if (ma->fsx_xflags & FS_XFLAG_PROJINHERIT)
> + ma->flags |= FS_PROJINHERIT_FL;
> +}
> +EXPORT_SYMBOL(miscattr_fill_xflags);
> +
> +/**
> + * miscattr_fill_flags - initialize miscattr with flags
> + * @ma: miscattr pointer
> + * @flags: FS_*_FL flags
> + *
> + * Set ->flags, ->flags_valid and ->fsx_xflags (translated flags).
> + * All other fields are zeroed.
> + */
> +void miscattr_fill_flags(struct miscattr *ma, u32 flags)
> +{
> + memset(ma, 0, sizeof(*ma));
> + ma->flags_valid = true;
> + ma->flags = flags;
> + if (ma->flags & FS_SYNC_FL)
> + ma->fsx_xflags |= FS_XFLAG_SYNC;
> + if (ma->flags & FS_IMMUTABLE_FL)
> + ma->fsx_xflags |= FS_XFLAG_IMMUTABLE;
> + if (ma->flags & FS_APPEND_FL)
> + ma->fsx_xflags |= FS_XFLAG_APPEND;
> + if (ma->flags & FS_NODUMP_FL)
> + ma->fsx_xflags |= FS_XFLAG_NODUMP;
> + if (ma->flags & FS_NOATIME_FL)
> + ma->fsx_xflags |= FS_XFLAG_NOATIME;
> + if (ma->flags & FS_DAX_FL)
> + ma->fsx_xflags |= FS_XFLAG_DAX;
> + if (ma->flags & FS_PROJINHERIT_FL)
> + ma->fsx_xflags |= FS_XFLAG_PROJINHERIT;
> +}
> +EXPORT_SYMBOL(miscattr_fill_flags);
> +
> +/**
> + * vfs_miscattr_get - retrieve miscellaneous inode attributes
> + * @dentry: the object to retrieve from
> + * @ma: miscattr pointer
> + *
> + * Call i_op->miscattr_get() callback, if exists.
> + *
> + * Returns 0 on success, or a negative error on failure.
> + */
> +int vfs_miscattr_get(struct dentry *dentry, struct miscattr *ma)
> +{
> + struct inode *inode = d_inode(dentry);
> +
> + if (d_is_special(dentry))
> + return -ENOTTY;
> +
> + if (!inode->i_op->miscattr_get)
> + return -ENOIOCTLCMD;
> +
> + memset(ma, 0, sizeof(*ma));
So here we clear whole 'ma' but callers already set e.g. xattr_valid field
and cleared the 'ma' as well which just looks silly...
> + return inode->i_op->miscattr_get(dentry, ma);
> +}
> +EXPORT_SYMBOL(vfs_miscattr_get);
...
> +/*
> + * Generic function to check FS_IOC_FSSETXATTR/FS_IOC_SETFLAGS values and reject
> + * any invalid configurations.
> + *
> + * Note: must be called with inode lock held.
> + */
> +static int miscattr_set_prepare(struct inode *inode,
> + const struct miscattr *old_ma,
> + struct miscattr *ma)
> +{
> + int err;
> +
> + /*
> + * The IMMUTABLE and APPEND_ONLY flags can only be changed by
> + * the relevant capability.
> + */
> + if ((ma->flags ^ old_ma->flags) & (FS_APPEND_FL | FS_IMMUTABLE_FL) &&
> + !capable(CAP_LINUX_IMMUTABLE))
> + return -EPERM;
> +
> + err = fscrypt_prepare_setflags(inode, old_ma->flags, ma->flags);
> + if (err)
> + return err;
> +
> + /*
> + * Project Quota ID state is only allowed to change from within the init
> + * namespace. Enforce that restriction only if we are trying to change
> + * the quota ID state. Everything else is allowed in user namespaces.
> + */
> + if (current_user_ns() != &init_user_ns) {
> + if (old_ma->fsx_projid != ma->fsx_projid)
> + return -EINVAL;
> + if ((old_ma->fsx_xflags ^ ma->fsx_xflags) &
> + FS_XFLAG_PROJINHERIT)
> + return -EINVAL;
> + }
> +
> + /* Check extent size hints. */
> + if ((ma->fsx_xflags & FS_XFLAG_EXTSIZE) && !S_ISREG(inode->i_mode))
> + return -EINVAL;
> +
> + if ((ma->fsx_xflags & FS_XFLAG_EXTSZINHERIT) &&
> + !S_ISDIR(inode->i_mode))
> + return -EINVAL;
> +
> + if ((ma->fsx_xflags & FS_XFLAG_COWEXTSIZE) &&
> + !S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode))
> + return -EINVAL;
> +
> + /*
> + * It is only valid to set the DAX flag on regular files and
> + * directories on filesystems.
> + */
> + if ((ma->fsx_xflags & FS_XFLAG_DAX) &&
> + !(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode)))
> + return -EINVAL;
> +
> + /* Extent size hints of zero turn off the flags. */
> + if (ma->fsx_extsize == 0)
> + ma->fsx_xflags &= ~(FS_XFLAG_EXTSIZE | FS_XFLAG_EXTSZINHERIT);
> + if (ma->fsx_cowextsize == 0)
> + ma->fsx_xflags &= ~FS_XFLAG_COWEXTSIZE;
> +
> + return 0;
> +}
> +
> +/**
> + * vfs_miscattr_set - change miscellaneous inode attributes
> + * @dentry: the object to change
> + * @ma: miscattr pointer
> + *
> + * After verifying permissions, call i_op->miscattr_set() callback, if
> + * exists.
> + *
> + * Verifying attributes involves retrieving current attributes with
> + * i_op->miscattr_get(), this also allows initilaizing attributes that have
> + * not been set by the caller to current values. Inode lock is held
> + * thoughout to prevent racing with another instance.
> + *
> + * Returns 0 on success, or a negative error on failure.
> + */
> +int vfs_miscattr_set(struct dentry *dentry, struct miscattr *ma)
> +{
> + struct inode *inode = d_inode(dentry);
> + struct miscattr old_ma = {};
> + int err;
> +
> + if (d_is_special(dentry))
> + return -ENOTTY;
> +
> + if (!inode->i_op->miscattr_set)
> + return -ENOIOCTLCMD;
> +
> + if (!inode_owner_or_capable(inode))
> + return -EPERM;
> +
> + inode_lock(inode);
> + err = vfs_miscattr_get(dentry, &old_ma);
> + if (!err) {
> + /* initialize missing bits from old_ma */
> + if (ma->flags_valid) {
> + ma->fsx_xflags |= old_ma.fsx_xflags & ~FS_XFLAG_COMMON;
> + ma->fsx_extsize = old_ma.fsx_extsize;
> + ma->fsx_nextents = old_ma.fsx_nextents;
> + ma->fsx_projid = old_ma.fsx_projid;
> + ma->fsx_cowextsize = old_ma.fsx_cowextsize;
> + } else {
> + ma->flags |= old_ma.flags & ~FS_COMMON_FL;
> + }
> + err = miscattr_set_prepare(inode, &old_ma, ma);
> + if (!err)
> + err = inode->i_op->miscattr_set(dentry, ma);
So I somewhat wonder here - not all filesystems support all the xflags or
other extended attributes. Currently these would be just silently ignored
AFAICT. Which seems a bit dangerous to me - most notably because it makes
future extensions of these filesystems difficult. So how are we going to go
about this? Is every filesystem supposed to check what it supports and
refuse other stuff (but currently e.g. your ext2 conversion patch doesn't do
that AFAICT)? Shouldn't we make things easier for filesystems to provide a
bitmask of changing fields (instead of flags / xflags bools) so that they
can refuse unsupported stuff with a single mask check?
To make things more complex, ext2/4 has traditionally silently cleared
unknown flags for setflags but not for setxflags. Unlike e.g. XFS which
refuses unknown flags.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH 01/18] vfs: add miscattr ops
2021-02-03 15:04 ` Jan Kara
@ 2021-02-03 15:21 ` Miklos Szeredi
0 siblings, 0 replies; 40+ messages in thread
From: Miklos Szeredi @ 2021-02-03 15:21 UTC (permalink / raw)
To: Jan Kara
Cc: linux-fsdevel, lkml, Al Viro, Andreas Dilger,
Andreas Gruenbacher, Christoph Hellwig, Darrick J . Wong,
Dave Kleikamp, David Sterba, Jaegeuk Kim, Joel Becker,
Matthew Garrett, Mike Marshall, Richard Weinberger,
Ryusuke Konishi, Theodore Ts'o, Tyler Hicks
On Wed, Feb 3, 2021 at 4:05 PM Jan Kara <jack@suse.cz> wrote:
[...]
> > +/**
> > + * miscattr_fill_xflags - initialize miscattr with xflags
> > + * @ma: miscattr pointer
> > + * @xflags: FS_XFLAG_* flags
> > + *
> > + * Set ->fsx_xflags, ->xattr_valid and ->flags (translated xflags). All
> > + * other fields are zeroed.
> > + */
> > +void miscattr_fill_xflags(struct miscattr *ma, u32 xflags)
>
> Maybe call this miscattr_fill_from_xflags() and the next function
> miscattr_fill_from_flags()? At least to me it would be clearer when I want
> to use which function just by looking at the name...
Yes, more clarity for the cost of a longer name. I'm not sure...
[...]
> > +/**
> > + * vfs_miscattr_get - retrieve miscellaneous inode attributes
> > + * @dentry: the object to retrieve from
> > + * @ma: miscattr pointer
> > + *
> > + * Call i_op->miscattr_get() callback, if exists.
> > + *
> > + * Returns 0 on success, or a negative error on failure.
> > + */
> > +int vfs_miscattr_get(struct dentry *dentry, struct miscattr *ma)
> > +{
> > + struct inode *inode = d_inode(dentry);
> > +
> > + if (d_is_special(dentry))
> > + return -ENOTTY;
> > +
> > + if (!inode->i_op->miscattr_get)
> > + return -ENOIOCTLCMD;
> > +
> > + memset(ma, 0, sizeof(*ma));
>
> So here we clear whole 'ma' but callers already set e.g. xattr_valid field
> and cleared the 'ma' as well which just looks silly...
Well spotted. Fixed.
[...]
> > +/**
> > + * vfs_miscattr_set - change miscellaneous inode attributes
> > + * @dentry: the object to change
> > + * @ma: miscattr pointer
> > + *
> > + * After verifying permissions, call i_op->miscattr_set() callback, if
> > + * exists.
> > + *
> > + * Verifying attributes involves retrieving current attributes with
> > + * i_op->miscattr_get(), this also allows initilaizing attributes that have
> > + * not been set by the caller to current values. Inode lock is held
> > + * thoughout to prevent racing with another instance.
> > + *
> > + * Returns 0 on success, or a negative error on failure.
> > + */
> > +int vfs_miscattr_set(struct dentry *dentry, struct miscattr *ma)
> > +{
> > + struct inode *inode = d_inode(dentry);
> > + struct miscattr old_ma = {};
> > + int err;
> > +
> > + if (d_is_special(dentry))
> > + return -ENOTTY;
> > +
> > + if (!inode->i_op->miscattr_set)
> > + return -ENOIOCTLCMD;
> > +
> > + if (!inode_owner_or_capable(inode))
> > + return -EPERM;
> > +
> > + inode_lock(inode);
> > + err = vfs_miscattr_get(dentry, &old_ma);
> > + if (!err) {
> > + /* initialize missing bits from old_ma */
> > + if (ma->flags_valid) {
> > + ma->fsx_xflags |= old_ma.fsx_xflags & ~FS_XFLAG_COMMON;
> > + ma->fsx_extsize = old_ma.fsx_extsize;
> > + ma->fsx_nextents = old_ma.fsx_nextents;
> > + ma->fsx_projid = old_ma.fsx_projid;
> > + ma->fsx_cowextsize = old_ma.fsx_cowextsize;
> > + } else {
> > + ma->flags |= old_ma.flags & ~FS_COMMON_FL;
> > + }
> > + err = miscattr_set_prepare(inode, &old_ma, ma);
> > + if (!err)
> > + err = inode->i_op->miscattr_set(dentry, ma);
>
> So I somewhat wonder here - not all filesystems support all the xflags or
> other extended attributes. Currently these would be just silently ignored
> AFAICT. Which seems a bit dangerous to me - most notably because it makes
> future extensions of these filesystems difficult. So how are we going to go
> about this? Is every filesystem supposed to check what it supports and
> refuse other stuff (but currently e.g. your ext2 conversion patch doesn't do
> that AFAICT)? Shouldn't we make things easier for filesystems to provide a
> bitmask of changing fields (instead of flags / xflags bools) so that they
> can refuse unsupported stuff with a single mask check?
Ah, ext2 one is missing miscattr_has_xattr() check and doesn't use the
miscattr_fill_flags() helper. It was one of the earlier fs I
converted, and the API wasn't so refined then. Fixed.
Will review all conversions too for this type of omission.
Creating a mask instead of bool makes sense, I'll look into this.
> To make things more complex, ext2/4 has traditionally silently cleared
> unknown flags for setflags but not for setxflags. Unlike e.g. XFS which
> refuses unknown flags.
Right. Not sure if this can be fixed. Documenting rules and
exceptions should be a first step.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 01/18] vfs: add miscattr ops
2021-02-03 12:40 ` [PATCH 01/18] vfs: add miscattr ops Miklos Szeredi
2021-02-03 15:04 ` Jan Kara
@ 2021-03-23 0:23 ` Eric Biggers
2021-03-25 14:52 ` Miklos Szeredi
1 sibling, 1 reply; 40+ messages in thread
From: Eric Biggers @ 2021-03-23 0:23 UTC (permalink / raw)
To: Miklos Szeredi
Cc: linux-fsdevel, linux-kernel, Al Viro, Andreas Dilger,
Andreas Gruenbacher, Christoph Hellwig, Darrick J . Wong,
Dave Kleikamp, David Sterba, Jaegeuk Kim, Jan Kara, Joel Becker,
Matthew Garrett, Mike Marshall, Richard Weinberger,
Ryusuke Konishi, Theodore Ts'o, Tyler Hicks
On Wed, Feb 03, 2021 at 01:40:55PM +0100, Miklos Szeredi wrote:
> + * Verifying attributes involves retrieving current attributes with
> + * i_op->miscattr_get(), this also allows initilaizing attributes that have
initilaizing => initializing
> +int vfs_miscattr_set(struct dentry *dentry, struct miscattr *ma)
> +{
> + struct inode *inode = d_inode(dentry);
> + struct miscattr old_ma = {};
> + int err;
> +
> + if (d_is_special(dentry))
> + return -ENOTTY;
> +
> + if (!inode->i_op->miscattr_set)
> + return -ENOIOCTLCMD;
> +
> + if (!inode_owner_or_capable(inode))
> + return -EPERM;
Shouldn't this be EACCES, not EPERM?
> +/**
> + * miscattr_has_xattr - check for extentended flags/attributes
extentended => extended
- Eric
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH 01/18] vfs: add miscattr ops
2021-03-23 0:23 ` Eric Biggers
@ 2021-03-25 14:52 ` Miklos Szeredi
2021-03-25 15:17 ` Al Viro
0 siblings, 1 reply; 40+ messages in thread
From: Miklos Szeredi @ 2021-03-25 14:52 UTC (permalink / raw)
To: Eric Biggers
Cc: Miklos Szeredi, linux-fsdevel, linux-kernel, Al Viro,
Andreas Dilger, Andreas Gruenbacher, Christoph Hellwig,
Darrick J . Wong, Dave Kleikamp, David Sterba, Jaegeuk Kim,
Jan Kara, Joel Becker, Matthew Garrett, Mike Marshall,
Richard Weinberger, Ryusuke Konishi, Theodore Ts'o,
Tyler Hicks
On Tue, Mar 23, 2021 at 1:24 AM Eric Biggers <ebiggers@kernel.org> wrote:
> > +int vfs_miscattr_set(struct dentry *dentry, struct miscattr *ma)
> > +{
> > + struct inode *inode = d_inode(dentry);
> > + struct miscattr old_ma = {};
> > + int err;
> > +
> > + if (d_is_special(dentry))
> > + return -ENOTTY;
> > +
> > + if (!inode->i_op->miscattr_set)
> > + return -ENOIOCTLCMD;
> > +
> > + if (!inode_owner_or_capable(inode))
> > + return -EPERM;
>
> Shouldn't this be EACCES, not EPERM?
$ git diff master.. | grep -C1 inode_owner_or_capable | grep
"^-.*\(EPERM\|EACCES\)" | cut -d- -f3 | sort | uniq -c
12 EACCES;
4 EPERM;
So EACCES would win if this was a democracy. However:
"[EACCES]
Permission denied. An attempt was made to access a file in a way
forbidden by its file access permissions."
"[EPERM]
Operation not permitted. An attempt was made to perform an operation
limited to processes with appropriate privileges or to the owner of a
file or other resource."
The EPERM description matches the semantics of
inode_owner_or_capable() exactly. It's a pretty clear choice.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH 01/18] vfs: add miscattr ops
2021-03-25 14:52 ` Miklos Szeredi
@ 2021-03-25 15:17 ` Al Viro
0 siblings, 0 replies; 40+ messages in thread
From: Al Viro @ 2021-03-25 15:17 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Eric Biggers, Miklos Szeredi, linux-fsdevel, linux-kernel,
Andreas Dilger, Andreas Gruenbacher, Christoph Hellwig,
Darrick J . Wong, Dave Kleikamp, David Sterba, Jaegeuk Kim,
Jan Kara, Joel Becker, Matthew Garrett, Mike Marshall,
Richard Weinberger, Ryusuke Konishi, Theodore Ts'o,
Tyler Hicks
On Thu, Mar 25, 2021 at 03:52:28PM +0100, Miklos Szeredi wrote:
> On Tue, Mar 23, 2021 at 1:24 AM Eric Biggers <ebiggers@kernel.org> wrote:
>
> > > +int vfs_miscattr_set(struct dentry *dentry, struct miscattr *ma)
> > > +{
> > > + struct inode *inode = d_inode(dentry);
> > > + struct miscattr old_ma = {};
> > > + int err;
> > > +
> > > + if (d_is_special(dentry))
> > > + return -ENOTTY;
> > > +
> > > + if (!inode->i_op->miscattr_set)
> > > + return -ENOIOCTLCMD;
> > > +
> > > + if (!inode_owner_or_capable(inode))
> > > + return -EPERM;
> >
> > Shouldn't this be EACCES, not EPERM?
>
> $ git diff master.. | grep -C1 inode_owner_or_capable | grep
> "^-.*\(EPERM\|EACCES\)" | cut -d- -f3 | sort | uniq -c
> 12 EACCES;
> 4 EPERM;
>
> So EACCES would win if this was a democracy. However:
>
> "[EACCES]
> Permission denied. An attempt was made to access a file in a way
> forbidden by its file access permissions."
>
> "[EPERM]
> Operation not permitted. An attempt was made to perform an operation
> limited to processes with appropriate privileges or to the owner of a
> file or other resource."
>
> The EPERM description matches the semantics of
> inode_owner_or_capable() exactly. It's a pretty clear choice.
Except that existing implementation (e.g. for ext2) gives -EACCES here...
OTOH, EPERM matches the behaviour of chown(2), as well as that of
*BSD chflags(2), which is the best match to functionality (setting and
clearing immutable/append-only/etc.)
So I'd probably go with EPERM, and watched for userland breakage;
if something *does* rely upon the historical EACCES here, we might
have to restore that.
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 02/18] ecryptfs: stack miscattr ops
2021-02-03 12:40 [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR Miklos Szeredi
2021-02-03 12:40 ` [PATCH 01/18] vfs: add miscattr ops Miklos Szeredi
@ 2021-02-03 12:40 ` Miklos Szeredi
2021-02-03 12:40 ` [PATCH 03/18] ovl: stack miscattr Miklos Szeredi
` (16 subsequent siblings)
18 siblings, 0 replies; 40+ messages in thread
From: Miklos Szeredi @ 2021-02-03 12:40 UTC (permalink / raw)
To: linux-fsdevel; +Cc: linux-kernel, Tyler Hicks
Add stacking for the miscattr operations.
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Cc: Tyler Hicks <code@tyhicks.com>
---
fs/ecryptfs/inode.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index 58d0f7187997..6ed24fc0e5f3 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -18,6 +18,7 @@
#include <linux/fs_stack.h>
#include <linux/slab.h>
#include <linux/xattr.h>
+#include <linux/miscattr.h>
#include <asm/unaligned.h>
#include "ecryptfs_kernel.h"
@@ -1097,6 +1098,22 @@ static int ecryptfs_removexattr(struct dentry *dentry, struct inode *inode,
return rc;
}
+static int ecryptfs_miscattr_get(struct dentry *dentry, struct miscattr *ma)
+{
+ return vfs_miscattr_get(ecryptfs_dentry_to_lower(dentry), ma);
+}
+
+static int ecryptfs_miscattr_set(struct dentry *dentry, struct miscattr *ma)
+{
+ struct dentry *lower_dentry = ecryptfs_dentry_to_lower(dentry);
+ int rc;
+
+ rc = vfs_miscattr_set(lower_dentry, ma);
+ fsstack_copy_attr_all(d_inode(dentry), d_inode(lower_dentry));
+
+ return rc;
+}
+
const struct inode_operations ecryptfs_symlink_iops = {
.get_link = ecryptfs_get_link,
.permission = ecryptfs_permission,
@@ -1118,6 +1135,8 @@ const struct inode_operations ecryptfs_dir_iops = {
.permission = ecryptfs_permission,
.setattr = ecryptfs_setattr,
.listxattr = ecryptfs_listxattr,
+ .miscattr_get = ecryptfs_miscattr_get,
+ .miscattr_set = ecryptfs_miscattr_set,
};
const struct inode_operations ecryptfs_main_iops = {
@@ -1125,6 +1144,8 @@ const struct inode_operations ecryptfs_main_iops = {
.setattr = ecryptfs_setattr,
.getattr = ecryptfs_getattr,
.listxattr = ecryptfs_listxattr,
+ .miscattr_get = ecryptfs_miscattr_get,
+ .miscattr_set = ecryptfs_miscattr_set,
};
static int ecryptfs_xattr_get(const struct xattr_handler *handler,
--
2.26.2
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 03/18] ovl: stack miscattr
2021-02-03 12:40 [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR Miklos Szeredi
2021-02-03 12:40 ` [PATCH 01/18] vfs: add miscattr ops Miklos Szeredi
2021-02-03 12:40 ` [PATCH 02/18] ecryptfs: stack " Miklos Szeredi
@ 2021-02-03 12:40 ` Miklos Szeredi
2021-02-04 23:42 ` Vivek Goyal
2021-02-03 12:40 ` [PATCH 04/18] btrfs: convert to miscattr Miklos Szeredi
` (15 subsequent siblings)
18 siblings, 1 reply; 40+ messages in thread
From: Miklos Szeredi @ 2021-02-03 12:40 UTC (permalink / raw)
To: linux-fsdevel; +Cc: linux-kernel
Add stacking for the miscattr operations.
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
fs/overlayfs/dir.c | 2 ++
fs/overlayfs/inode.c | 43 ++++++++++++++++++++++++++++++++++++++++
fs/overlayfs/overlayfs.h | 2 ++
3 files changed, 47 insertions(+)
diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 28a075b5f5b2..77c6b44f8d83 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -1300,4 +1300,6 @@ const struct inode_operations ovl_dir_inode_operations = {
.listxattr = ovl_listxattr,
.get_acl = ovl_get_acl,
.update_time = ovl_update_time,
+ .miscattr_get = ovl_miscattr_get,
+ .miscattr_set = ovl_miscattr_set,
};
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index d739e14c6814..97d36d1f28c3 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -11,6 +11,7 @@
#include <linux/posix_acl.h>
#include <linux/ratelimit.h>
#include <linux/fiemap.h>
+#include <linux/miscattr.h>
#include "overlayfs.h"
@@ -495,6 +496,46 @@ static int ovl_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
return err;
}
+int ovl_miscattr_set(struct dentry *dentry, struct miscattr *ma)
+{
+ struct inode *inode = d_inode(dentry);
+ struct dentry *upperdentry;
+ const struct cred *old_cred;
+ int err;
+
+ err = ovl_want_write(dentry);
+ if (err)
+ goto out;
+
+ err = ovl_copy_up(dentry);
+ if (!err) {
+ upperdentry = ovl_dentry_upper(dentry);
+
+ old_cred = ovl_override_creds(inode->i_sb);
+ /* err = security_file_ioctl(real.file, cmd, arg); */
+ err = vfs_miscattr_set(upperdentry, ma);
+ revert_creds(old_cred);
+ ovl_copyflags(ovl_inode_real(inode), inode);
+ }
+ ovl_drop_write(dentry);
+out:
+ return err;
+}
+
+int ovl_miscattr_get(struct dentry *dentry, struct miscattr *ma)
+{
+ struct inode *inode = d_inode(dentry);
+ struct dentry *realdentry = ovl_dentry_real(dentry);
+ const struct cred *old_cred;
+ int err;
+
+ old_cred = ovl_override_creds(inode->i_sb);
+ err = vfs_miscattr_get(realdentry, ma);
+ revert_creds(old_cred);
+
+ return err;
+}
+
static const struct inode_operations ovl_file_inode_operations = {
.setattr = ovl_setattr,
.permission = ovl_permission,
@@ -503,6 +544,8 @@ static const struct inode_operations ovl_file_inode_operations = {
.get_acl = ovl_get_acl,
.update_time = ovl_update_time,
.fiemap = ovl_fiemap,
+ .miscattr_get = ovl_miscattr_get,
+ .miscattr_set = ovl_miscattr_set,
};
static const struct inode_operations ovl_symlink_inode_operations = {
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index b487e48c7fd4..d3ad02c34cca 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -509,6 +509,8 @@ int __init ovl_aio_request_cache_init(void);
void ovl_aio_request_cache_destroy(void);
long ovl_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
long ovl_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
+int ovl_miscattr_get(struct dentry *dentry, struct miscattr *ma);
+int ovl_miscattr_set(struct dentry *dentry, struct miscattr *ma);
/* copy_up.c */
int ovl_copy_up(struct dentry *dentry);
--
2.26.2
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH 03/18] ovl: stack miscattr
2021-02-03 12:40 ` [PATCH 03/18] ovl: stack miscattr Miklos Szeredi
@ 2021-02-04 23:42 ` Vivek Goyal
2021-02-05 15:25 ` Miklos Szeredi
0 siblings, 1 reply; 40+ messages in thread
From: Vivek Goyal @ 2021-02-04 23:42 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: linux-fsdevel, linux-kernel
On Wed, Feb 03, 2021 at 01:40:57PM +0100, Miklos Szeredi wrote:
> Add stacking for the miscattr operations.
>
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
> fs/overlayfs/dir.c | 2 ++
> fs/overlayfs/inode.c | 43 ++++++++++++++++++++++++++++++++++++++++
> fs/overlayfs/overlayfs.h | 2 ++
> 3 files changed, 47 insertions(+)
>
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index 28a075b5f5b2..77c6b44f8d83 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -1300,4 +1300,6 @@ const struct inode_operations ovl_dir_inode_operations = {
> .listxattr = ovl_listxattr,
> .get_acl = ovl_get_acl,
> .update_time = ovl_update_time,
> + .miscattr_get = ovl_miscattr_get,
> + .miscattr_set = ovl_miscattr_set,
> };
> diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
> index d739e14c6814..97d36d1f28c3 100644
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -11,6 +11,7 @@
> #include <linux/posix_acl.h>
> #include <linux/ratelimit.h>
> #include <linux/fiemap.h>
> +#include <linux/miscattr.h>
> #include "overlayfs.h"
>
>
> @@ -495,6 +496,46 @@ static int ovl_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> return err;
> }
>
> +int ovl_miscattr_set(struct dentry *dentry, struct miscattr *ma)
> +{
> + struct inode *inode = d_inode(dentry);
> + struct dentry *upperdentry;
> + const struct cred *old_cred;
> + int err;
> +
> + err = ovl_want_write(dentry);
> + if (err)
> + goto out;
> +
> + err = ovl_copy_up(dentry);
> + if (!err) {
> + upperdentry = ovl_dentry_upper(dentry);
> +
> + old_cred = ovl_override_creds(inode->i_sb);
> + /* err = security_file_ioctl(real.file, cmd, arg); */
Is this an comment intended?
Vivek
> + err = vfs_miscattr_set(upperdentry, ma);
> + revert_creds(old_cred);
> + ovl_copyflags(ovl_inode_real(inode), inode);
> + }
> + ovl_drop_write(dentry);
> +out:
> + return err;
> +}
> +
> +int ovl_miscattr_get(struct dentry *dentry, struct miscattr *ma)
> +{
> + struct inode *inode = d_inode(dentry);
> + struct dentry *realdentry = ovl_dentry_real(dentry);
> + const struct cred *old_cred;
> + int err;
> +
> + old_cred = ovl_override_creds(inode->i_sb);
> + err = vfs_miscattr_get(realdentry, ma);
> + revert_creds(old_cred);
> +
> + return err;
> +}
> +
> static const struct inode_operations ovl_file_inode_operations = {
> .setattr = ovl_setattr,
> .permission = ovl_permission,
> @@ -503,6 +544,8 @@ static const struct inode_operations ovl_file_inode_operations = {
> .get_acl = ovl_get_acl,
> .update_time = ovl_update_time,
> .fiemap = ovl_fiemap,
> + .miscattr_get = ovl_miscattr_get,
> + .miscattr_set = ovl_miscattr_set,
> };
>
> static const struct inode_operations ovl_symlink_inode_operations = {
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index b487e48c7fd4..d3ad02c34cca 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -509,6 +509,8 @@ int __init ovl_aio_request_cache_init(void);
> void ovl_aio_request_cache_destroy(void);
> long ovl_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
> long ovl_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
> +int ovl_miscattr_get(struct dentry *dentry, struct miscattr *ma);
> +int ovl_miscattr_set(struct dentry *dentry, struct miscattr *ma);
>
> /* copy_up.c */
> int ovl_copy_up(struct dentry *dentry);
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH 03/18] ovl: stack miscattr
2021-02-04 23:42 ` Vivek Goyal
@ 2021-02-05 15:25 ` Miklos Szeredi
2021-02-05 15:28 ` Miklos Szeredi
0 siblings, 1 reply; 40+ messages in thread
From: Miklos Szeredi @ 2021-02-05 15:25 UTC (permalink / raw)
To: Vivek Goyal; +Cc: Miklos Szeredi, linux-fsdevel, linux-kernel
On Fri, Feb 5, 2021 at 12:49 AM Vivek Goyal <vgoyal@redhat.com> wrote:
> > +int ovl_miscattr_set(struct dentry *dentry, struct miscattr *ma)
> > +{
> > + struct inode *inode = d_inode(dentry);
> > + struct dentry *upperdentry;
> > + const struct cred *old_cred;
> > + int err;
> > +
> > + err = ovl_want_write(dentry);
> > + if (err)
> > + goto out;
> > +
> > + err = ovl_copy_up(dentry);
> > + if (!err) {
> > + upperdentry = ovl_dentry_upper(dentry);
> > +
> > + old_cred = ovl_override_creds(inode->i_sb);
> > + /* err = security_file_ioctl(real.file, cmd, arg); */
>
> Is this an comment intended?
I don't remember, but I guess not. Will fix and test.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH 03/18] ovl: stack miscattr
2021-02-05 15:25 ` Miklos Szeredi
@ 2021-02-05 15:28 ` Miklos Szeredi
0 siblings, 0 replies; 40+ messages in thread
From: Miklos Szeredi @ 2021-02-05 15:28 UTC (permalink / raw)
To: Vivek Goyal; +Cc: Miklos Szeredi, linux-fsdevel, linux-kernel
On Fri, Feb 5, 2021 at 4:25 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Fri, Feb 5, 2021 at 12:49 AM Vivek Goyal <vgoyal@redhat.com> wrote:
>
> > > +int ovl_miscattr_set(struct dentry *dentry, struct miscattr *ma)
> > > +{
> > > + struct inode *inode = d_inode(dentry);
> > > + struct dentry *upperdentry;
> > > + const struct cred *old_cred;
> > > + int err;
> > > +
> > > + err = ovl_want_write(dentry);
> > > + if (err)
> > > + goto out;
> > > +
> > > + err = ovl_copy_up(dentry);
> > > + if (!err) {
> > > + upperdentry = ovl_dentry_upper(dentry);
> > > +
> > > + old_cred = ovl_override_creds(inode->i_sb);
> > > + /* err = security_file_ioctl(real.file, cmd, arg); */
> >
> > Is this an comment intended?
>
> I don't remember, but I guess not. Will fix and test.
Sorry, yes, problem is that there's no file pointer available at this point.
Fix is probably to introduce security_inode_miscattr_perm() hook.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 04/18] btrfs: convert to miscattr
2021-02-03 12:40 [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR Miklos Szeredi
` (2 preceding siblings ...)
2021-02-03 12:40 ` [PATCH 03/18] ovl: stack miscattr Miklos Szeredi
@ 2021-02-03 12:40 ` Miklos Szeredi
2021-02-03 12:40 ` [PATCH 05/18] ext2: " Miklos Szeredi
` (14 subsequent siblings)
18 siblings, 0 replies; 40+ messages in thread
From: Miklos Szeredi @ 2021-02-03 12:40 UTC (permalink / raw)
To: linux-fsdevel; +Cc: linux-kernel, David Sterba
Use the miscattr API to let the VFS handle locking, permission checking and
conversion.
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Cc: David Sterba <dsterba@suse.com>
---
fs/btrfs/ctree.h | 2 +
fs/btrfs/inode.c | 4 +
fs/btrfs/ioctl.c | 248 +++++++++--------------------------------------
3 files changed, 50 insertions(+), 204 deletions(-)
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 4debdbdde2ab..c8e48ed4ea70 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3177,6 +3177,8 @@ void btrfs_update_inode_bytes(struct btrfs_inode *inode,
/* ioctl.c */
long btrfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
long btrfs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
+int btrfs_miscattr_get(struct dentry *dentry, struct miscattr *ma);
+int btrfs_miscattr_set(struct dentry *dentry, struct miscattr *ma);
int btrfs_ioctl_get_supported_features(void __user *arg);
void btrfs_sync_inode_flags_to_i_flags(struct inode *inode);
int __pure btrfs_is_empty_uuid(u8 *uuid);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index a8e0a6b038d3..048044f4c44b 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -10348,6 +10348,8 @@ static const struct inode_operations btrfs_dir_inode_operations = {
.set_acl = btrfs_set_acl,
.update_time = btrfs_update_time,
.tmpfile = btrfs_tmpfile,
+ .miscattr_get = btrfs_miscattr_get,
+ .miscattr_set = btrfs_miscattr_set,
};
static const struct file_operations btrfs_dir_file_operations = {
@@ -10401,6 +10403,8 @@ static const struct inode_operations btrfs_file_inode_operations = {
.get_acl = btrfs_get_acl,
.set_acl = btrfs_set_acl,
.update_time = btrfs_update_time,
+ .miscattr_get = btrfs_miscattr_get,
+ .miscattr_set = btrfs_miscattr_set,
};
static const struct inode_operations btrfs_special_inode_operations = {
.getattr = btrfs_getattr,
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index dde49a791f3e..d40cd719a013 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -26,6 +26,7 @@
#include <linux/btrfs.h>
#include <linux/uaccess.h>
#include <linux/iversion.h>
+#include <linux/miscattr.h>
#include "ctree.h"
#include "disk-io.h"
#include "export.h"
@@ -153,16 +154,6 @@ void btrfs_sync_inode_flags_to_i_flags(struct inode *inode)
new_fl);
}
-static int btrfs_ioctl_getflags(struct file *file, void __user *arg)
-{
- struct btrfs_inode *binode = BTRFS_I(file_inode(file));
- unsigned int flags = btrfs_inode_flags_to_fsflags(binode->flags);
-
- if (copy_to_user(arg, &flags, sizeof(flags)))
- return -EFAULT;
- return 0;
-}
-
/*
* Check if @flags are a supported and valid set of FS_*_FL flags and that
* the old and new flags are not conflicting
@@ -201,9 +192,33 @@ static int check_fsflags_compatible(struct btrfs_fs_info *fs_info,
return 0;
}
-static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
+bool btrfs_exclop_start(struct btrfs_fs_info *fs_info,
+ enum btrfs_exclusive_operation type)
{
- struct inode *inode = file_inode(file);
+ return !cmpxchg(&fs_info->exclusive_operation, BTRFS_EXCLOP_NONE, type);
+}
+
+void btrfs_exclop_finish(struct btrfs_fs_info *fs_info)
+{
+ WRITE_ONCE(fs_info->exclusive_operation, BTRFS_EXCLOP_NONE);
+ sysfs_notify(&fs_info->fs_devices->fsid_kobj, NULL, "exclusive_operation");
+}
+
+/*
+ * Set flags/xflags from the internal inode flags. The remaining items of
+ * fsxattr are zeroed.
+ */
+int btrfs_miscattr_get(struct dentry *dentry, struct miscattr *ma)
+{
+ struct btrfs_inode *binode = BTRFS_I(d_inode(dentry));
+
+ miscattr_fill_flags(ma, btrfs_inode_flags_to_fsflags(binode->flags));
+ return 0;
+}
+
+int btrfs_miscattr_set(struct dentry *dentry, struct miscattr *ma)
+{
+ struct inode *inode = d_inode(dentry);
struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
struct btrfs_inode *binode = BTRFS_I(inode);
struct btrfs_root *root = binode->root;
@@ -213,34 +228,21 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
const char *comp = NULL;
u32 binode_flags;
- if (!inode_owner_or_capable(inode))
- return -EPERM;
-
if (btrfs_root_readonly(root))
return -EROFS;
- if (copy_from_user(&fsflags, arg, sizeof(fsflags)))
- return -EFAULT;
-
- ret = mnt_want_write_file(file);
- if (ret)
- return ret;
+ if (miscattr_has_xattr(ma))
+ return -EOPNOTSUPP;
- inode_lock(inode);
- fsflags = btrfs_mask_fsflags_for_type(inode, fsflags);
+ fsflags = btrfs_mask_fsflags_for_type(inode, ma->flags);
old_fsflags = btrfs_inode_flags_to_fsflags(binode->flags);
-
- ret = vfs_ioc_setflags_prepare(inode, old_fsflags, fsflags);
- if (ret)
- goto out_unlock;
-
ret = check_fsflags(old_fsflags, fsflags);
if (ret)
- goto out_unlock;
+ return ret;
ret = check_fsflags_compatible(fs_info, fsflags);
if (ret)
- goto out_unlock;
+ return ret;
binode_flags = binode->flags;
if (fsflags & FS_SYNC_FL)
@@ -263,6 +265,13 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
binode_flags |= BTRFS_INODE_NOATIME;
else
binode_flags &= ~BTRFS_INODE_NOATIME;
+
+ /* if coming from FS_IOC_FSSETXATTR then skip unconverted flags */
+ if (!ma->flags_valid) {
+ trans = btrfs_start_transaction(root, 1);
+ goto update_flags;
+ }
+
if (fsflags & FS_DIRSYNC_FL)
binode_flags |= BTRFS_INODE_DIRSYNC;
else
@@ -303,10 +312,8 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
binode_flags |= BTRFS_INODE_NOCOMPRESS;
} else if (fsflags & FS_COMPR_FL) {
- if (IS_SWAPFILE(inode)) {
- ret = -ETXTBSY;
- goto out_unlock;
- }
+ if (IS_SWAPFILE(inode))
+ return -ETXTBSY;
binode_flags |= BTRFS_INODE_COMPRESS;
binode_flags &= ~BTRFS_INODE_NOCOMPRESS;
@@ -323,10 +330,8 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
* 2 for properties
*/
trans = btrfs_start_transaction(root, 3);
- if (IS_ERR(trans)) {
- ret = PTR_ERR(trans);
- goto out_unlock;
- }
+ if (IS_ERR(trans))
+ return PTR_ERR(trans);
if (comp) {
ret = btrfs_set_prop(trans, inode, "btrfs.compression", comp,
@@ -344,6 +349,7 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
}
}
+update_flags:
binode->flags = binode_flags;
btrfs_sync_inode_flags_to_i_flags(inode);
inode_inc_iversion(inode);
@@ -352,158 +358,6 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
out_end_trans:
btrfs_end_transaction(trans);
- out_unlock:
- inode_unlock(inode);
- mnt_drop_write_file(file);
- return ret;
-}
-
-/*
- * Translate btrfs internal inode flags to xflags as expected by the
- * FS_IOC_FSGETXATT ioctl. Filter only the supported ones, unknown flags are
- * silently dropped.
- */
-static unsigned int btrfs_inode_flags_to_xflags(unsigned int flags)
-{
- unsigned int xflags = 0;
-
- if (flags & BTRFS_INODE_APPEND)
- xflags |= FS_XFLAG_APPEND;
- if (flags & BTRFS_INODE_IMMUTABLE)
- xflags |= FS_XFLAG_IMMUTABLE;
- if (flags & BTRFS_INODE_NOATIME)
- xflags |= FS_XFLAG_NOATIME;
- if (flags & BTRFS_INODE_NODUMP)
- xflags |= FS_XFLAG_NODUMP;
- if (flags & BTRFS_INODE_SYNC)
- xflags |= FS_XFLAG_SYNC;
-
- return xflags;
-}
-
-/* Check if @flags are a supported and valid set of FS_XFLAGS_* flags */
-static int check_xflags(unsigned int flags)
-{
- if (flags & ~(FS_XFLAG_APPEND | FS_XFLAG_IMMUTABLE | FS_XFLAG_NOATIME |
- FS_XFLAG_NODUMP | FS_XFLAG_SYNC))
- return -EOPNOTSUPP;
- return 0;
-}
-
-bool btrfs_exclop_start(struct btrfs_fs_info *fs_info,
- enum btrfs_exclusive_operation type)
-{
- return !cmpxchg(&fs_info->exclusive_operation, BTRFS_EXCLOP_NONE, type);
-}
-
-void btrfs_exclop_finish(struct btrfs_fs_info *fs_info)
-{
- WRITE_ONCE(fs_info->exclusive_operation, BTRFS_EXCLOP_NONE);
- sysfs_notify(&fs_info->fs_devices->fsid_kobj, NULL, "exclusive_operation");
-}
-
-/*
- * Set the xflags from the internal inode flags. The remaining items of fsxattr
- * are zeroed.
- */
-static int btrfs_ioctl_fsgetxattr(struct file *file, void __user *arg)
-{
- struct btrfs_inode *binode = BTRFS_I(file_inode(file));
- struct fsxattr fa;
-
- simple_fill_fsxattr(&fa, btrfs_inode_flags_to_xflags(binode->flags));
- if (copy_to_user(arg, &fa, sizeof(fa)))
- return -EFAULT;
-
- return 0;
-}
-
-static int btrfs_ioctl_fssetxattr(struct file *file, void __user *arg)
-{
- struct inode *inode = file_inode(file);
- struct btrfs_inode *binode = BTRFS_I(inode);
- struct btrfs_root *root = binode->root;
- struct btrfs_trans_handle *trans;
- struct fsxattr fa, old_fa;
- unsigned old_flags;
- unsigned old_i_flags;
- int ret = 0;
-
- if (!inode_owner_or_capable(inode))
- return -EPERM;
-
- if (btrfs_root_readonly(root))
- return -EROFS;
-
- if (copy_from_user(&fa, arg, sizeof(fa)))
- return -EFAULT;
-
- ret = check_xflags(fa.fsx_xflags);
- if (ret)
- return ret;
-
- if (fa.fsx_extsize != 0 || fa.fsx_projid != 0 || fa.fsx_cowextsize != 0)
- return -EOPNOTSUPP;
-
- ret = mnt_want_write_file(file);
- if (ret)
- return ret;
-
- inode_lock(inode);
-
- old_flags = binode->flags;
- old_i_flags = inode->i_flags;
-
- simple_fill_fsxattr(&old_fa,
- btrfs_inode_flags_to_xflags(binode->flags));
- ret = vfs_ioc_fssetxattr_check(inode, &old_fa, &fa);
- if (ret)
- goto out_unlock;
-
- if (fa.fsx_xflags & FS_XFLAG_SYNC)
- binode->flags |= BTRFS_INODE_SYNC;
- else
- binode->flags &= ~BTRFS_INODE_SYNC;
- if (fa.fsx_xflags & FS_XFLAG_IMMUTABLE)
- binode->flags |= BTRFS_INODE_IMMUTABLE;
- else
- binode->flags &= ~BTRFS_INODE_IMMUTABLE;
- if (fa.fsx_xflags & FS_XFLAG_APPEND)
- binode->flags |= BTRFS_INODE_APPEND;
- else
- binode->flags &= ~BTRFS_INODE_APPEND;
- if (fa.fsx_xflags & FS_XFLAG_NODUMP)
- binode->flags |= BTRFS_INODE_NODUMP;
- else
- binode->flags &= ~BTRFS_INODE_NODUMP;
- if (fa.fsx_xflags & FS_XFLAG_NOATIME)
- binode->flags |= BTRFS_INODE_NOATIME;
- else
- binode->flags &= ~BTRFS_INODE_NOATIME;
-
- /* 1 item for the inode */
- trans = btrfs_start_transaction(root, 1);
- if (IS_ERR(trans)) {
- ret = PTR_ERR(trans);
- goto out_unlock;
- }
-
- btrfs_sync_inode_flags_to_i_flags(inode);
- inode_inc_iversion(inode);
- inode->i_ctime = current_time(inode);
- ret = btrfs_update_inode(trans, root, BTRFS_I(inode));
-
- btrfs_end_transaction(trans);
-
-out_unlock:
- if (ret) {
- binode->flags = old_flags;
- inode->i_flags = old_i_flags;
- }
-
- inode_unlock(inode);
- mnt_drop_write_file(file);
-
return ret;
}
@@ -4886,10 +4740,6 @@ long btrfs_ioctl(struct file *file, unsigned int
void __user *argp = (void __user *)arg;
switch (cmd) {
- case FS_IOC_GETFLAGS:
- return btrfs_ioctl_getflags(file, argp);
- case FS_IOC_SETFLAGS:
- return btrfs_ioctl_setflags(file, argp);
case FS_IOC_GETVERSION:
return btrfs_ioctl_getversion(file, argp);
case FS_IOC_GETFSLABEL:
@@ -5015,10 +4865,6 @@ long btrfs_ioctl(struct file *file, unsigned int
return btrfs_ioctl_get_features(fs_info, argp);
case BTRFS_IOC_SET_FEATURES:
return btrfs_ioctl_set_features(file, argp);
- case FS_IOC_FSGETXATTR:
- return btrfs_ioctl_fsgetxattr(file, argp);
- case FS_IOC_FSSETXATTR:
- return btrfs_ioctl_fssetxattr(file, argp);
case BTRFS_IOC_GET_SUBVOL_INFO:
return btrfs_ioctl_get_subvol_info(file, argp);
case BTRFS_IOC_GET_SUBVOL_ROOTREF:
@@ -5038,12 +4884,6 @@ long btrfs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
* handling is necessary.
*/
switch (cmd) {
- case FS_IOC32_GETFLAGS:
- cmd = FS_IOC_GETFLAGS;
- break;
- case FS_IOC32_SETFLAGS:
- cmd = FS_IOC_SETFLAGS;
- break;
case FS_IOC32_GETVERSION:
cmd = FS_IOC_GETVERSION;
break;
--
2.26.2
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 05/18] ext2: convert to miscattr
2021-02-03 12:40 [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR Miklos Szeredi
` (3 preceding siblings ...)
2021-02-03 12:40 ` [PATCH 04/18] btrfs: convert to miscattr Miklos Szeredi
@ 2021-02-03 12:40 ` Miklos Szeredi
2021-02-03 12:41 ` [PATCH 06/18] ext4: " Miklos Szeredi
` (13 subsequent siblings)
18 siblings, 0 replies; 40+ messages in thread
From: Miklos Szeredi @ 2021-02-03 12:40 UTC (permalink / raw)
To: linux-fsdevel; +Cc: linux-kernel, Jan Kara
Use the miscattr API to let the VFS handle locking, permission checking and
conversion.
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Cc: Jan Kara <jack@suse.cz>
---
fs/ext2/ext2.h | 6 ++--
fs/ext2/file.c | 2 ++
fs/ext2/ioctl.c | 85 ++++++++++++++++---------------------------------
fs/ext2/namei.c | 2 ++
4 files changed, 34 insertions(+), 61 deletions(-)
diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h
index 2a4175fbaf5e..fe06953770ad 100644
--- a/fs/ext2/ext2.h
+++ b/fs/ext2/ext2.h
@@ -283,8 +283,6 @@ static inline __u32 ext2_mask_flags(umode_t mode, __u32 flags)
/*
* ioctl commands
*/
-#define EXT2_IOC_GETFLAGS FS_IOC_GETFLAGS
-#define EXT2_IOC_SETFLAGS FS_IOC_SETFLAGS
#define EXT2_IOC_GETVERSION FS_IOC_GETVERSION
#define EXT2_IOC_SETVERSION FS_IOC_SETVERSION
#define EXT2_IOC_GETRSVSZ _IOR('f', 5, long)
@@ -293,8 +291,6 @@ static inline __u32 ext2_mask_flags(umode_t mode, __u32 flags)
/*
* ioctl commands in 32 bit emulation
*/
-#define EXT2_IOC32_GETFLAGS FS_IOC32_GETFLAGS
-#define EXT2_IOC32_SETFLAGS FS_IOC32_SETFLAGS
#define EXT2_IOC32_GETVERSION FS_IOC32_GETVERSION
#define EXT2_IOC32_SETVERSION FS_IOC32_SETVERSION
@@ -771,6 +767,8 @@ extern int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
u64 start, u64 len);
/* ioctl.c */
+extern int ext2_miscattr_get(struct dentry *dentry, struct miscattr *ma);
+extern int ext2_miscattr_set(struct dentry *dentry, struct miscattr *ma);
extern long ext2_ioctl(struct file *, unsigned int, unsigned long);
extern long ext2_compat_ioctl(struct file *, unsigned int, unsigned long);
diff --git a/fs/ext2/file.c b/fs/ext2/file.c
index 96044f5dbc0e..096c03612129 100644
--- a/fs/ext2/file.c
+++ b/fs/ext2/file.c
@@ -204,4 +204,6 @@ const struct inode_operations ext2_file_inode_operations = {
.get_acl = ext2_get_acl,
.set_acl = ext2_set_acl,
.fiemap = ext2_fiemap,
+ .miscattr_get = ext2_miscattr_get,
+ .miscattr_set = ext2_miscattr_set,
};
diff --git a/fs/ext2/ioctl.c b/fs/ext2/ioctl.c
index 32a8d10b579d..9900d9054a02 100644
--- a/fs/ext2/ioctl.c
+++ b/fs/ext2/ioctl.c
@@ -16,69 +16,46 @@
#include <linux/mount.h>
#include <asm/current.h>
#include <linux/uaccess.h>
+#include <linux/miscattr.h>
-
-long ext2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
+int ext2_miscattr_get(struct dentry *dentry, struct miscattr *ma)
{
- struct inode *inode = file_inode(filp);
- struct ext2_inode_info *ei = EXT2_I(inode);
- unsigned int flags;
- unsigned short rsv_window_size;
- int ret;
-
- ext2_debug ("cmd = %u, arg = %lu\n", cmd, arg);
+ struct ext2_inode_info *ei = EXT2_I(d_inode(dentry));
- switch (cmd) {
- case EXT2_IOC_GETFLAGS:
- flags = ei->i_flags & EXT2_FL_USER_VISIBLE;
- return put_user(flags, (int __user *) arg);
- case EXT2_IOC_SETFLAGS: {
- unsigned int oldflags;
+ ma->flags = ei->i_flags & EXT2_FL_USER_VISIBLE;
+ return 0;
+}
- ret = mnt_want_write_file(filp);
- if (ret)
- return ret;
+int ext2_miscattr_set(struct dentry *dentry, struct miscattr *ma)
+{
+ struct inode *inode = d_inode(dentry);
+ struct ext2_inode_info *ei = EXT2_I(inode);
- if (!inode_owner_or_capable(inode)) {
- ret = -EACCES;
- goto setflags_out;
- }
+ /* Is it quota file? Do not allow user to mess with it */
+ if (IS_NOQUOTA(inode))
+ return -EPERM;
- if (get_user(flags, (int __user *) arg)) {
- ret = -EFAULT;
- goto setflags_out;
- }
+ ei->i_flags = (ei->i_flags & ~EXT2_FL_USER_MODIFIABLE) |
+ (ma->flags & EXT2_FL_USER_MODIFIABLE);
- flags = ext2_mask_flags(inode->i_mode, flags);
+ ext2_set_inode_flags(inode);
+ inode->i_ctime = current_time(inode);
+ mark_inode_dirty(inode);
- inode_lock(inode);
- /* Is it quota file? Do not allow user to mess with it */
- if (IS_NOQUOTA(inode)) {
- inode_unlock(inode);
- ret = -EPERM;
- goto setflags_out;
- }
- oldflags = ei->i_flags;
+ return 0;
+}
- ret = vfs_ioc_setflags_prepare(inode, oldflags, flags);
- if (ret) {
- inode_unlock(inode);
- goto setflags_out;
- }
- flags = flags & EXT2_FL_USER_MODIFIABLE;
- flags |= oldflags & ~EXT2_FL_USER_MODIFIABLE;
- ei->i_flags = flags;
+long ext2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
+{
+ struct inode *inode = file_inode(filp);
+ struct ext2_inode_info *ei = EXT2_I(inode);
+ unsigned short rsv_window_size;
+ int ret;
- ext2_set_inode_flags(inode);
- inode->i_ctime = current_time(inode);
- inode_unlock(inode);
+ ext2_debug ("cmd = %u, arg = %lu\n", cmd, arg);
- mark_inode_dirty(inode);
-setflags_out:
- mnt_drop_write_file(filp);
- return ret;
- }
+ switch (cmd) {
case EXT2_IOC_GETVERSION:
return put_user(inode->i_generation, (int __user *) arg);
case EXT2_IOC_SETVERSION: {
@@ -163,12 +140,6 @@ long ext2_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
{
/* These are just misnamed, they actually get/put from/to user an int */
switch (cmd) {
- case EXT2_IOC32_GETFLAGS:
- cmd = EXT2_IOC_GETFLAGS;
- break;
- case EXT2_IOC32_SETFLAGS:
- cmd = EXT2_IOC_SETFLAGS;
- break;
case EXT2_IOC32_GETVERSION:
cmd = EXT2_IOC_GETVERSION;
break;
diff --git a/fs/ext2/namei.c b/fs/ext2/namei.c
index ea980f1e2e99..f1f81d07155d 100644
--- a/fs/ext2/namei.c
+++ b/fs/ext2/namei.c
@@ -421,6 +421,8 @@ const struct inode_operations ext2_dir_inode_operations = {
.get_acl = ext2_get_acl,
.set_acl = ext2_set_acl,
.tmpfile = ext2_tmpfile,
+ .miscattr_get = ext2_miscattr_get,
+ .miscattr_set = ext2_miscattr_set,
};
const struct inode_operations ext2_special_inode_operations = {
--
2.26.2
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 06/18] ext4: convert to miscattr
2021-02-03 12:40 [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR Miklos Szeredi
` (4 preceding siblings ...)
2021-02-03 12:40 ` [PATCH 05/18] ext2: " Miklos Szeredi
@ 2021-02-03 12:41 ` Miklos Szeredi
2021-02-03 12:41 ` [PATCH 07/18] f2fs: " Miklos Szeredi
` (12 subsequent siblings)
18 siblings, 0 replies; 40+ messages in thread
From: Miklos Szeredi @ 2021-02-03 12:41 UTC (permalink / raw)
To: linux-fsdevel; +Cc: linux-kernel, Theodore Ts'o
Use the miscattr API to let the VFS handle locking, permission checking and
conversion.
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Cc: "Theodore Ts'o" <tytso@mit.edu>
---
fs/ext4/ext4.h | 11 +--
fs/ext4/file.c | 2 +
fs/ext4/ioctl.c | 209 ++++++++++--------------------------------------
fs/ext4/namei.c | 2 +
4 files changed, 49 insertions(+), 175 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 2866d249f3d2..dc4f8c59f279 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -472,15 +472,6 @@ struct flex_groups {
EXT4_VERITY_FL | \
EXT4_INLINE_DATA_FL)
-/* Flags we can manipulate with through FS_IOC_FSSETXATTR */
-#define EXT4_FL_XFLAG_VISIBLE (EXT4_SYNC_FL | \
- EXT4_IMMUTABLE_FL | \
- EXT4_APPEND_FL | \
- EXT4_NODUMP_FL | \
- EXT4_NOATIME_FL | \
- EXT4_PROJINHERIT_FL | \
- EXT4_DAX_FL)
-
/* Flags that should be inherited by new inodes from their parent. */
#define EXT4_FL_INHERITED (EXT4_SECRM_FL | EXT4_UNRM_FL | EXT4_COMPR_FL |\
EXT4_SYNC_FL | EXT4_NODUMP_FL | EXT4_NOATIME_FL |\
@@ -2921,6 +2912,8 @@ extern int ext4_ind_remove_space(handle_t *handle, struct inode *inode,
/* ioctl.c */
extern long ext4_ioctl(struct file *, unsigned int, unsigned long);
extern long ext4_compat_ioctl(struct file *, unsigned int, unsigned long);
+int ext4_miscattr_set(struct dentry *dentry, struct miscattr *ma);
+int ext4_miscattr_get(struct dentry *dentry, struct miscattr *ma);
extern void ext4_reset_inode_seed(struct inode *inode);
/* migrate.c */
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 349b27f0dda0..cbc35bd1a7da 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -920,5 +920,7 @@ const struct inode_operations ext4_file_inode_operations = {
.get_acl = ext4_get_acl,
.set_acl = ext4_set_acl,
.fiemap = ext4_fiemap,
+ .miscattr_get = ext4_miscattr_get,
+ .miscattr_set = ext4_miscattr_set,
};
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index d9665d2f82db..883f39340318 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -20,6 +20,7 @@
#include <linux/uaccess.h>
#include <linux/delay.h>
#include <linux/iversion.h>
+#include <linux/miscattr.h>
#include "ext4_jbd2.h"
#include "ext4.h"
#include <linux/fsmap.h>
@@ -341,11 +342,6 @@ static int ext4_ioctl_setflags(struct inode *inode,
goto flags_out;
oldflags = ei->i_flags;
-
- err = vfs_ioc_setflags_prepare(inode, oldflags, flags);
- if (err)
- goto flags_out;
-
/*
* The JOURNAL_DATA flag can only be changed by
* the relevant capability.
@@ -456,9 +452,8 @@ static int ext4_ioctl_setflags(struct inode *inode,
}
#ifdef CONFIG_QUOTA
-static int ext4_ioctl_setproject(struct file *filp, __u32 projid)
+static int ext4_ioctl_setproject(struct inode *inode, __u32 projid)
{
- struct inode *inode = file_inode(filp);
struct super_block *sb = inode->i_sb;
struct ext4_inode_info *ei = EXT4_I(inode);
int err, rc;
@@ -542,7 +537,7 @@ static int ext4_ioctl_setproject(struct file *filp, __u32 projid)
return err;
}
#else
-static int ext4_ioctl_setproject(struct file *filp, __u32 projid)
+static int ext4_ioctl_setproject(struct inode *inode, __u32 projid)
{
if (projid != EXT4_DEF_PROJID)
return -EOPNOTSUPP;
@@ -550,56 +545,6 @@ static int ext4_ioctl_setproject(struct file *filp, __u32 projid)
}
#endif
-/* Transfer internal flags to xflags */
-static inline __u32 ext4_iflags_to_xflags(unsigned long iflags)
-{
- __u32 xflags = 0;
-
- if (iflags & EXT4_SYNC_FL)
- xflags |= FS_XFLAG_SYNC;
- if (iflags & EXT4_IMMUTABLE_FL)
- xflags |= FS_XFLAG_IMMUTABLE;
- if (iflags & EXT4_APPEND_FL)
- xflags |= FS_XFLAG_APPEND;
- if (iflags & EXT4_NODUMP_FL)
- xflags |= FS_XFLAG_NODUMP;
- if (iflags & EXT4_NOATIME_FL)
- xflags |= FS_XFLAG_NOATIME;
- if (iflags & EXT4_PROJINHERIT_FL)
- xflags |= FS_XFLAG_PROJINHERIT;
- if (iflags & EXT4_DAX_FL)
- xflags |= FS_XFLAG_DAX;
- return xflags;
-}
-
-#define EXT4_SUPPORTED_FS_XFLAGS (FS_XFLAG_SYNC | FS_XFLAG_IMMUTABLE | \
- FS_XFLAG_APPEND | FS_XFLAG_NODUMP | \
- FS_XFLAG_NOATIME | FS_XFLAG_PROJINHERIT | \
- FS_XFLAG_DAX)
-
-/* Transfer xflags flags to internal */
-static inline unsigned long ext4_xflags_to_iflags(__u32 xflags)
-{
- unsigned long iflags = 0;
-
- if (xflags & FS_XFLAG_SYNC)
- iflags |= EXT4_SYNC_FL;
- if (xflags & FS_XFLAG_IMMUTABLE)
- iflags |= EXT4_IMMUTABLE_FL;
- if (xflags & FS_XFLAG_APPEND)
- iflags |= EXT4_APPEND_FL;
- if (xflags & FS_XFLAG_NODUMP)
- iflags |= EXT4_NODUMP_FL;
- if (xflags & FS_XFLAG_NOATIME)
- iflags |= EXT4_NOATIME_FL;
- if (xflags & FS_XFLAG_PROJINHERIT)
- iflags |= EXT4_PROJINHERIT_FL;
- if (xflags & FS_XFLAG_DAX)
- iflags |= EXT4_DAX_FL;
-
- return iflags;
-}
-
static int ext4_shutdown(struct super_block *sb, unsigned long arg)
{
struct ext4_sb_info *sbi = EXT4_SB(sb);
@@ -767,15 +712,51 @@ static long ext4_ioctl_group_add(struct file *file,
return err;
}
-static void ext4_fill_fsxattr(struct inode *inode, struct fsxattr *fa)
+int ext4_miscattr_get(struct dentry *dentry, struct miscattr *ma)
{
+ struct inode *inode = d_inode(dentry);
struct ext4_inode_info *ei = EXT4_I(inode);
+ u32 flags = ei->i_flags & EXT4_FL_USER_VISIBLE;
- simple_fill_fsxattr(fa, ext4_iflags_to_xflags(ei->i_flags &
- EXT4_FL_USER_VISIBLE));
+ if (S_ISREG(inode->i_mode))
+ flags &= ~FS_PROJINHERIT_FL;
+ miscattr_fill_flags(ma, flags);
if (ext4_has_feature_project(inode->i_sb))
- fa->fsx_projid = from_kprojid(&init_user_ns, ei->i_projid);
+ ma->fsx_projid = from_kprojid(&init_user_ns, ei->i_projid);
+
+ return 0;
+}
+
+int ext4_miscattr_set(struct dentry *dentry, struct miscattr *ma)
+{
+ struct inode *inode = d_inode(dentry);
+ u32 flags = ma->flags;
+ int err = -EOPNOTSUPP;
+
+ ext4_fc_start_update(inode);
+ if (flags & ~EXT4_FL_USER_VISIBLE)
+ goto out;
+
+ /*
+ * chattr(1) grabs flags via GETFLAGS, modifies the result and
+ * passes that to SETFLAGS. So we cannot easily make SETFLAGS
+ * more restrictive than just silently masking off visible but
+ * not settable flags as we always did.
+ */
+ flags &= EXT4_FL_USER_MODIFIABLE;
+ if (ext4_mask_flags(inode->i_mode, flags) != flags)
+ goto out;
+ err = ext4_ioctl_check_immutable(inode, ma->fsx_projid, flags);
+ if (err)
+ goto out;
+ err = ext4_ioctl_setflags(inode, flags);
+ if (err)
+ goto out;
+ err = ext4_ioctl_setproject(inode, ma->fsx_projid);
+out:
+ ext4_fc_stop_update(inode);
+ return err;
}
/* So that the fiemap access checks can't overflow on 32 bit machines. */
@@ -813,54 +794,12 @@ static long __ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
{
struct inode *inode = file_inode(filp);
struct super_block *sb = inode->i_sb;
- struct ext4_inode_info *ei = EXT4_I(inode);
- unsigned int flags;
ext4_debug("cmd = %u, arg = %lu\n", cmd, arg);
switch (cmd) {
case FS_IOC_GETFSMAP:
return ext4_ioc_getfsmap(sb, (void __user *)arg);
- case FS_IOC_GETFLAGS:
- flags = ei->i_flags & EXT4_FL_USER_VISIBLE;
- if (S_ISREG(inode->i_mode))
- flags &= ~EXT4_PROJINHERIT_FL;
- return put_user(flags, (int __user *) arg);
- case FS_IOC_SETFLAGS: {
- int err;
-
- if (!inode_owner_or_capable(inode))
- return -EACCES;
-
- if (get_user(flags, (int __user *) arg))
- return -EFAULT;
-
- if (flags & ~EXT4_FL_USER_VISIBLE)
- return -EOPNOTSUPP;
- /*
- * chattr(1) grabs flags via GETFLAGS, modifies the result and
- * passes that to SETFLAGS. So we cannot easily make SETFLAGS
- * more restrictive than just silently masking off visible but
- * not settable flags as we always did.
- */
- flags &= EXT4_FL_USER_MODIFIABLE;
- if (ext4_mask_flags(inode->i_mode, flags) != flags)
- return -EOPNOTSUPP;
-
- err = mnt_want_write_file(filp);
- if (err)
- return err;
-
- inode_lock(inode);
- err = ext4_ioctl_check_immutable(inode,
- from_kprojid(&init_user_ns, ei->i_projid),
- flags);
- if (!err)
- err = ext4_ioctl_setflags(inode, flags);
- inode_unlock(inode);
- mnt_drop_write_file(filp);
- return err;
- }
case EXT4_IOC_GETVERSION:
case EXT4_IOC_GETVERSION_OLD:
return put_user(inode->i_generation, (int __user *) arg);
@@ -1242,60 +1181,6 @@ static long __ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
case EXT4_IOC_GET_ES_CACHE:
return ext4_ioctl_get_es_cache(filp, arg);
- case FS_IOC_FSGETXATTR:
- {
- struct fsxattr fa;
-
- ext4_fill_fsxattr(inode, &fa);
-
- if (copy_to_user((struct fsxattr __user *)arg,
- &fa, sizeof(fa)))
- return -EFAULT;
- return 0;
- }
- case FS_IOC_FSSETXATTR:
- {
- struct fsxattr fa, old_fa;
- int err;
-
- if (copy_from_user(&fa, (struct fsxattr __user *)arg,
- sizeof(fa)))
- return -EFAULT;
-
- /* Make sure caller has proper permission */
- if (!inode_owner_or_capable(inode))
- return -EACCES;
-
- if (fa.fsx_xflags & ~EXT4_SUPPORTED_FS_XFLAGS)
- return -EOPNOTSUPP;
-
- flags = ext4_xflags_to_iflags(fa.fsx_xflags);
- if (ext4_mask_flags(inode->i_mode, flags) != flags)
- return -EOPNOTSUPP;
-
- err = mnt_want_write_file(filp);
- if (err)
- return err;
-
- inode_lock(inode);
- ext4_fill_fsxattr(inode, &old_fa);
- err = vfs_ioc_fssetxattr_check(inode, &old_fa, &fa);
- if (err)
- goto out;
- flags = (ei->i_flags & ~EXT4_FL_XFLAG_VISIBLE) |
- (flags & EXT4_FL_XFLAG_VISIBLE);
- err = ext4_ioctl_check_immutable(inode, fa.fsx_projid, flags);
- if (err)
- goto out;
- err = ext4_ioctl_setflags(inode, flags);
- if (err)
- goto out;
- err = ext4_ioctl_setproject(filp, fa.fsx_projid);
-out:
- inode_unlock(inode);
- mnt_drop_write_file(filp);
- return err;
- }
case EXT4_IOC_SHUTDOWN:
return ext4_shutdown(sb, arg);
@@ -1330,12 +1215,6 @@ long ext4_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
{
/* These are just misnamed, they actually get/put from/to user an int */
switch (cmd) {
- case FS_IOC32_GETFLAGS:
- cmd = FS_IOC_GETFLAGS;
- break;
- case FS_IOC32_SETFLAGS:
- cmd = FS_IOC_SETFLAGS;
- break;
case EXT4_IOC32_GETVERSION:
cmd = EXT4_IOC_GETVERSION;
break;
@@ -1394,8 +1273,6 @@ long ext4_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
case EXT4_IOC_CLEAR_ES_CACHE:
case EXT4_IOC_GETSTATE:
case EXT4_IOC_GET_ES_CACHE:
- case FS_IOC_FSGETXATTR:
- case FS_IOC_FSSETXATTR:
break;
default:
return -ENOIOCTLCMD;
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index cf652ba3e74d..76dc27834322 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -4130,6 +4130,8 @@ const struct inode_operations ext4_dir_inode_operations = {
.get_acl = ext4_get_acl,
.set_acl = ext4_set_acl,
.fiemap = ext4_fiemap,
+ .miscattr_get = ext4_miscattr_get,
+ .miscattr_set = ext4_miscattr_set,
};
const struct inode_operations ext4_special_inode_operations = {
--
2.26.2
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 07/18] f2fs: convert to miscattr
2021-02-03 12:40 [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR Miklos Szeredi
` (5 preceding siblings ...)
2021-02-03 12:41 ` [PATCH 06/18] ext4: " Miklos Szeredi
@ 2021-02-03 12:41 ` Miklos Szeredi
2021-03-23 0:28 ` Eric Biggers
2021-02-03 12:41 ` [PATCH 08/18] gfs2: " Miklos Szeredi
` (11 subsequent siblings)
18 siblings, 1 reply; 40+ messages in thread
From: Miklos Szeredi @ 2021-02-03 12:41 UTC (permalink / raw)
To: linux-fsdevel; +Cc: linux-kernel, Jaegeuk Kim
Use the miscattr API to let the VFS handle locking, permission checking and
conversion.
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Cc: Jaegeuk Kim <jaegeuk@kernel.org>
---
fs/f2fs/f2fs.h | 2 +
fs/f2fs/file.c | 212 ++++++++----------------------------------------
fs/f2fs/namei.c | 2 +
3 files changed, 38 insertions(+), 178 deletions(-)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index bb11759191dc..6526516788de 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3141,6 +3141,8 @@ int f2fs_setattr(struct dentry *dentry, struct iattr *attr);
int f2fs_truncate_hole(struct inode *inode, pgoff_t pg_start, pgoff_t pg_end);
void f2fs_truncate_data_blocks_range(struct dnode_of_data *dn, int count);
int f2fs_precache_extents(struct inode *inode);
+int f2fs_miscattr_get(struct dentry *dentry, struct miscattr *ma);
+int f2fs_miscattr_set(struct dentry *dentry, struct miscattr *ma);
long f2fs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);
long f2fs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
int f2fs_transfer_project_quota(struct inode *inode, kprojid_t kprojid);
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index f585545277d7..404f989f6954 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -22,6 +22,7 @@
#include <linux/file.h>
#include <linux/nls.h>
#include <linux/sched/signal.h>
+#include <linux/miscattr.h>
#include "f2fs.h"
#include "node.h"
@@ -971,6 +972,8 @@ const struct inode_operations f2fs_file_inode_operations = {
.set_acl = f2fs_set_acl,
.listxattr = f2fs_listxattr,
.fiemap = f2fs_fiemap,
+ .miscattr_get = f2fs_miscattr_get,
+ .miscattr_set = f2fs_miscattr_set,
};
static int fill_zero(struct inode *inode, pgoff_t index,
@@ -1933,67 +1936,6 @@ static inline u32 f2fs_fsflags_to_iflags(u32 fsflags)
return iflags;
}
-static int f2fs_ioc_getflags(struct file *filp, unsigned long arg)
-{
- struct inode *inode = file_inode(filp);
- struct f2fs_inode_info *fi = F2FS_I(inode);
- u32 fsflags = f2fs_iflags_to_fsflags(fi->i_flags);
-
- if (IS_ENCRYPTED(inode))
- fsflags |= FS_ENCRYPT_FL;
- if (IS_VERITY(inode))
- fsflags |= FS_VERITY_FL;
- if (f2fs_has_inline_data(inode) || f2fs_has_inline_dentry(inode))
- fsflags |= FS_INLINE_DATA_FL;
- if (is_inode_flag_set(inode, FI_PIN_FILE))
- fsflags |= FS_NOCOW_FL;
-
- fsflags &= F2FS_GETTABLE_FS_FL;
-
- return put_user(fsflags, (int __user *)arg);
-}
-
-static int f2fs_ioc_setflags(struct file *filp, unsigned long arg)
-{
- struct inode *inode = file_inode(filp);
- struct f2fs_inode_info *fi = F2FS_I(inode);
- u32 fsflags, old_fsflags;
- u32 iflags;
- int ret;
-
- if (!inode_owner_or_capable(inode))
- return -EACCES;
-
- if (get_user(fsflags, (int __user *)arg))
- return -EFAULT;
-
- if (fsflags & ~F2FS_GETTABLE_FS_FL)
- return -EOPNOTSUPP;
- fsflags &= F2FS_SETTABLE_FS_FL;
-
- iflags = f2fs_fsflags_to_iflags(fsflags);
- if (f2fs_mask_flags(inode->i_mode, iflags) != iflags)
- return -EOPNOTSUPP;
-
- ret = mnt_want_write_file(filp);
- if (ret)
- return ret;
-
- inode_lock(inode);
-
- old_fsflags = f2fs_iflags_to_fsflags(fi->i_flags);
- ret = vfs_ioc_setflags_prepare(inode, old_fsflags, fsflags);
- if (ret)
- goto out;
-
- ret = f2fs_setflags_common(inode, iflags,
- f2fs_fsflags_to_iflags(F2FS_SETTABLE_FS_FL));
-out:
- inode_unlock(inode);
- mnt_drop_write_file(filp);
- return ret;
-}
-
static int f2fs_ioc_getversion(struct file *filp, unsigned long arg)
{
struct inode *inode = file_inode(filp);
@@ -3000,9 +2942,8 @@ int f2fs_transfer_project_quota(struct inode *inode, kprojid_t kprojid)
return err;
}
-static int f2fs_ioc_setproject(struct file *filp, __u32 projid)
+static int f2fs_ioc_setproject(struct inode *inode, __u32 projid)
{
- struct inode *inode = file_inode(filp);
struct f2fs_inode_info *fi = F2FS_I(inode);
struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
struct page *ipage;
@@ -3063,7 +3004,7 @@ int f2fs_transfer_project_quota(struct inode *inode, kprojid_t kprojid)
return 0;
}
-static int f2fs_ioc_setproject(struct file *filp, __u32 projid)
+static int f2fs_ioc_setproject(struct inode *inode, __u32 projid)
{
if (projid != F2FS_DEF_PROJID)
return -EOPNOTSUPP;
@@ -3071,123 +3012,54 @@ static int f2fs_ioc_setproject(struct file *filp, __u32 projid)
}
#endif
-/* FS_IOC_FSGETXATTR and FS_IOC_FSSETXATTR support */
-
-/*
- * To make a new on-disk f2fs i_flag gettable via FS_IOC_FSGETXATTR and settable
- * via FS_IOC_FSSETXATTR, add an entry for it to f2fs_xflags_map[], and add its
- * FS_XFLAG_* equivalent to F2FS_SUPPORTED_XFLAGS.
- */
-
-static const struct {
- u32 iflag;
- u32 xflag;
-} f2fs_xflags_map[] = {
- { F2FS_SYNC_FL, FS_XFLAG_SYNC },
- { F2FS_IMMUTABLE_FL, FS_XFLAG_IMMUTABLE },
- { F2FS_APPEND_FL, FS_XFLAG_APPEND },
- { F2FS_NODUMP_FL, FS_XFLAG_NODUMP },
- { F2FS_NOATIME_FL, FS_XFLAG_NOATIME },
- { F2FS_PROJINHERIT_FL, FS_XFLAG_PROJINHERIT },
-};
-
-#define F2FS_SUPPORTED_XFLAGS ( \
- FS_XFLAG_SYNC | \
- FS_XFLAG_IMMUTABLE | \
- FS_XFLAG_APPEND | \
- FS_XFLAG_NODUMP | \
- FS_XFLAG_NOATIME | \
- FS_XFLAG_PROJINHERIT)
-
-/* Convert f2fs on-disk i_flags to FS_IOC_FS{GET,SET}XATTR flags */
-static inline u32 f2fs_iflags_to_xflags(u32 iflags)
-{
- u32 xflags = 0;
- int i;
-
- for (i = 0; i < ARRAY_SIZE(f2fs_xflags_map); i++)
- if (iflags & f2fs_xflags_map[i].iflag)
- xflags |= f2fs_xflags_map[i].xflag;
-
- return xflags;
-}
-
-/* Convert FS_IOC_FS{GET,SET}XATTR flags to f2fs on-disk i_flags */
-static inline u32 f2fs_xflags_to_iflags(u32 xflags)
-{
- u32 iflags = 0;
- int i;
-
- for (i = 0; i < ARRAY_SIZE(f2fs_xflags_map); i++)
- if (xflags & f2fs_xflags_map[i].xflag)
- iflags |= f2fs_xflags_map[i].iflag;
-
- return iflags;
-}
-
-static void f2fs_fill_fsxattr(struct inode *inode, struct fsxattr *fa)
+int f2fs_miscattr_get(struct dentry *dentry, struct miscattr *ma)
{
+ struct inode *inode = d_inode(dentry);
struct f2fs_inode_info *fi = F2FS_I(inode);
+ u32 fsflags = f2fs_iflags_to_fsflags(fi->i_flags);
- simple_fill_fsxattr(fa, f2fs_iflags_to_xflags(fi->i_flags));
-
- if (f2fs_sb_has_project_quota(F2FS_I_SB(inode)))
- fa->fsx_projid = from_kprojid(&init_user_ns, fi->i_projid);
-}
+ if (IS_ENCRYPTED(inode))
+ fsflags |= FS_ENCRYPT_FL;
+ if (IS_VERITY(inode))
+ fsflags |= FS_VERITY_FL;
+ if (f2fs_has_inline_data(inode) || f2fs_has_inline_dentry(inode))
+ fsflags |= FS_INLINE_DATA_FL;
+ if (is_inode_flag_set(inode, FI_PIN_FILE))
+ fsflags |= FS_NOCOW_FL;
-static int f2fs_ioc_fsgetxattr(struct file *filp, unsigned long arg)
-{
- struct inode *inode = file_inode(filp);
- struct fsxattr fa;
+ miscattr_fill_flags(ma, fsflags & F2FS_GETTABLE_FS_FL);
- f2fs_fill_fsxattr(inode, &fa);
+ if (f2fs_sb_has_project_quota(F2FS_I_SB(inode)))
+ ma->fsx_projid = from_kprojid(&init_user_ns, fi->i_projid);
- if (copy_to_user((struct fsxattr __user *)arg, &fa, sizeof(fa)))
- return -EFAULT;
return 0;
}
-static int f2fs_ioc_fssetxattr(struct file *filp, unsigned long arg)
+int f2fs_miscattr_set(struct dentry *dentry, struct miscattr *ma)
{
- struct inode *inode = file_inode(filp);
- struct fsxattr fa, old_fa;
+ struct inode *inode = d_inode(dentry);
+ u32 fsflags = ma->flags, mask = F2FS_SETTABLE_FS_FL;
u32 iflags;
int err;
- if (copy_from_user(&fa, (struct fsxattr __user *)arg, sizeof(fa)))
- return -EFAULT;
-
- /* Make sure caller has proper permission */
- if (!inode_owner_or_capable(inode))
- return -EACCES;
-
- if (fa.fsx_xflags & ~F2FS_SUPPORTED_XFLAGS)
+ if (unlikely(f2fs_cp_error(F2FS_I_SB(inode))))
+ return -EIO;
+ if (!f2fs_is_checkpoint_ready(F2FS_I_SB(inode)))
+ return -ENOSPC;
+ if (fsflags & ~F2FS_GETTABLE_FS_FL)
return -EOPNOTSUPP;
+ fsflags &= F2FS_SETTABLE_FS_FL;
+ if (!ma->flags_valid)
+ mask &= FS_COMMON_FL;
- iflags = f2fs_xflags_to_iflags(fa.fsx_xflags);
+ iflags = f2fs_fsflags_to_iflags(fsflags);
if (f2fs_mask_flags(inode->i_mode, iflags) != iflags)
return -EOPNOTSUPP;
- err = mnt_want_write_file(filp);
- if (err)
- return err;
-
- inode_lock(inode);
-
- f2fs_fill_fsxattr(inode, &old_fa);
- err = vfs_ioc_fssetxattr_check(inode, &old_fa, &fa);
- if (err)
- goto out;
-
- err = f2fs_setflags_common(inode, iflags,
- f2fs_xflags_to_iflags(F2FS_SUPPORTED_XFLAGS));
- if (err)
- goto out;
+ err = f2fs_setflags_common(inode, iflags, f2fs_fsflags_to_iflags(mask));
+ if (!err)
+ err = f2fs_ioc_setproject(inode, ma->fsx_projid);
- err = f2fs_ioc_setproject(filp, fa.fsx_projid);
-out:
- inode_unlock(inode);
- mnt_drop_write_file(filp);
return err;
}
@@ -4204,10 +4076,6 @@ static int f2fs_ioc_compress_file(struct file *filp, unsigned long arg)
static long __f2fs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
{
switch (cmd) {
- case FS_IOC_GETFLAGS:
- return f2fs_ioc_getflags(filp, arg);
- case FS_IOC_SETFLAGS:
- return f2fs_ioc_setflags(filp, arg);
case FS_IOC_GETVERSION:
return f2fs_ioc_getversion(filp, arg);
case F2FS_IOC_START_ATOMIC_WRITE:
@@ -4256,10 +4124,6 @@ static long __f2fs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
return f2fs_ioc_flush_device(filp, arg);
case F2FS_IOC_GET_FEATURES:
return f2fs_ioc_get_features(filp, arg);
- case FS_IOC_FSGETXATTR:
- return f2fs_ioc_fsgetxattr(filp, arg);
- case FS_IOC_FSSETXATTR:
- return f2fs_ioc_fssetxattr(filp, arg);
case F2FS_IOC_GET_PIN_FILE:
return f2fs_ioc_get_pin_file(filp, arg);
case F2FS_IOC_SET_PIN_FILE:
@@ -4481,12 +4345,6 @@ long f2fs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
return -ENOSPC;
switch (cmd) {
- case FS_IOC32_GETFLAGS:
- cmd = FS_IOC_GETFLAGS;
- break;
- case FS_IOC32_SETFLAGS:
- cmd = FS_IOC_SETFLAGS;
- break;
case FS_IOC32_GETVERSION:
cmd = FS_IOC_GETVERSION;
break;
@@ -4515,8 +4373,6 @@ long f2fs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
case F2FS_IOC_DEFRAGMENT:
case F2FS_IOC_FLUSH_DEVICE:
case F2FS_IOC_GET_FEATURES:
- case FS_IOC_FSGETXATTR:
- case FS_IOC_FSSETXATTR:
case F2FS_IOC_GET_PIN_FILE:
case F2FS_IOC_SET_PIN_FILE:
case F2FS_IOC_PRECACHE_EXTENTS:
diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index 6edb1ab579a1..0085d038abd7 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -1316,6 +1316,8 @@ const struct inode_operations f2fs_dir_inode_operations = {
.set_acl = f2fs_set_acl,
.listxattr = f2fs_listxattr,
.fiemap = f2fs_fiemap,
+ .miscattr_get = f2fs_miscattr_get,
+ .miscattr_set = f2fs_miscattr_set,
};
const struct inode_operations f2fs_symlink_inode_operations = {
--
2.26.2
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH 07/18] f2fs: convert to miscattr
2021-02-03 12:41 ` [PATCH 07/18] f2fs: " Miklos Szeredi
@ 2021-03-23 0:28 ` Eric Biggers
0 siblings, 0 replies; 40+ messages in thread
From: Eric Biggers @ 2021-03-23 0:28 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: linux-fsdevel, linux-kernel, Jaegeuk Kim
On Wed, Feb 03, 2021 at 01:41:01PM +0100, Miklos Szeredi wrote:
> @@ -3071,123 +3012,54 @@ static int f2fs_ioc_setproject(struct file *filp, __u32 projid)
> }
> #endif
>
> -/* FS_IOC_FSGETXATTR and FS_IOC_FSSETXATTR support */
> -
> -/*
> - * To make a new on-disk f2fs i_flag gettable via FS_IOC_FSGETXATTR and settable
> - * via FS_IOC_FSSETXATTR, add an entry for it to f2fs_xflags_map[], and add its
> - * FS_XFLAG_* equivalent to F2FS_SUPPORTED_XFLAGS.
> - */
> -
> -static const struct {
> - u32 iflag;
> - u32 xflag;
> -} f2fs_xflags_map[] = {
> - { F2FS_SYNC_FL, FS_XFLAG_SYNC },
> - { F2FS_IMMUTABLE_FL, FS_XFLAG_IMMUTABLE },
> - { F2FS_APPEND_FL, FS_XFLAG_APPEND },
> - { F2FS_NODUMP_FL, FS_XFLAG_NODUMP },
> - { F2FS_NOATIME_FL, FS_XFLAG_NOATIME },
> - { F2FS_PROJINHERIT_FL, FS_XFLAG_PROJINHERIT },
> -};
There's another comment just above which talks about FS_IOC_GETFLAGS and
FS_IOC_SETFLAGS:
/* FS_IOC_GETFLAGS and FS_IOC_SETFLAGS support */
/*
* To make a new on-disk f2fs i_flag gettable via FS_IOC_GETFLAGS, add an entry
* for it to f2fs_fsflags_map[], and add its FS_*_FL equivalent to
* F2FS_GETTABLE_FS_FL. To also make it settable via FS_IOC_SETFLAGS, also add
* its FS_*_FL equivalent to F2FS_SETTABLE_FS_FL.
*/
This patch seems to make that outdated, since now both FS_IOC_[GS]ETFLAGS and
FS_IOC_[GS]ETFSXATTR are handled together.
Can you please update the comment to properly describe what's going on?
- Eric
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 08/18] gfs2: convert to miscattr
2021-02-03 12:40 [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR Miklos Szeredi
` (6 preceding siblings ...)
2021-02-03 12:41 ` [PATCH 07/18] f2fs: " Miklos Szeredi
@ 2021-02-03 12:41 ` Miklos Szeredi
2021-02-03 12:41 ` [PATCH 09/18] orangefs: " Miklos Szeredi
` (10 subsequent siblings)
18 siblings, 0 replies; 40+ messages in thread
From: Miklos Szeredi @ 2021-02-03 12:41 UTC (permalink / raw)
To: linux-fsdevel; +Cc: linux-kernel, Andreas Gruenbacher
Use the miscattr API to let the VFS handle locking, permission checking and
conversion.
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Cc: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/gfs2/file.c | 56 ++++++++++++-------------------------------------
fs/gfs2/inode.c | 4 ++++
fs/gfs2/inode.h | 2 ++
3 files changed, 19 insertions(+), 43 deletions(-)
diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index b39b339feddc..d54c07965665 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -25,6 +25,7 @@
#include <linux/dlm_plock.h>
#include <linux/delay.h>
#include <linux/backing-dev.h>
+#include <linux/miscattr.h>
#include "gfs2.h"
#include "incore.h"
@@ -153,9 +154,9 @@ static inline u32 gfs2_gfsflags_to_fsflags(struct inode *inode, u32 gfsflags)
return fsflags;
}
-static int gfs2_get_flags(struct file *filp, u32 __user *ptr)
+int gfs2_miscattr_get(struct dentry *dentry, struct miscattr *ma)
{
- struct inode *inode = file_inode(filp);
+ struct inode *inode = d_inode(dentry);
struct gfs2_inode *ip = GFS2_I(inode);
struct gfs2_holder gh;
int error;
@@ -168,8 +169,7 @@ static int gfs2_get_flags(struct file *filp, u32 __user *ptr)
fsflags = gfs2_gfsflags_to_fsflags(inode, ip->i_diskflags);
- if (put_user(fsflags, ptr))
- error = -EFAULT;
+ miscattr_fill_flags(ma, fsflags);
gfs2_glock_dq(&gh);
out_uninit:
@@ -213,33 +213,19 @@ void gfs2_set_inode_flags(struct inode *inode)
* @fsflags: The FS_* inode flags passed in
*
*/
-static int do_gfs2_set_flags(struct file *filp, u32 reqflags, u32 mask,
+static int do_gfs2_set_flags(struct inode *inode, u32 reqflags, u32 mask,
const u32 fsflags)
{
- struct inode *inode = file_inode(filp);
struct gfs2_inode *ip = GFS2_I(inode);
struct gfs2_sbd *sdp = GFS2_SB(inode);
struct buffer_head *bh;
struct gfs2_holder gh;
int error;
- u32 new_flags, flags, oldflags;
-
- error = mnt_want_write_file(filp);
- if (error)
- return error;
+ u32 new_flags, flags;
error = gfs2_glock_nq_init(ip->i_gl, LM_ST_EXCLUSIVE, 0, &gh);
if (error)
- goto out_drop_write;
-
- oldflags = gfs2_gfsflags_to_fsflags(inode, ip->i_diskflags);
- error = vfs_ioc_setflags_prepare(inode, oldflags, fsflags);
- if (error)
- goto out;
-
- error = -EACCES;
- if (!inode_owner_or_capable(inode))
- goto out;
+ return error;
error = 0;
flags = ip->i_diskflags;
@@ -252,9 +238,6 @@ static int do_gfs2_set_flags(struct file *filp, u32 reqflags, u32 mask,
goto out;
if (IS_APPEND(inode) && (new_flags & GFS2_DIF_APPENDONLY))
goto out;
- if (((new_flags ^ flags) & GFS2_DIF_IMMUTABLE) &&
- !capable(CAP_LINUX_IMMUTABLE))
- goto out;
if (!IS_IMMUTABLE(inode)) {
error = gfs2_permission(inode, MAY_WRITE);
if (error)
@@ -291,20 +274,18 @@ static int do_gfs2_set_flags(struct file *filp, u32 reqflags, u32 mask,
gfs2_trans_end(sdp);
out:
gfs2_glock_dq_uninit(&gh);
-out_drop_write:
- mnt_drop_write_file(filp);
return error;
}
-static int gfs2_set_flags(struct file *filp, u32 __user *ptr)
+int gfs2_miscattr_set(struct dentry *dentry, struct miscattr *ma)
{
- struct inode *inode = file_inode(filp);
- u32 fsflags, gfsflags = 0;
+ struct inode *inode = d_inode(dentry);
+ u32 fsflags = ma->flags, gfsflags = 0;
u32 mask;
int i;
- if (get_user(fsflags, ptr))
- return -EFAULT;
+ if (miscattr_has_xattr(ma))
+ return -EOPNOTSUPP;
for (i = 0; i < ARRAY_SIZE(fsflag_gfs2flag); i++) {
if (fsflags & fsflag_gfs2flag[i].fsflag) {
@@ -325,7 +306,7 @@ static int gfs2_set_flags(struct file *filp, u32 __user *ptr)
mask &= ~(GFS2_DIF_TOPDIR | GFS2_DIF_INHERIT_JDATA);
}
- return do_gfs2_set_flags(filp, gfsflags, mask, fsflags);
+ return do_gfs2_set_flags(inode, gfsflags, mask, fsflags);
}
static int gfs2_getlabel(struct file *filp, char __user *label)
@@ -342,10 +323,6 @@ static int gfs2_getlabel(struct file *filp, char __user *label)
static long gfs2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
{
switch(cmd) {
- case FS_IOC_GETFLAGS:
- return gfs2_get_flags(filp, (u32 __user *)arg);
- case FS_IOC_SETFLAGS:
- return gfs2_set_flags(filp, (u32 __user *)arg);
case FITRIM:
return gfs2_fitrim(filp, (void __user *)arg);
case FS_IOC_GETFSLABEL:
@@ -359,13 +336,6 @@ static long gfs2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
static long gfs2_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
{
switch(cmd) {
- /* These are just misnamed, they actually get/put from/to user an int */
- case FS_IOC32_GETFLAGS:
- cmd = FS_IOC_GETFLAGS;
- break;
- case FS_IOC32_SETFLAGS:
- cmd = FS_IOC_SETFLAGS;
- break;
/* Keep this list in sync with gfs2_ioctl */
case FITRIM:
case FS_IOC_GETFSLABEL:
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index c1b77e8d6b1c..243c3862d43b 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -2145,6 +2145,8 @@ static const struct inode_operations gfs2_file_iops = {
.get_acl = gfs2_get_acl,
.set_acl = gfs2_set_acl,
.update_time = gfs2_update_time,
+ .miscattr_get = gfs2_miscattr_get,
+ .miscattr_set = gfs2_miscattr_set,
};
static const struct inode_operations gfs2_dir_iops = {
@@ -2166,6 +2168,8 @@ static const struct inode_operations gfs2_dir_iops = {
.set_acl = gfs2_set_acl,
.update_time = gfs2_update_time,
.atomic_open = gfs2_atomic_open,
+ .miscattr_get = gfs2_miscattr_get,
+ .miscattr_set = gfs2_miscattr_set,
};
static const struct inode_operations gfs2_symlink_iops = {
diff --git a/fs/gfs2/inode.h b/fs/gfs2/inode.h
index 8073b8d2c7fa..446fbeb97045 100644
--- a/fs/gfs2/inode.h
+++ b/fs/gfs2/inode.h
@@ -110,6 +110,8 @@ extern loff_t gfs2_seek_hole(struct file *file, loff_t offset);
extern const struct file_operations gfs2_file_fops_nolock;
extern const struct file_operations gfs2_dir_fops_nolock;
+extern int gfs2_miscattr_get(struct dentry *dentry, struct miscattr *ma);
+extern int gfs2_miscattr_set(struct dentry *dentry, struct miscattr *ma);
extern void gfs2_set_inode_flags(struct inode *inode);
#ifdef CONFIG_GFS2_FS_LOCKING_DLM
--
2.26.2
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 09/18] orangefs: convert to miscattr
2021-02-03 12:40 [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR Miklos Szeredi
` (7 preceding siblings ...)
2021-02-03 12:41 ` [PATCH 08/18] gfs2: " Miklos Szeredi
@ 2021-02-03 12:41 ` Miklos Szeredi
2021-02-03 12:41 ` [PATCH 10/18] xfs: " Miklos Szeredi
` (9 subsequent siblings)
18 siblings, 0 replies; 40+ messages in thread
From: Miklos Szeredi @ 2021-02-03 12:41 UTC (permalink / raw)
To: linux-fsdevel; +Cc: linux-kernel, Mike Marshall
Use the miscattr API to let the VFS handle locking, permission checking and
conversion.
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Cc: Mike Marshall <hubcap@omnibond.com>
---
fs/orangefs/file.c | 79 ---------------------------------------------
fs/orangefs/inode.c | 49 ++++++++++++++++++++++++++++
2 files changed, 49 insertions(+), 79 deletions(-)
diff --git a/fs/orangefs/file.c b/fs/orangefs/file.c
index ec8ae4257975..8958bf9c5786 100644
--- a/fs/orangefs/file.c
+++ b/fs/orangefs/file.c
@@ -375,84 +375,6 @@ static ssize_t orangefs_file_write_iter(struct kiocb *iocb,
return ret;
}
-static int orangefs_getflags(struct inode *inode, unsigned long *uval)
-{
- __u64 val = 0;
- int ret;
-
- ret = orangefs_inode_getxattr(inode,
- "user.pvfs2.meta_hint",
- &val, sizeof(val));
- if (ret < 0 && ret != -ENODATA)
- return ret;
- else if (ret == -ENODATA)
- val = 0;
- *uval = val;
- return 0;
-}
-
-/*
- * Perform a miscellaneous operation on a file.
- */
-static long orangefs_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
-{
- struct inode *inode = file_inode(file);
- int ret = -ENOTTY;
- __u64 val = 0;
- unsigned long uval;
-
- gossip_debug(GOSSIP_FILE_DEBUG,
- "orangefs_ioctl: called with cmd %d\n",
- cmd);
-
- /*
- * we understand some general ioctls on files, such as the immutable
- * and append flags
- */
- if (cmd == FS_IOC_GETFLAGS) {
- ret = orangefs_getflags(inode, &uval);
- if (ret)
- return ret;
- gossip_debug(GOSSIP_FILE_DEBUG,
- "orangefs_ioctl: FS_IOC_GETFLAGS: %llu\n",
- (unsigned long long)uval);
- return put_user(uval, (int __user *)arg);
- } else if (cmd == FS_IOC_SETFLAGS) {
- unsigned long old_uval;
-
- ret = 0;
- if (get_user(uval, (int __user *)arg))
- return -EFAULT;
- /*
- * ORANGEFS_MIRROR_FL is set internally when the mirroring mode
- * is turned on for a file. The user is not allowed to turn
- * on this bit, but the bit is present if the user first gets
- * the flags and then updates the flags with some new
- * settings. So, we ignore it in the following edit. bligon.
- */
- if ((uval & ~ORANGEFS_MIRROR_FL) &
- (~(FS_IMMUTABLE_FL | FS_APPEND_FL | FS_NOATIME_FL))) {
- gossip_err("orangefs_ioctl: the FS_IOC_SETFLAGS only supports setting one of FS_IMMUTABLE_FL|FS_APPEND_FL|FS_NOATIME_FL\n");
- return -EINVAL;
- }
- ret = orangefs_getflags(inode, &old_uval);
- if (ret)
- return ret;
- ret = vfs_ioc_setflags_prepare(inode, old_uval, uval);
- if (ret)
- return ret;
- val = uval;
- gossip_debug(GOSSIP_FILE_DEBUG,
- "orangefs_ioctl: FS_IOC_SETFLAGS: %llu\n",
- (unsigned long long)val);
- ret = orangefs_inode_setxattr(inode,
- "user.pvfs2.meta_hint",
- &val, sizeof(val), 0);
- }
-
- return ret;
-}
-
static vm_fault_t orangefs_fault(struct vm_fault *vmf)
{
struct file *file = vmf->vma->vm_file;
@@ -660,7 +582,6 @@ const struct file_operations orangefs_file_operations = {
.read_iter = orangefs_file_read_iter,
.write_iter = orangefs_file_write_iter,
.lock = orangefs_lock,
- .unlocked_ioctl = orangefs_ioctl,
.mmap = orangefs_file_mmap,
.open = generic_file_open,
.splice_read = generic_file_splice_read,
diff --git a/fs/orangefs/inode.c b/fs/orangefs/inode.c
index 48f0547d4850..84461d2229ac 100644
--- a/fs/orangefs/inode.c
+++ b/fs/orangefs/inode.c
@@ -11,6 +11,7 @@
*/
#include <linux/bvec.h>
+#include <linux/miscattr.h>
#include "protocol.h"
#include "orangefs-kernel.h"
#include "orangefs-bufmap.h"
@@ -952,6 +953,52 @@ int orangefs_update_time(struct inode *inode, struct timespec64 *time, int flags
return __orangefs_setattr(inode, &iattr);
}
+static int orangefs_miscattr_get(struct dentry *dentry, struct miscattr *ma)
+{
+ u64 val = 0;
+ int ret;
+
+ gossip_debug(GOSSIP_FILE_DEBUG, "%s: called on %pd\n", __func__,
+ dentry);
+
+ ret = orangefs_inode_getxattr(d_inode(dentry),
+ "user.pvfs2.meta_hint",
+ &val, sizeof(val));
+ if (ret < 0 && ret != -ENODATA)
+ return ret;
+
+ gossip_debug(GOSSIP_FILE_DEBUG, "%s: flags=%u\n", __func__, (u32) val);
+
+ miscattr_fill_flags(ma, val);
+ return 0;
+}
+
+static int orangefs_miscattr_set(struct dentry *dentry, struct miscattr *ma)
+{
+ u64 val = 0;
+
+ gossip_debug(GOSSIP_FILE_DEBUG, "%s: called on %pd\n", __func__,
+ dentry);
+ /*
+ * ORANGEFS_MIRROR_FL is set internally when the mirroring mode is
+ * turned on for a file. The user is not allowed to turn on this bit,
+ * but the bit is present if the user first gets the flags and then
+ * updates the flags with some new settings. So, we ignore it in the
+ * following edit. bligon.
+ */
+ if (miscattr_has_xattr(ma) ||
+ (ma->flags & ~(FS_IMMUTABLE_FL | FS_APPEND_FL | FS_NOATIME_FL | ORANGEFS_MIRROR_FL))) {
+ gossip_err("%s: only supports setting one of FS_IMMUTABLE_FL|FS_APPEND_FL|FS_NOATIME_FL\n",
+ __func__);
+ return -EOPNOTSUPP;
+ }
+ val = ma->flags;
+ gossip_debug(GOSSIP_FILE_DEBUG, "%s: flags=%u\n", __func__, (u32) val);
+ return orangefs_inode_setxattr(d_inode(dentry),
+ "user.pvfs2.meta_hint",
+ &val, sizeof(val), 0);
+}
+
/* ORANGEFS2 implementation of VFS inode operations for files */
static const struct inode_operations orangefs_file_inode_operations = {
.get_acl = orangefs_get_acl,
@@ -961,6 +1008,8 @@ static const struct inode_operations orangefs_file_inode_operations = {
.listxattr = orangefs_listxattr,
.permission = orangefs_permission,
.update_time = orangefs_update_time,
+ .miscattr_get = orangefs_miscattr_get,
+ .miscattr_set = orangefs_miscattr_set,
};
static int orangefs_init_iops(struct inode *inode)
--
2.26.2
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 10/18] xfs: convert to miscattr
2021-02-03 12:40 [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR Miklos Szeredi
` (8 preceding siblings ...)
2021-02-03 12:41 ` [PATCH 09/18] orangefs: " Miklos Szeredi
@ 2021-02-03 12:41 ` Miklos Szeredi
2021-02-03 12:41 ` [PATCH 11/18] efivars: " Miklos Szeredi
` (8 subsequent siblings)
18 siblings, 0 replies; 40+ messages in thread
From: Miklos Szeredi @ 2021-02-03 12:41 UTC (permalink / raw)
To: linux-fsdevel; +Cc: linux-kernel, Darrick J . Wong
Use the miscattr API to let the VFS handle locking, permission checking and
conversion.
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Cc: Darrick J. Wong <djwong@kernel.org>
---
fs/xfs/libxfs/xfs_fs.h | 4 -
fs/xfs/xfs_ioctl.c | 294 ++++++++++++-----------------------------
fs/xfs/xfs_ioctl.h | 10 ++
fs/xfs/xfs_ioctl32.c | 2 -
fs/xfs/xfs_ioctl32.h | 2 -
fs/xfs/xfs_iops.c | 7 +
6 files changed, 101 insertions(+), 218 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
index 2a2e3cfd94f0..3ed41aab0473 100644
--- a/fs/xfs/libxfs/xfs_fs.h
+++ b/fs/xfs/libxfs/xfs_fs.h
@@ -769,8 +769,6 @@ struct xfs_scrub_metadata {
/*
* ioctl commands that are used by Linux filesystems
*/
-#define XFS_IOC_GETXFLAGS FS_IOC_GETFLAGS
-#define XFS_IOC_SETXFLAGS FS_IOC_SETFLAGS
#define XFS_IOC_GETVERSION FS_IOC_GETVERSION
/*
@@ -781,8 +779,6 @@ struct xfs_scrub_metadata {
#define XFS_IOC_ALLOCSP _IOW ('X', 10, struct xfs_flock64)
#define XFS_IOC_FREESP _IOW ('X', 11, struct xfs_flock64)
#define XFS_IOC_DIOINFO _IOR ('X', 30, struct dioattr)
-#define XFS_IOC_FSGETXATTR FS_IOC_FSGETXATTR
-#define XFS_IOC_FSSETXATTR FS_IOC_FSSETXATTR
#define XFS_IOC_ALLOCSP64 _IOW ('X', 36, struct xfs_flock64)
#define XFS_IOC_FREESP64 _IOW ('X', 37, struct xfs_flock64)
#define XFS_IOC_GETBMAP _IOWR('X', 38, struct getbmap)
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 3fbd98f61ea5..771adfd2faea 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -40,6 +40,7 @@
#include <linux/mount.h>
#include <linux/namei.h>
+#include <linux/miscattr.h>
/*
* xfs_find_handle maps from userspace xfs_fsop_handlereq structure to
@@ -1048,97 +1049,51 @@ xfs_ioc_ag_geometry(
* Linux extended inode flags interface.
*/
-STATIC unsigned int
-xfs_merge_ioc_xflags(
- unsigned int flags,
- unsigned int start)
-{
- unsigned int xflags = start;
-
- if (flags & FS_IMMUTABLE_FL)
- xflags |= FS_XFLAG_IMMUTABLE;
- else
- xflags &= ~FS_XFLAG_IMMUTABLE;
- if (flags & FS_APPEND_FL)
- xflags |= FS_XFLAG_APPEND;
- else
- xflags &= ~FS_XFLAG_APPEND;
- if (flags & FS_SYNC_FL)
- xflags |= FS_XFLAG_SYNC;
- else
- xflags &= ~FS_XFLAG_SYNC;
- if (flags & FS_NOATIME_FL)
- xflags |= FS_XFLAG_NOATIME;
- else
- xflags &= ~FS_XFLAG_NOATIME;
- if (flags & FS_NODUMP_FL)
- xflags |= FS_XFLAG_NODUMP;
- else
- xflags &= ~FS_XFLAG_NODUMP;
- if (flags & FS_DAX_FL)
- xflags |= FS_XFLAG_DAX;
- else
- xflags &= ~FS_XFLAG_DAX;
-
- return xflags;
-}
-
-STATIC unsigned int
-xfs_di2lxflags(
- uint16_t di_flags,
- uint64_t di_flags2)
-{
- unsigned int flags = 0;
-
- if (di_flags & XFS_DIFLAG_IMMUTABLE)
- flags |= FS_IMMUTABLE_FL;
- if (di_flags & XFS_DIFLAG_APPEND)
- flags |= FS_APPEND_FL;
- if (di_flags & XFS_DIFLAG_SYNC)
- flags |= FS_SYNC_FL;
- if (di_flags & XFS_DIFLAG_NOATIME)
- flags |= FS_NOATIME_FL;
- if (di_flags & XFS_DIFLAG_NODUMP)
- flags |= FS_NODUMP_FL;
- if (di_flags2 & XFS_DIFLAG2_DAX) {
- flags |= FS_DAX_FL;
- }
- return flags;
-}
-
static void
xfs_fill_fsxattr(
struct xfs_inode *ip,
bool attr,
- struct fsxattr *fa)
+ struct miscattr *ma)
{
struct xfs_ifork *ifp = attr ? ip->i_afp : &ip->i_df;
- simple_fill_fsxattr(fa, xfs_ip2xflags(ip));
- fa->fsx_extsize = ip->i_d.di_extsize << ip->i_mount->m_sb.sb_blocklog;
- fa->fsx_cowextsize = ip->i_d.di_cowextsize <<
+ miscattr_fill_xflags(ma, xfs_ip2xflags(ip));
+ ma->flags &= ~FS_PROJINHERIT_FL; /* Accidental? */
+ ma->fsx_extsize = ip->i_d.di_extsize << ip->i_mount->m_sb.sb_blocklog;
+ ma->fsx_cowextsize = ip->i_d.di_cowextsize <<
ip->i_mount->m_sb.sb_blocklog;
- fa->fsx_projid = ip->i_d.di_projid;
+ ma->fsx_projid = ip->i_d.di_projid;
if (ifp && (ifp->if_flags & XFS_IFEXTENTS))
- fa->fsx_nextents = xfs_iext_count(ifp);
+ ma->fsx_nextents = xfs_iext_count(ifp);
else
- fa->fsx_nextents = xfs_ifork_nextents(ifp);
+ ma->fsx_nextents = xfs_ifork_nextents(ifp);
}
STATIC int
-xfs_ioc_fsgetxattr(
+xfs_ioc_fsgetxattra(
xfs_inode_t *ip,
- int attr,
void __user *arg)
{
- struct fsxattr fa;
+ struct miscattr ma;
xfs_ilock(ip, XFS_ILOCK_SHARED);
- xfs_fill_fsxattr(ip, attr, &fa);
+ xfs_fill_fsxattr(ip, true, &ma);
+ xfs_iunlock(ip, XFS_ILOCK_SHARED);
+
+ return fsxattr_copy_to_user(&ma, arg);
+}
+
+int
+xfs_miscattr_get(
+ struct dentry *dentry,
+ struct miscattr *ma)
+{
+ xfs_inode_t *ip = XFS_I(d_inode(dentry));
+
+ xfs_ilock(ip, XFS_ILOCK_SHARED);
+ xfs_fill_fsxattr(ip, false, ma);
xfs_iunlock(ip, XFS_ILOCK_SHARED);
- if (copy_to_user(arg, &fa, sizeof(fa)))
- return -EFAULT;
return 0;
}
@@ -1205,37 +1160,37 @@ static int
xfs_ioctl_setattr_xflags(
struct xfs_trans *tp,
struct xfs_inode *ip,
- struct fsxattr *fa)
+ struct miscattr *ma)
{
struct xfs_mount *mp = ip->i_mount;
uint64_t di_flags2;
/* Can't change realtime flag if any extents are allocated. */
if ((ip->i_df.if_nextents || ip->i_delayed_blks) &&
- XFS_IS_REALTIME_INODE(ip) != (fa->fsx_xflags & FS_XFLAG_REALTIME))
+ XFS_IS_REALTIME_INODE(ip) != (ma->fsx_xflags & FS_XFLAG_REALTIME))
return -EINVAL;
/* If realtime flag is set then must have realtime device */
- if (fa->fsx_xflags & FS_XFLAG_REALTIME) {
+ if (ma->fsx_xflags & FS_XFLAG_REALTIME) {
if (mp->m_sb.sb_rblocks == 0 || mp->m_sb.sb_rextsize == 0 ||
(ip->i_d.di_extsize % mp->m_sb.sb_rextsize))
return -EINVAL;
}
/* Clear reflink if we are actually able to set the rt flag. */
- if ((fa->fsx_xflags & FS_XFLAG_REALTIME) && xfs_is_reflink_inode(ip))
+ if ((ma->fsx_xflags & FS_XFLAG_REALTIME) && xfs_is_reflink_inode(ip))
ip->i_d.di_flags2 &= ~XFS_DIFLAG2_REFLINK;
/* Don't allow us to set DAX mode for a reflinked file for now. */
- if ((fa->fsx_xflags & FS_XFLAG_DAX) && xfs_is_reflink_inode(ip))
+ if ((ma->fsx_xflags & FS_XFLAG_DAX) && xfs_is_reflink_inode(ip))
return -EINVAL;
/* diflags2 only valid for v3 inodes. */
- di_flags2 = xfs_flags2diflags2(ip, fa->fsx_xflags);
+ di_flags2 = xfs_flags2diflags2(ip, ma->fsx_xflags);
if (di_flags2 && !xfs_sb_version_has_v3inode(&mp->m_sb))
return -EINVAL;
- ip->i_d.di_flags = xfs_flags2diflags(ip, fa->fsx_xflags);
+ ip->i_d.di_flags = xfs_flags2diflags(ip, ma->fsx_xflags);
ip->i_d.di_flags2 = di_flags2;
xfs_diflags_to_iflags(ip, false);
@@ -1248,7 +1203,7 @@ xfs_ioctl_setattr_xflags(
static void
xfs_ioctl_setattr_prepare_dax(
struct xfs_inode *ip,
- struct fsxattr *fa)
+ struct miscattr *ma)
{
struct xfs_mount *mp = ip->i_mount;
struct inode *inode = VFS_I(ip);
@@ -1260,9 +1215,9 @@ xfs_ioctl_setattr_prepare_dax(
(mp->m_flags & XFS_MOUNT_DAX_NEVER))
return;
- if (((fa->fsx_xflags & FS_XFLAG_DAX) &&
+ if (((ma->fsx_xflags & FS_XFLAG_DAX) &&
!(ip->i_d.di_flags2 & XFS_DIFLAG2_DAX)) ||
- (!(fa->fsx_xflags & FS_XFLAG_DAX) &&
+ (!(ma->fsx_xflags & FS_XFLAG_DAX) &&
(ip->i_d.di_flags2 & XFS_DIFLAG2_DAX)))
d_mark_dontcache(inode);
}
@@ -1335,25 +1290,25 @@ xfs_ioctl_setattr_get_trans(
static int
xfs_ioctl_setattr_check_extsize(
struct xfs_inode *ip,
- struct fsxattr *fa)
+ struct miscattr *ma)
{
struct xfs_mount *mp = ip->i_mount;
xfs_extlen_t size;
xfs_fsblock_t extsize_fsb;
if (S_ISREG(VFS_I(ip)->i_mode) && ip->i_df.if_nextents &&
- ((ip->i_d.di_extsize << mp->m_sb.sb_blocklog) != fa->fsx_extsize))
+ ((ip->i_d.di_extsize << mp->m_sb.sb_blocklog) != ma->fsx_extsize))
return -EINVAL;
- if (fa->fsx_extsize == 0)
+ if (ma->fsx_extsize == 0)
return 0;
- extsize_fsb = XFS_B_TO_FSB(mp, fa->fsx_extsize);
+ extsize_fsb = XFS_B_TO_FSB(mp, ma->fsx_extsize);
if (extsize_fsb > MAXEXTLEN)
return -EINVAL;
if (XFS_IS_REALTIME_INODE(ip) ||
- (fa->fsx_xflags & FS_XFLAG_REALTIME)) {
+ (ma->fsx_xflags & FS_XFLAG_REALTIME)) {
size = mp->m_sb.sb_rextsize << mp->m_sb.sb_blocklog;
} else {
size = mp->m_sb.sb_blocksize;
@@ -1361,7 +1316,7 @@ xfs_ioctl_setattr_check_extsize(
return -EINVAL;
}
- if (fa->fsx_extsize % size)
+ if (ma->fsx_extsize % size)
return -EINVAL;
return 0;
@@ -1385,22 +1340,22 @@ xfs_ioctl_setattr_check_extsize(
static int
xfs_ioctl_setattr_check_cowextsize(
struct xfs_inode *ip,
- struct fsxattr *fa)
+ struct miscattr *ma)
{
struct xfs_mount *mp = ip->i_mount;
xfs_extlen_t size;
xfs_fsblock_t cowextsize_fsb;
- if (!(fa->fsx_xflags & FS_XFLAG_COWEXTSIZE))
+ if (!(ma->fsx_xflags & FS_XFLAG_COWEXTSIZE))
return 0;
if (!xfs_sb_version_hasreflink(&ip->i_mount->m_sb))
return -EINVAL;
- if (fa->fsx_cowextsize == 0)
+ if (ma->fsx_cowextsize == 0)
return 0;
- cowextsize_fsb = XFS_B_TO_FSB(mp, fa->fsx_cowextsize);
+ cowextsize_fsb = XFS_B_TO_FSB(mp, ma->fsx_cowextsize);
if (cowextsize_fsb > MAXEXTLEN)
return -EINVAL;
@@ -1408,7 +1363,7 @@ xfs_ioctl_setattr_check_cowextsize(
if (cowextsize_fsb > mp->m_sb.sb_agblocks / 2)
return -EINVAL;
- if (fa->fsx_cowextsize % size)
+ if (ma->fsx_cowextsize % size)
return -EINVAL;
return 0;
@@ -1417,21 +1372,21 @@ xfs_ioctl_setattr_check_cowextsize(
static int
xfs_ioctl_setattr_check_projid(
struct xfs_inode *ip,
- struct fsxattr *fa)
+ struct miscattr *ma)
{
/* Disallow 32bit project ids if projid32bit feature is not enabled. */
- if (fa->fsx_projid > (uint16_t)-1 &&
+ if (ma->fsx_projid > (uint16_t)-1 &&
!xfs_sb_version_hasprojid32bit(&ip->i_mount->m_sb))
return -EINVAL;
return 0;
}
-STATIC int
-xfs_ioctl_setattr(
- xfs_inode_t *ip,
- struct fsxattr *fa)
+int
+xfs_miscattr_set(
+ struct dentry *dentry,
+ struct miscattr *ma)
{
- struct fsxattr old_fa;
+ xfs_inode_t *ip = XFS_I(d_inode(dentry));
struct xfs_mount *mp = ip->i_mount;
struct xfs_trans *tp;
struct xfs_dquot *pdqp = NULL;
@@ -1440,7 +1395,17 @@ xfs_ioctl_setattr(
trace_xfs_ioctl_setattr(ip);
- code = xfs_ioctl_setattr_check_projid(ip, fa);
+ if (!ma->xattr_valid) {
+ /* FS_PROJINHERIT_FL not accepted, deliberate? */
+ if (ma->flags & ~(FS_IMMUTABLE_FL | FS_APPEND_FL |
+ FS_NOATIME_FL | FS_NODUMP_FL |
+ FS_SYNC_FL | FS_DAX_FL))
+ return -EOPNOTSUPP;
+
+ goto skip_xattr_1;
+ }
+
+ code = xfs_ioctl_setattr_check_projid(ip, ma);
if (code)
return code;
@@ -1454,13 +1419,14 @@ xfs_ioctl_setattr(
*/
if (XFS_IS_QUOTA_ON(mp)) {
code = xfs_qm_vop_dqalloc(ip, VFS_I(ip)->i_uid,
- VFS_I(ip)->i_gid, fa->fsx_projid,
+ VFS_I(ip)->i_gid, ma->fsx_projid,
XFS_QMOPT_PQUOTA, NULL, NULL, &pdqp);
if (code)
return code;
}
- xfs_ioctl_setattr_prepare_dax(ip, fa);
+skip_xattr_1:
+ xfs_ioctl_setattr_prepare_dax(ip, ma);
tp = xfs_ioctl_setattr_get_trans(ip);
if (IS_ERR(tp)) {
@@ -1468,31 +1434,32 @@ xfs_ioctl_setattr(
goto error_free_dquots;
}
+ if (!ma->xattr_valid)
+ goto skip_xattr_2;
+
if (XFS_IS_QUOTA_RUNNING(mp) && XFS_IS_PQUOTA_ON(mp) &&
- ip->i_d.di_projid != fa->fsx_projid) {
+ ip->i_d.di_projid != ma->fsx_projid) {
code = xfs_qm_vop_chown_reserve(tp, ip, NULL, NULL, pdqp,
capable(CAP_FOWNER) ? XFS_QMOPT_FORCE_RES : 0);
if (code) /* out of quota */
goto error_trans_cancel;
}
- xfs_fill_fsxattr(ip, false, &old_fa);
- code = vfs_ioc_fssetxattr_check(VFS_I(ip), &old_fa, fa);
- if (code)
- goto error_trans_cancel;
-
- code = xfs_ioctl_setattr_check_extsize(ip, fa);
+ code = xfs_ioctl_setattr_check_extsize(ip, ma);
if (code)
goto error_trans_cancel;
- code = xfs_ioctl_setattr_check_cowextsize(ip, fa);
+ code = xfs_ioctl_setattr_check_cowextsize(ip, ma);
if (code)
goto error_trans_cancel;
- code = xfs_ioctl_setattr_xflags(tp, ip, fa);
+skip_xattr_2:
+ code = xfs_ioctl_setattr_xflags(tp, ip, ma);
if (code)
goto error_trans_cancel;
+ if (!ma->xattr_valid)
+ goto skip_xattr_3;
/*
* Change file ownership. Must be the owner or privileged. CAP_FSETID
* overrides the following restrictions:
@@ -1506,12 +1473,12 @@ xfs_ioctl_setattr(
VFS_I(ip)->i_mode &= ~(S_ISUID|S_ISGID);
/* Change the ownerships and register project quota modifications */
- if (ip->i_d.di_projid != fa->fsx_projid) {
+ if (ip->i_d.di_projid != ma->fsx_projid) {
if (XFS_IS_QUOTA_RUNNING(mp) && XFS_IS_PQUOTA_ON(mp)) {
olddquot = xfs_qm_vop_chown(tp, ip,
&ip->i_pdquot, pdqp);
}
- ip->i_d.di_projid = fa->fsx_projid;
+ ip->i_d.di_projid = ma->fsx_projid;
}
/*
@@ -1520,16 +1487,17 @@ xfs_ioctl_setattr(
* are set on the inode then unconditionally clear the extent size hint.
*/
if (ip->i_d.di_flags & (XFS_DIFLAG_EXTSIZE | XFS_DIFLAG_EXTSZINHERIT))
- ip->i_d.di_extsize = fa->fsx_extsize >> mp->m_sb.sb_blocklog;
+ ip->i_d.di_extsize = ma->fsx_extsize >> mp->m_sb.sb_blocklog;
else
ip->i_d.di_extsize = 0;
if (xfs_sb_version_has_v3inode(&mp->m_sb) &&
(ip->i_d.di_flags2 & XFS_DIFLAG2_COWEXTSIZE))
- ip->i_d.di_cowextsize = fa->fsx_cowextsize >>
+ ip->i_d.di_cowextsize = ma->fsx_cowextsize >>
mp->m_sb.sb_blocklog;
else
ip->i_d.di_cowextsize = 0;
+skip_xattr_3:
code = xfs_trans_commit(tp);
/*
@@ -1547,92 +1515,6 @@ xfs_ioctl_setattr(
return code;
}
-STATIC int
-xfs_ioc_fssetxattr(
- xfs_inode_t *ip,
- struct file *filp,
- void __user *arg)
-{
- struct fsxattr fa;
- int error;
-
- if (copy_from_user(&fa, arg, sizeof(fa)))
- return -EFAULT;
-
- error = mnt_want_write_file(filp);
- if (error)
- return error;
- error = xfs_ioctl_setattr(ip, &fa);
- mnt_drop_write_file(filp);
- return error;
-}
-
-STATIC int
-xfs_ioc_getxflags(
- xfs_inode_t *ip,
- void __user *arg)
-{
- unsigned int flags;
-
- flags = xfs_di2lxflags(ip->i_d.di_flags, ip->i_d.di_flags2);
- if (copy_to_user(arg, &flags, sizeof(flags)))
- return -EFAULT;
- return 0;
-}
-
-STATIC int
-xfs_ioc_setxflags(
- struct xfs_inode *ip,
- struct file *filp,
- void __user *arg)
-{
- struct xfs_trans *tp;
- struct fsxattr fa;
- struct fsxattr old_fa;
- unsigned int flags;
- int error;
-
- if (copy_from_user(&flags, arg, sizeof(flags)))
- return -EFAULT;
-
- if (flags & ~(FS_IMMUTABLE_FL | FS_APPEND_FL | \
- FS_NOATIME_FL | FS_NODUMP_FL | \
- FS_SYNC_FL | FS_DAX_FL))
- return -EOPNOTSUPP;
-
- fa.fsx_xflags = xfs_merge_ioc_xflags(flags, xfs_ip2xflags(ip));
-
- error = mnt_want_write_file(filp);
- if (error)
- return error;
-
- xfs_ioctl_setattr_prepare_dax(ip, &fa);
-
- tp = xfs_ioctl_setattr_get_trans(ip);
- if (IS_ERR(tp)) {
- error = PTR_ERR(tp);
- goto out_drop_write;
- }
-
- xfs_fill_fsxattr(ip, false, &old_fa);
- error = vfs_ioc_fssetxattr_check(VFS_I(ip), &old_fa, &fa);
- if (error) {
- xfs_trans_cancel(tp);
- goto out_drop_write;
- }
-
- error = xfs_ioctl_setattr_xflags(tp, ip, &fa);
- if (error) {
- xfs_trans_cancel(tp);
- goto out_drop_write;
- }
-
- error = xfs_trans_commit(tp);
-out_drop_write:
- mnt_drop_write_file(filp);
- return error;
-}
-
static bool
xfs_getbmap_format(
struct kgetbmap *p,
@@ -2139,16 +2021,8 @@ xfs_file_ioctl(
case XFS_IOC_GETVERSION:
return put_user(inode->i_generation, (int __user *)arg);
- case XFS_IOC_FSGETXATTR:
- return xfs_ioc_fsgetxattr(ip, 0, arg);
case XFS_IOC_FSGETXATTRA:
- return xfs_ioc_fsgetxattr(ip, 1, arg);
- case XFS_IOC_FSSETXATTR:
- return xfs_ioc_fssetxattr(ip, filp, arg);
- case XFS_IOC_GETXFLAGS:
- return xfs_ioc_getxflags(ip, arg);
- case XFS_IOC_SETXFLAGS:
- return xfs_ioc_setxflags(ip, filp, arg);
+ return xfs_ioc_fsgetxattra(ip, arg);
case XFS_IOC_GETBMAP:
case XFS_IOC_GETBMAPA:
diff --git a/fs/xfs/xfs_ioctl.h b/fs/xfs/xfs_ioctl.h
index bab6a5a92407..afe13c70e20e 100644
--- a/fs/xfs/xfs_ioctl.h
+++ b/fs/xfs/xfs_ioctl.h
@@ -47,6 +47,16 @@ xfs_handle_to_dentry(
void __user *uhandle,
u32 hlen);
+extern int
+xfs_miscattr_get(
+ struct dentry *dentry,
+ struct miscattr *ma);
+
+extern int
+xfs_miscattr_set(
+ struct dentry *dentry,
+ struct miscattr *ma);
+
extern long
xfs_file_ioctl(
struct file *filp,
diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
index c1771e728117..54448bbd76e2 100644
--- a/fs/xfs/xfs_ioctl32.c
+++ b/fs/xfs/xfs_ioctl32.c
@@ -483,8 +483,6 @@ xfs_file_compat_ioctl(
}
#endif
/* long changes size, but xfs only copiese out 32 bits */
- case XFS_IOC_GETXFLAGS_32:
- case XFS_IOC_SETXFLAGS_32:
case XFS_IOC_GETVERSION_32:
cmd = _NATIVE_IOC(cmd, long);
return xfs_file_ioctl(filp, cmd, p);
diff --git a/fs/xfs/xfs_ioctl32.h b/fs/xfs/xfs_ioctl32.h
index 053de7d894cd..9929482bf358 100644
--- a/fs/xfs/xfs_ioctl32.h
+++ b/fs/xfs/xfs_ioctl32.h
@@ -17,8 +17,6 @@
*/
/* stock kernel-level ioctls we support */
-#define XFS_IOC_GETXFLAGS_32 FS_IOC32_GETFLAGS
-#define XFS_IOC_SETXFLAGS_32 FS_IOC32_SETFLAGS
#define XFS_IOC_GETVERSION_32 FS_IOC32_GETVERSION
/*
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 67c8dc9de8aa..904fc358a0fc 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -21,6 +21,7 @@
#include "xfs_dir2.h"
#include "xfs_iomap.h"
#include "xfs_error.h"
+#include "xfs_ioctl.h"
#include <linux/posix_acl.h>
#include <linux/security.h>
@@ -1159,6 +1160,8 @@ static const struct inode_operations xfs_inode_operations = {
.listxattr = xfs_vn_listxattr,
.fiemap = xfs_vn_fiemap,
.update_time = xfs_vn_update_time,
+ .miscattr_get = xfs_miscattr_get,
+ .miscattr_set = xfs_miscattr_set,
};
static const struct inode_operations xfs_dir_inode_operations = {
@@ -1184,6 +1187,8 @@ static const struct inode_operations xfs_dir_inode_operations = {
.listxattr = xfs_vn_listxattr,
.update_time = xfs_vn_update_time,
.tmpfile = xfs_vn_tmpfile,
+ .miscattr_get = xfs_miscattr_get,
+ .miscattr_set = xfs_miscattr_set,
};
static const struct inode_operations xfs_dir_ci_inode_operations = {
@@ -1209,6 +1214,8 @@ static const struct inode_operations xfs_dir_ci_inode_operations = {
.listxattr = xfs_vn_listxattr,
.update_time = xfs_vn_update_time,
.tmpfile = xfs_vn_tmpfile,
+ .miscattr_get = xfs_miscattr_get,
+ .miscattr_set = xfs_miscattr_set,
};
static const struct inode_operations xfs_symlink_inode_operations = {
--
2.26.2
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 11/18] efivars: convert to miscattr
2021-02-03 12:40 [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR Miklos Szeredi
` (9 preceding siblings ...)
2021-02-03 12:41 ` [PATCH 10/18] xfs: " Miklos Szeredi
@ 2021-02-03 12:41 ` Miklos Szeredi
2021-02-03 12:41 ` [PATCH 12/18] hfsplus: " Miklos Szeredi
` (7 subsequent siblings)
18 siblings, 0 replies; 40+ messages in thread
From: Miklos Szeredi @ 2021-02-03 12:41 UTC (permalink / raw)
To: linux-fsdevel; +Cc: linux-kernel, Matthew Garrett
Use the miscattr API to let the VFS handle locking, permission checking and
conversion.
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Cc: Matthew Garrett <matthew.garrett@nebula.com>
---
fs/efivarfs/file.c | 77 ---------------------------------------------
fs/efivarfs/inode.c | 43 +++++++++++++++++++++++++
2 files changed, 43 insertions(+), 77 deletions(-)
diff --git a/fs/efivarfs/file.c b/fs/efivarfs/file.c
index feaa5e182b7b..d57ee15874f9 100644
--- a/fs/efivarfs/file.c
+++ b/fs/efivarfs/file.c
@@ -106,86 +106,9 @@ static ssize_t efivarfs_file_read(struct file *file, char __user *userbuf,
return size;
}
-static inline unsigned int efivarfs_getflags(struct inode *inode)
-{
- unsigned int i_flags;
- unsigned int flags = 0;
-
- i_flags = inode->i_flags;
- if (i_flags & S_IMMUTABLE)
- flags |= FS_IMMUTABLE_FL;
- return flags;
-}
-
-static int
-efivarfs_ioc_getxflags(struct file *file, void __user *arg)
-{
- struct inode *inode = file->f_mapping->host;
- unsigned int flags = efivarfs_getflags(inode);
-
- if (copy_to_user(arg, &flags, sizeof(flags)))
- return -EFAULT;
- return 0;
-}
-
-static int
-efivarfs_ioc_setxflags(struct file *file, void __user *arg)
-{
- struct inode *inode = file->f_mapping->host;
- unsigned int flags;
- unsigned int i_flags = 0;
- unsigned int oldflags = efivarfs_getflags(inode);
- int error;
-
- if (!inode_owner_or_capable(inode))
- return -EACCES;
-
- if (copy_from_user(&flags, arg, sizeof(flags)))
- return -EFAULT;
-
- if (flags & ~FS_IMMUTABLE_FL)
- return -EOPNOTSUPP;
-
- if (flags & FS_IMMUTABLE_FL)
- i_flags |= S_IMMUTABLE;
-
-
- error = mnt_want_write_file(file);
- if (error)
- return error;
-
- inode_lock(inode);
-
- error = vfs_ioc_setflags_prepare(inode, oldflags, flags);
- if (error)
- goto out;
-
- inode_set_flags(inode, i_flags, S_IMMUTABLE);
-out:
- inode_unlock(inode);
- mnt_drop_write_file(file);
- return error;
-}
-
-static long
-efivarfs_file_ioctl(struct file *file, unsigned int cmd, unsigned long p)
-{
- void __user *arg = (void __user *)p;
-
- switch (cmd) {
- case FS_IOC_GETFLAGS:
- return efivarfs_ioc_getxflags(file, arg);
- case FS_IOC_SETFLAGS:
- return efivarfs_ioc_setxflags(file, arg);
- }
-
- return -ENOTTY;
-}
-
const struct file_operations efivarfs_file_operations = {
.open = simple_open,
.read = efivarfs_file_read,
.write = efivarfs_file_write,
.llseek = no_llseek,
- .unlocked_ioctl = efivarfs_file_ioctl,
};
diff --git a/fs/efivarfs/inode.c b/fs/efivarfs/inode.c
index 0297ad95eb5c..35d9aba303a4 100644
--- a/fs/efivarfs/inode.c
+++ b/fs/efivarfs/inode.c
@@ -10,9 +10,12 @@
#include <linux/kmemleak.h>
#include <linux/slab.h>
#include <linux/uuid.h>
+#include <linux/miscattr.h>
#include "internal.h"
+static const struct inode_operations efivarfs_file_inode_operations;
+
struct inode *efivarfs_get_inode(struct super_block *sb,
const struct inode *dir, int mode,
dev_t dev, bool is_removable)
@@ -26,6 +29,7 @@ struct inode *efivarfs_get_inode(struct super_block *sb,
inode->i_flags = is_removable ? 0 : S_IMMUTABLE;
switch (mode & S_IFMT) {
case S_IFREG:
+ inode->i_op = &efivarfs_file_inode_operations;
inode->i_fop = &efivarfs_file_operations;
break;
case S_IFDIR:
@@ -138,3 +142,42 @@ const struct inode_operations efivarfs_dir_inode_operations = {
.unlink = efivarfs_unlink,
.create = efivarfs_create,
};
+
+static int
+efivarfs_miscattr_get(struct dentry *dentry, struct miscattr *ma)
+{
+ unsigned int i_flags;
+ unsigned int flags = 0;
+
+ i_flags = d_inode(dentry)->i_flags;
+ if (i_flags & S_IMMUTABLE)
+ flags |= FS_IMMUTABLE_FL;
+
+ miscattr_fill_flags(ma, flags);
+
+ return 0;
+}
+
+static int
+efivarfs_miscattr_set(struct dentry *dentry, struct miscattr *ma)
+{
+ unsigned int i_flags = 0;
+
+ if (miscattr_has_xattr(ma))
+ return -EOPNOTSUPP;
+
+ if (ma->flags & ~FS_IMMUTABLE_FL)
+ return -EOPNOTSUPP;
+
+ if (ma->flags & FS_IMMUTABLE_FL)
+ i_flags |= S_IMMUTABLE;
+
+ inode_set_flags(d_inode(dentry), i_flags, S_IMMUTABLE);
+
+ return 0;
+}
+
+static const struct inode_operations efivarfs_file_inode_operations = {
+ .miscattr_get = efivarfs_miscattr_get,
+ .miscattr_set = efivarfs_miscattr_set,
+};
--
2.26.2
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 12/18] hfsplus: convert to miscattr
2021-02-03 12:40 [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR Miklos Szeredi
` (10 preceding siblings ...)
2021-02-03 12:41 ` [PATCH 11/18] efivars: " Miklos Szeredi
@ 2021-02-03 12:41 ` Miklos Szeredi
2021-02-03 12:41 ` [PATCH 13/18] jfs: " Miklos Szeredi
` (6 subsequent siblings)
18 siblings, 0 replies; 40+ messages in thread
From: Miklos Szeredi @ 2021-02-03 12:41 UTC (permalink / raw)
To: linux-fsdevel; +Cc: linux-kernel
Use the miscattr API to let the VFS handle locking, permission checking and
conversion.
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
fs/hfsplus/dir.c | 2 +
fs/hfsplus/hfsplus_fs.h | 13 +------
fs/hfsplus/inode.c | 53 ++++++++++++++++++++++++++
fs/hfsplus/ioctl.c | 84 -----------------------------------------
4 files changed, 57 insertions(+), 95 deletions(-)
diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c
index 29a9dcfbe81f..8ac85c3f01df 100644
--- a/fs/hfsplus/dir.c
+++ b/fs/hfsplus/dir.c
@@ -567,6 +567,8 @@ const struct inode_operations hfsplus_dir_inode_operations = {
.rename = hfsplus_rename,
.getattr = hfsplus_getattr,
.listxattr = hfsplus_listxattr,
+ .miscattr_get = hfsplus_miscattr_get,
+ .miscattr_set = hfsplus_miscattr_set,
};
const struct file_operations hfsplus_dir_operations = {
diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
index a92de5199ec3..55613fd3d176 100644
--- a/fs/hfsplus/hfsplus_fs.h
+++ b/fs/hfsplus/hfsplus_fs.h
@@ -344,17 +344,6 @@ static inline unsigned short hfsplus_min_io_size(struct super_block *sb)
#define hfs_brec_goto hfsplus_brec_goto
#define hfs_part_find hfsplus_part_find
-/*
- * definitions for ext2 flag ioctls (linux really needs a generic
- * interface for this).
- */
-
-/* ext2 ioctls (EXT2_IOC_GETFLAGS and EXT2_IOC_SETFLAGS) to support
- * chattr/lsattr */
-#define HFSPLUS_IOC_EXT2_GETFLAGS FS_IOC_GETFLAGS
-#define HFSPLUS_IOC_EXT2_SETFLAGS FS_IOC_SETFLAGS
-
-
/*
* hfs+-specific ioctl for making the filesystem bootable
*/
@@ -492,6 +481,8 @@ int hfsplus_getattr(const struct path *path, struct kstat *stat,
u32 request_mask, unsigned int query_flags);
int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end,
int datasync);
+int hfsplus_miscattr_get(struct dentry *dentry, struct miscattr *ma);
+int hfsplus_miscattr_set(struct dentry *dentry, struct miscattr *ma);
/* ioctl.c */
long hfsplus_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);
diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
index e3da9e96b835..400c7999968c 100644
--- a/fs/hfsplus/inode.c
+++ b/fs/hfsplus/inode.c
@@ -17,6 +17,7 @@
#include <linux/sched.h>
#include <linux/cred.h>
#include <linux/uio.h>
+#include <linux/miscattr.h>
#include "hfsplus_fs.h"
#include "hfsplus_raw.h"
@@ -351,6 +352,8 @@ static const struct inode_operations hfsplus_file_inode_operations = {
.setattr = hfsplus_setattr,
.getattr = hfsplus_getattr,
.listxattr = hfsplus_listxattr,
+ .miscattr_get = hfsplus_miscattr_get,
+ .miscattr_set = hfsplus_miscattr_set,
};
static const struct file_operations hfsplus_file_operations = {
@@ -626,3 +629,53 @@ int hfsplus_cat_write_inode(struct inode *inode)
hfs_find_exit(&fd);
return 0;
}
+
+int hfsplus_miscattr_get(struct dentry *dentry, struct miscattr *ma)
+{
+ struct inode *inode = d_inode(dentry);
+ struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
+ unsigned int flags = 0;
+
+ if (inode->i_flags & S_IMMUTABLE)
+ flags |= FS_IMMUTABLE_FL;
+ if (inode->i_flags & S_APPEND)
+ flags |= FS_APPEND_FL;
+ if (hip->userflags & HFSPLUS_FLG_NODUMP)
+ flags |= FS_NODUMP_FL;
+
+ miscattr_fill_flags(ma, flags);
+
+ return 0;
+}
+
+int hfsplus_miscattr_set(struct dentry *dentry, struct miscattr *ma)
+{
+ struct inode *inode = d_inode(dentry);
+ struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
+ unsigned int new_fl = 0;
+
+ if (miscattr_has_xattr(ma))
+ return -EOPNOTSUPP;
+
+ /* don't silently ignore unsupported ext2 flags */
+ if (ma->flags & ~(FS_IMMUTABLE_FL|FS_APPEND_FL|FS_NODUMP_FL))
+ return -EOPNOTSUPP;
+
+ if (ma->flags & FS_IMMUTABLE_FL)
+ new_fl |= S_IMMUTABLE;
+
+ if (ma->flags & FS_APPEND_FL)
+ new_fl |= S_APPEND;
+
+ inode_set_flags(inode, new_fl, S_IMMUTABLE | S_APPEND);
+
+ if (ma->flags & FS_NODUMP_FL)
+ hip->userflags |= HFSPLUS_FLG_NODUMP;
+ else
+ hip->userflags &= ~HFSPLUS_FLG_NODUMP;
+
+ inode->i_ctime = current_time(inode);
+ mark_inode_dirty(inode);
+
+ return 0;
+}
diff --git a/fs/hfsplus/ioctl.c b/fs/hfsplus/ioctl.c
index ce15b9496b77..5661a2e24d03 100644
--- a/fs/hfsplus/ioctl.c
+++ b/fs/hfsplus/ioctl.c
@@ -57,95 +57,11 @@ static int hfsplus_ioctl_bless(struct file *file, int __user *user_flags)
return 0;
}
-static inline unsigned int hfsplus_getflags(struct inode *inode)
-{
- struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
- unsigned int flags = 0;
-
- if (inode->i_flags & S_IMMUTABLE)
- flags |= FS_IMMUTABLE_FL;
- if (inode->i_flags & S_APPEND)
- flags |= FS_APPEND_FL;
- if (hip->userflags & HFSPLUS_FLG_NODUMP)
- flags |= FS_NODUMP_FL;
- return flags;
-}
-
-static int hfsplus_ioctl_getflags(struct file *file, int __user *user_flags)
-{
- struct inode *inode = file_inode(file);
- unsigned int flags = hfsplus_getflags(inode);
-
- return put_user(flags, user_flags);
-}
-
-static int hfsplus_ioctl_setflags(struct file *file, int __user *user_flags)
-{
- struct inode *inode = file_inode(file);
- struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
- unsigned int flags, new_fl = 0;
- unsigned int oldflags = hfsplus_getflags(inode);
- int err = 0;
-
- err = mnt_want_write_file(file);
- if (err)
- goto out;
-
- if (!inode_owner_or_capable(inode)) {
- err = -EACCES;
- goto out_drop_write;
- }
-
- if (get_user(flags, user_flags)) {
- err = -EFAULT;
- goto out_drop_write;
- }
-
- inode_lock(inode);
-
- err = vfs_ioc_setflags_prepare(inode, oldflags, flags);
- if (err)
- goto out_unlock_inode;
-
- /* don't silently ignore unsupported ext2 flags */
- if (flags & ~(FS_IMMUTABLE_FL|FS_APPEND_FL|FS_NODUMP_FL)) {
- err = -EOPNOTSUPP;
- goto out_unlock_inode;
- }
-
- if (flags & FS_IMMUTABLE_FL)
- new_fl |= S_IMMUTABLE;
-
- if (flags & FS_APPEND_FL)
- new_fl |= S_APPEND;
-
- inode_set_flags(inode, new_fl, S_IMMUTABLE | S_APPEND);
-
- if (flags & FS_NODUMP_FL)
- hip->userflags |= HFSPLUS_FLG_NODUMP;
- else
- hip->userflags &= ~HFSPLUS_FLG_NODUMP;
-
- inode->i_ctime = current_time(inode);
- mark_inode_dirty(inode);
-
-out_unlock_inode:
- inode_unlock(inode);
-out_drop_write:
- mnt_drop_write_file(file);
-out:
- return err;
-}
-
long hfsplus_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
{
void __user *argp = (void __user *)arg;
switch (cmd) {
- case HFSPLUS_IOC_EXT2_GETFLAGS:
- return hfsplus_ioctl_getflags(file, argp);
- case HFSPLUS_IOC_EXT2_SETFLAGS:
- return hfsplus_ioctl_setflags(file, argp);
case HFSPLUS_IOC_BLESS:
return hfsplus_ioctl_bless(file, argp);
default:
--
2.26.2
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 13/18] jfs: convert to miscattr
2021-02-03 12:40 [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR Miklos Szeredi
` (11 preceding siblings ...)
2021-02-03 12:41 ` [PATCH 12/18] hfsplus: " Miklos Szeredi
@ 2021-02-03 12:41 ` Miklos Szeredi
2021-02-03 12:41 ` [PATCH 14/18] nilfs2: " Miklos Szeredi
` (5 subsequent siblings)
18 siblings, 0 replies; 40+ messages in thread
From: Miklos Szeredi @ 2021-02-03 12:41 UTC (permalink / raw)
To: linux-fsdevel; +Cc: linux-kernel, Dave Kleikamp
Use the miscattr API to let the VFS handle locking, permission checking and
conversion.
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Cc: Dave Kleikamp <shaggy@kernel.org>
---
fs/jfs/file.c | 6 +--
fs/jfs/ioctl.c | 104 ++++++++++++++------------------------------
fs/jfs/jfs_dinode.h | 7 ---
fs/jfs/jfs_inode.h | 3 +-
fs/jfs/namei.c | 6 +--
5 files changed, 41 insertions(+), 85 deletions(-)
diff --git a/fs/jfs/file.c b/fs/jfs/file.c
index 930d2701f206..cb9c6bea6fff 100644
--- a/fs/jfs/file.c
+++ b/fs/jfs/file.c
@@ -129,6 +129,8 @@ int jfs_setattr(struct dentry *dentry, struct iattr *iattr)
const struct inode_operations jfs_file_inode_operations = {
.listxattr = jfs_listxattr,
.setattr = jfs_setattr,
+ .miscattr_get = jfs_miscattr_get,
+ .miscattr_set = jfs_miscattr_set,
#ifdef CONFIG_JFS_POSIX_ACL
.get_acl = jfs_get_acl,
.set_acl = jfs_set_acl,
@@ -146,7 +148,5 @@ const struct file_operations jfs_file_operations = {
.fsync = jfs_fsync,
.release = jfs_release,
.unlocked_ioctl = jfs_ioctl,
-#ifdef CONFIG_COMPAT
- .compat_ioctl = jfs_compat_ioctl,
-#endif
+ .compat_ioctl = compat_ptr_ioctl,
};
diff --git a/fs/jfs/ioctl.c b/fs/jfs/ioctl.c
index 10ee0ecca1a8..954216662cbb 100644
--- a/fs/jfs/ioctl.c
+++ b/fs/jfs/ioctl.c
@@ -15,6 +15,7 @@
#include <linux/blkdev.h>
#include <asm/current.h>
#include <linux/uaccess.h>
+#include <linux/miscattr.h>
#include "jfs_filsys.h"
#include "jfs_debug.h"
@@ -56,69 +57,49 @@ static long jfs_map_ext2(unsigned long flags, int from)
return mapped;
}
+int jfs_miscattr_get(struct dentry *dentry, struct miscattr *ma)
+{
+ struct jfs_inode_info *jfs_inode = JFS_IP(d_inode(dentry));
+ unsigned int flags = jfs_inode->mode2 & JFS_FL_USER_VISIBLE;
-long jfs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
+ miscattr_fill_flags(ma, jfs_map_ext2(flags, 0));
+
+ return 0;
+}
+
+int jfs_miscattr_set(struct dentry *dentry, struct miscattr *ma)
{
- struct inode *inode = file_inode(filp);
+ struct inode *inode = d_inode(dentry);
struct jfs_inode_info *jfs_inode = JFS_IP(inode);
unsigned int flags;
- switch (cmd) {
- case JFS_IOC_GETFLAGS:
- flags = jfs_inode->mode2 & JFS_FL_USER_VISIBLE;
- flags = jfs_map_ext2(flags, 0);
- return put_user(flags, (int __user *) arg);
- case JFS_IOC_SETFLAGS: {
- unsigned int oldflags;
- int err;
-
- err = mnt_want_write_file(filp);
- if (err)
- return err;
-
- if (!inode_owner_or_capable(inode)) {
- err = -EACCES;
- goto setflags_out;
- }
- if (get_user(flags, (int __user *) arg)) {
- err = -EFAULT;
- goto setflags_out;
- }
+ if (miscattr_has_xattr(ma))
+ return -EOPNOTSUPP;
- flags = jfs_map_ext2(flags, 1);
- if (!S_ISDIR(inode->i_mode))
- flags &= ~JFS_DIRSYNC_FL;
+ flags = jfs_map_ext2(ma->flags, 1);
+ if (!S_ISDIR(inode->i_mode))
+ flags &= ~JFS_DIRSYNC_FL;
- /* Is it quota file? Do not allow user to mess with it */
- if (IS_NOQUOTA(inode)) {
- err = -EPERM;
- goto setflags_out;
- }
+ /* Is it quota file? Do not allow user to mess with it */
+ if (IS_NOQUOTA(inode))
+ return -EPERM;
- /* Lock against other parallel changes of flags */
- inode_lock(inode);
+ flags = flags & JFS_FL_USER_MODIFIABLE;
+ flags |= jfs_inode->mode2 & ~JFS_FL_USER_MODIFIABLE;
+ jfs_inode->mode2 = flags;
- oldflags = jfs_map_ext2(jfs_inode->mode2 & JFS_FL_USER_VISIBLE,
- 0);
- err = vfs_ioc_setflags_prepare(inode, oldflags, flags);
- if (err) {
- inode_unlock(inode);
- goto setflags_out;
- }
+ jfs_set_inode_flags(inode);
+ inode->i_ctime = current_time(inode);
+ mark_inode_dirty(inode);
- flags = flags & JFS_FL_USER_MODIFIABLE;
- flags |= jfs_inode->mode2 & ~JFS_FL_USER_MODIFIABLE;
- jfs_inode->mode2 = flags;
-
- jfs_set_inode_flags(inode);
- inode_unlock(inode);
- inode->i_ctime = current_time(inode);
- mark_inode_dirty(inode);
-setflags_out:
- mnt_drop_write_file(filp);
- return err;
- }
+ return 0;
+}
+
+long jfs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
+{
+ struct inode *inode = file_inode(filp);
+ switch (cmd) {
case FITRIM:
{
struct super_block *sb = inode->i_sb;
@@ -156,22 +137,3 @@ long jfs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
return -ENOTTY;
}
}
-
-#ifdef CONFIG_COMPAT
-long jfs_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
-{
- /* While these ioctl numbers defined with 'long' and have different
- * numbers than the 64bit ABI,
- * the actual implementation only deals with ints and is compatible.
- */
- switch (cmd) {
- case JFS_IOC_GETFLAGS32:
- cmd = JFS_IOC_GETFLAGS;
- break;
- case JFS_IOC_SETFLAGS32:
- cmd = JFS_IOC_SETFLAGS;
- break;
- }
- return jfs_ioctl(filp, cmd, arg);
-}
-#endif
diff --git a/fs/jfs/jfs_dinode.h b/fs/jfs/jfs_dinode.h
index 5fa9fd594115..d6af79e94263 100644
--- a/fs/jfs/jfs_dinode.h
+++ b/fs/jfs/jfs_dinode.h
@@ -160,11 +160,4 @@ struct dinode {
#define JFS_FL_USER_MODIFIABLE 0x03F80000
#define JFS_FL_INHERIT 0x03C80000
-/* These are identical to EXT[23]_IOC_GETFLAGS/SETFLAGS */
-#define JFS_IOC_GETFLAGS _IOR('f', 1, long)
-#define JFS_IOC_SETFLAGS _IOW('f', 2, long)
-
-#define JFS_IOC_GETFLAGS32 _IOR('f', 1, int)
-#define JFS_IOC_SETFLAGS32 _IOW('f', 2, int)
-
#endif /*_H_JFS_DINODE */
diff --git a/fs/jfs/jfs_inode.h b/fs/jfs/jfs_inode.h
index 70a0d12e427e..bfacad2b18e0 100644
--- a/fs/jfs/jfs_inode.h
+++ b/fs/jfs/jfs_inode.h
@@ -9,8 +9,9 @@ struct fid;
extern struct inode *ialloc(struct inode *, umode_t);
extern int jfs_fsync(struct file *, loff_t, loff_t, int);
+extern int jfs_miscattr_get(struct dentry *dentry, struct miscattr *ma);
+extern int jfs_miscattr_set(struct dentry *dentry, struct miscattr *ma);
extern long jfs_ioctl(struct file *, unsigned int, unsigned long);
-extern long jfs_compat_ioctl(struct file *, unsigned int, unsigned long);
extern struct inode *jfs_iget(struct super_block *, unsigned long);
extern int jfs_commit_inode(struct inode *, int);
extern int jfs_write_inode(struct inode *, struct writeback_control *);
diff --git a/fs/jfs/namei.c b/fs/jfs/namei.c
index 7a55d14cc1af..2a45b7c51721 100644
--- a/fs/jfs/namei.c
+++ b/fs/jfs/namei.c
@@ -1521,6 +1521,8 @@ const struct inode_operations jfs_dir_inode_operations = {
.rename = jfs_rename,
.listxattr = jfs_listxattr,
.setattr = jfs_setattr,
+ .miscattr_get = jfs_miscattr_get,
+ .miscattr_set = jfs_miscattr_set,
#ifdef CONFIG_JFS_POSIX_ACL
.get_acl = jfs_get_acl,
.set_acl = jfs_set_acl,
@@ -1532,9 +1534,7 @@ const struct file_operations jfs_dir_operations = {
.iterate = jfs_readdir,
.fsync = jfs_fsync,
.unlocked_ioctl = jfs_ioctl,
-#ifdef CONFIG_COMPAT
- .compat_ioctl = jfs_compat_ioctl,
-#endif
+ .compat_ioctl = compat_ptr_ioctl,
.llseek = generic_file_llseek,
};
--
2.26.2
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 14/18] nilfs2: convert to miscattr
2021-02-03 12:40 [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR Miklos Szeredi
` (12 preceding siblings ...)
2021-02-03 12:41 ` [PATCH 13/18] jfs: " Miklos Szeredi
@ 2021-02-03 12:41 ` Miklos Szeredi
2021-02-03 12:41 ` [PATCH 15/18] ocfs2: " Miklos Szeredi
` (4 subsequent siblings)
18 siblings, 0 replies; 40+ messages in thread
From: Miklos Szeredi @ 2021-02-03 12:41 UTC (permalink / raw)
To: linux-fsdevel; +Cc: linux-kernel, Ryusuke Konishi
Use the miscattr API to let the VFS handle locking, permission checking and
conversion.
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Cc: Ryusuke Konishi <konishi.ryusuke@gmail.com>
---
fs/nilfs2/file.c | 2 ++
fs/nilfs2/ioctl.c | 60 ++++++++++++++---------------------------------
fs/nilfs2/namei.c | 2 ++
fs/nilfs2/nilfs.h | 2 ++
4 files changed, 23 insertions(+), 43 deletions(-)
diff --git a/fs/nilfs2/file.c b/fs/nilfs2/file.c
index 64bc81363c6c..2a9eefd12098 100644
--- a/fs/nilfs2/file.c
+++ b/fs/nilfs2/file.c
@@ -147,6 +147,8 @@ const struct inode_operations nilfs_file_inode_operations = {
.setattr = nilfs_setattr,
.permission = nilfs_permission,
.fiemap = nilfs_fiemap,
+ .miscattr_get = nilfs_miscattr_get,
+ .miscattr_set = nilfs_miscattr_set,
};
/* end of file */
diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c
index 07d26f61f22a..571c48d8ab2e 100644
--- a/fs/nilfs2/ioctl.c
+++ b/fs/nilfs2/ioctl.c
@@ -16,6 +16,7 @@
#include <linux/compat.h> /* compat_ptr() */
#include <linux/mount.h> /* mnt_want_write_file(), mnt_drop_write_file() */
#include <linux/buffer_head.h>
+#include <linux/miscattr.h>
#include "nilfs.h"
#include "segment.h"
#include "bmap.h"
@@ -113,51 +114,38 @@ static int nilfs_ioctl_wrap_copy(struct the_nilfs *nilfs,
}
/**
- * nilfs_ioctl_getflags - ioctl to support lsattr
+ * nilfs_miscattr_get - ioctl to support lsattr
*/
-static int nilfs_ioctl_getflags(struct inode *inode, void __user *argp)
+int nilfs_miscattr_get(struct dentry *dentry, struct miscattr *ma)
{
- unsigned int flags = NILFS_I(inode)->i_flags & FS_FL_USER_VISIBLE;
+ struct inode *inode = d_inode(dentry);
- return put_user(flags, (int __user *)argp);
+ miscattr_fill_flags(ma, NILFS_I(inode)->i_flags & FS_FL_USER_VISIBLE);
+
+ return 0;
}
/**
- * nilfs_ioctl_setflags - ioctl to support chattr
+ * nilfs_miscattr_set - ioctl to support chattr
*/
-static int nilfs_ioctl_setflags(struct inode *inode, struct file *filp,
- void __user *argp)
+int nilfs_miscattr_set(struct dentry *dentry, struct miscattr *ma)
{
+ struct inode *inode = d_inode(dentry);
struct nilfs_transaction_info ti;
unsigned int flags, oldflags;
int ret;
- if (!inode_owner_or_capable(inode))
- return -EACCES;
-
- if (get_user(flags, (int __user *)argp))
- return -EFAULT;
-
- ret = mnt_want_write_file(filp);
- if (ret)
- return ret;
-
- flags = nilfs_mask_flags(inode->i_mode, flags);
-
- inode_lock(inode);
-
- oldflags = NILFS_I(inode)->i_flags;
+ if (miscattr_has_xattr(ma))
+ return -EOPNOTSUPP;
- ret = vfs_ioc_setflags_prepare(inode, oldflags, flags);
- if (ret)
- goto out;
+ flags = nilfs_mask_flags(inode->i_mode, ma->flags);
ret = nilfs_transaction_begin(inode->i_sb, &ti, 0);
if (ret)
- goto out;
+ return ret;
- NILFS_I(inode)->i_flags = (oldflags & ~FS_FL_USER_MODIFIABLE) |
- (flags & FS_FL_USER_MODIFIABLE);
+ oldflags = NILFS_I(inode)->i_flags & ~FS_FL_USER_MODIFIABLE;
+ NILFS_I(inode)->i_flags = oldflags | (flags & FS_FL_USER_MODIFIABLE);
nilfs_set_inode_flags(inode);
inode->i_ctime = current_time(inode);
@@ -165,11 +153,7 @@ static int nilfs_ioctl_setflags(struct inode *inode, struct file *filp,
nilfs_set_transaction_flag(NILFS_TI_SYNC);
nilfs_mark_inode_dirty(inode);
- ret = nilfs_transaction_commit(inode->i_sb);
-out:
- inode_unlock(inode);
- mnt_drop_write_file(filp);
- return ret;
+ return nilfs_transaction_commit(inode->i_sb);
}
/**
@@ -1282,10 +1266,6 @@ long nilfs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
void __user *argp = (void __user *)arg;
switch (cmd) {
- case FS_IOC_GETFLAGS:
- return nilfs_ioctl_getflags(inode, argp);
- case FS_IOC_SETFLAGS:
- return nilfs_ioctl_setflags(inode, filp, argp);
case FS_IOC_GETVERSION:
return nilfs_ioctl_getversion(inode, argp);
case NILFS_IOCTL_CHANGE_CPMODE:
@@ -1331,12 +1311,6 @@ long nilfs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
long nilfs_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
{
switch (cmd) {
- case FS_IOC32_GETFLAGS:
- cmd = FS_IOC_GETFLAGS;
- break;
- case FS_IOC32_SETFLAGS:
- cmd = FS_IOC_SETFLAGS;
- break;
case FS_IOC32_GETVERSION:
cmd = FS_IOC_GETVERSION;
break;
diff --git a/fs/nilfs2/namei.c b/fs/nilfs2/namei.c
index a6ec7961d4f5..ba32275fc999 100644
--- a/fs/nilfs2/namei.c
+++ b/fs/nilfs2/namei.c
@@ -549,6 +549,8 @@ const struct inode_operations nilfs_dir_inode_operations = {
.setattr = nilfs_setattr,
.permission = nilfs_permission,
.fiemap = nilfs_fiemap,
+ .miscattr_get = nilfs_miscattr_get,
+ .miscattr_set = nilfs_miscattr_set,
};
const struct inode_operations nilfs_special_inode_operations = {
diff --git a/fs/nilfs2/nilfs.h b/fs/nilfs2/nilfs.h
index f8450ee3fd06..b642a730bdbd 100644
--- a/fs/nilfs2/nilfs.h
+++ b/fs/nilfs2/nilfs.h
@@ -243,6 +243,8 @@ extern void nilfs_set_link(struct inode *, struct nilfs_dir_entry *,
extern int nilfs_sync_file(struct file *, loff_t, loff_t, int);
/* ioctl.c */
+int nilfs_miscattr_get(struct dentry *dentry, struct miscattr *m);
+int nilfs_miscattr_set(struct dentry *dentry, struct miscattr *ma);
long nilfs_ioctl(struct file *, unsigned int, unsigned long);
long nilfs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
int nilfs_ioctl_prepare_clean_segments(struct the_nilfs *, struct nilfs_argv *,
--
2.26.2
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 15/18] ocfs2: convert to miscattr
2021-02-03 12:40 [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR Miklos Szeredi
` (13 preceding siblings ...)
2021-02-03 12:41 ` [PATCH 14/18] nilfs2: " Miklos Szeredi
@ 2021-02-03 12:41 ` Miklos Szeredi
2021-02-03 12:41 ` [PATCH 16/18] reiserfs: " Miklos Szeredi
` (3 subsequent siblings)
18 siblings, 0 replies; 40+ messages in thread
From: Miklos Szeredi @ 2021-02-03 12:41 UTC (permalink / raw)
To: linux-fsdevel; +Cc: linux-kernel, Joel Becker
Use the miscattr API to let the VFS handle locking, permission checking and
conversion.
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Cc: Joel Becker <jlbec@evilplan.org>
---
fs/ocfs2/file.c | 2 ++
fs/ocfs2/ioctl.c | 58 +++++++++++++-----------------------------
fs/ocfs2/ioctl.h | 2 ++
fs/ocfs2/namei.c | 3 +++
fs/ocfs2/ocfs2_ioctl.h | 8 ------
5 files changed, 25 insertions(+), 48 deletions(-)
diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index 85979e2214b3..4e04341c22bc 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -2643,6 +2643,8 @@ const struct inode_operations ocfs2_file_iops = {
.fiemap = ocfs2_fiemap,
.get_acl = ocfs2_iop_get_acl,
.set_acl = ocfs2_iop_set_acl,
+ .miscattr_get = ocfs2_miscattr_get,
+ .miscattr_set = ocfs2_miscattr_set,
};
const struct inode_operations ocfs2_special_file_iops = {
diff --git a/fs/ocfs2/ioctl.c b/fs/ocfs2/ioctl.c
index 89984172fc4a..1131a5bb8bb3 100644
--- a/fs/ocfs2/ioctl.c
+++ b/fs/ocfs2/ioctl.c
@@ -10,6 +10,7 @@
#include <linux/mount.h>
#include <linux/blkdev.h>
#include <linux/compat.h>
+#include <linux/miscattr.h>
#include <cluster/masklog.h>
@@ -61,8 +62,10 @@ static inline int o2info_coherent(struct ocfs2_info_request *req)
return (!(req->ir_flags & OCFS2_INFO_FL_NON_COHERENT));
}
-static int ocfs2_get_inode_attr(struct inode *inode, unsigned *flags)
+int ocfs2_miscattr_get(struct dentry *dentry, struct miscattr *ma)
{
+ struct inode *inode = d_inode(dentry);
+ unsigned int flags;
int status;
status = ocfs2_inode_lock(inode, NULL, 0);
@@ -71,15 +74,18 @@ static int ocfs2_get_inode_attr(struct inode *inode, unsigned *flags)
return status;
}
ocfs2_get_inode_flags(OCFS2_I(inode));
- *flags = OCFS2_I(inode)->ip_attr;
+ flags = OCFS2_I(inode)->ip_attr;
ocfs2_inode_unlock(inode, 0);
+ miscattr_fill_flags(ma, flags & OCFS2_FL_VISIBLE);
+
return status;
}
-static int ocfs2_set_inode_attr(struct inode *inode, unsigned flags,
- unsigned mask)
+int ocfs2_miscattr_set(struct dentry *dentry, struct miscattr *ma)
{
+ struct inode *inode = d_inode(dentry);
+ unsigned int flags = ma->flags;
struct ocfs2_inode_info *ocfs2_inode = OCFS2_I(inode);
struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
handle_t *handle = NULL;
@@ -87,7 +93,8 @@ static int ocfs2_set_inode_attr(struct inode *inode, unsigned flags,
unsigned oldflags;
int status;
- inode_lock(inode);
+ if (miscattr_has_xattr(ma))
+ return -EOPNOTSUPP;
status = ocfs2_inode_lock(inode, &bh, 1);
if (status < 0) {
@@ -95,19 +102,17 @@ static int ocfs2_set_inode_attr(struct inode *inode, unsigned flags,
goto bail;
}
- status = -EACCES;
- if (!inode_owner_or_capable(inode))
- goto bail_unlock;
-
if (!S_ISDIR(inode->i_mode))
flags &= ~OCFS2_DIRSYNC_FL;
oldflags = ocfs2_inode->ip_attr;
- flags = flags & mask;
- flags |= oldflags & ~mask;
+ flags = flags & OCFS2_FL_MODIFIABLE;
+ flags |= oldflags & ~OCFS2_FL_MODIFIABLE;
- status = vfs_ioc_setflags_prepare(inode, oldflags, flags);
- if (status)
+ /* Check already done by VFS, but repeat with ocfs lock */
+ status = -EPERM;
+ if ((flags ^ oldflags) & (FS_APPEND_FL | FS_IMMUTABLE_FL) &&
+ !capable(CAP_LINUX_IMMUTABLE))
goto bail_unlock;
handle = ocfs2_start_trans(osb, OCFS2_INODE_UPDATE_CREDITS);
@@ -129,8 +134,6 @@ static int ocfs2_set_inode_attr(struct inode *inode, unsigned flags,
bail_unlock:
ocfs2_inode_unlock(inode, 1);
bail:
- inode_unlock(inode);
-
brelse(bh);
return status;
@@ -836,7 +839,6 @@ static int ocfs2_info_handle(struct inode *inode, struct ocfs2_info *info,
long ocfs2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
{
struct inode *inode = file_inode(filp);
- unsigned int flags;
int new_clusters;
int status;
struct ocfs2_space_resv sr;
@@ -849,24 +851,6 @@ long ocfs2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
void __user *argp = (void __user *)arg;
switch (cmd) {
- case OCFS2_IOC_GETFLAGS:
- status = ocfs2_get_inode_attr(inode, &flags);
- if (status < 0)
- return status;
-
- flags &= OCFS2_FL_VISIBLE;
- return put_user(flags, (int __user *) arg);
- case OCFS2_IOC_SETFLAGS:
- if (get_user(flags, (int __user *) arg))
- return -EFAULT;
-
- status = mnt_want_write_file(filp);
- if (status)
- return status;
- status = ocfs2_set_inode_attr(inode, flags,
- OCFS2_FL_MODIFIABLE);
- mnt_drop_write_file(filp);
- return status;
case OCFS2_IOC_RESVSP:
case OCFS2_IOC_RESVSP64:
case OCFS2_IOC_UNRESVSP:
@@ -959,12 +943,6 @@ long ocfs2_compat_ioctl(struct file *file, unsigned cmd, unsigned long arg)
void __user *argp = (void __user *)arg;
switch (cmd) {
- case OCFS2_IOC32_GETFLAGS:
- cmd = OCFS2_IOC_GETFLAGS;
- break;
- case OCFS2_IOC32_SETFLAGS:
- cmd = OCFS2_IOC_SETFLAGS;
- break;
case OCFS2_IOC_RESVSP:
case OCFS2_IOC_RESVSP64:
case OCFS2_IOC_UNRESVSP:
diff --git a/fs/ocfs2/ioctl.h b/fs/ocfs2/ioctl.h
index 9f5e4d95e37f..f69ec2260865 100644
--- a/fs/ocfs2/ioctl.h
+++ b/fs/ocfs2/ioctl.h
@@ -11,6 +11,8 @@
#ifndef OCFS2_IOCTL_PROTO_H
#define OCFS2_IOCTL_PROTO_H
+int ocfs2_miscattr_get(struct dentry *dentry, struct miscattr *ma);
+int ocfs2_miscattr_set(struct dentry *dentry, struct miscattr *ma);
long ocfs2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);
long ocfs2_compat_ioctl(struct file *file, unsigned cmd, unsigned long arg);
diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c
index 2a237ab00453..f955aa7891bb 100644
--- a/fs/ocfs2/namei.c
+++ b/fs/ocfs2/namei.c
@@ -50,6 +50,7 @@
#include "xattr.h"
#include "acl.h"
#include "ocfs2_trace.h"
+#include "ioctl.h"
#include "buffer_head_io.h"
@@ -2913,4 +2914,6 @@ const struct inode_operations ocfs2_dir_iops = {
.fiemap = ocfs2_fiemap,
.get_acl = ocfs2_iop_get_acl,
.set_acl = ocfs2_iop_set_acl,
+ .miscattr_get = ocfs2_miscattr_get,
+ .miscattr_set = ocfs2_miscattr_set,
};
diff --git a/fs/ocfs2/ocfs2_ioctl.h b/fs/ocfs2/ocfs2_ioctl.h
index d7b31734f6be..273616bd4f19 100644
--- a/fs/ocfs2/ocfs2_ioctl.h
+++ b/fs/ocfs2/ocfs2_ioctl.h
@@ -12,14 +12,6 @@
#ifndef OCFS2_IOCTL_H
#define OCFS2_IOCTL_H
-/*
- * ioctl commands
- */
-#define OCFS2_IOC_GETFLAGS FS_IOC_GETFLAGS
-#define OCFS2_IOC_SETFLAGS FS_IOC_SETFLAGS
-#define OCFS2_IOC32_GETFLAGS FS_IOC32_GETFLAGS
-#define OCFS2_IOC32_SETFLAGS FS_IOC32_SETFLAGS
-
/*
* Space reservation / allocation / free ioctls and argument structure
* are designed to be compatible with XFS.
--
2.26.2
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 16/18] reiserfs: convert to miscattr
2021-02-03 12:40 [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR Miklos Szeredi
` (14 preceding siblings ...)
2021-02-03 12:41 ` [PATCH 15/18] ocfs2: " Miklos Szeredi
@ 2021-02-03 12:41 ` Miklos Szeredi
2021-02-03 12:41 ` [PATCH 17/18] ubifs: " Miklos Szeredi
` (2 subsequent siblings)
18 siblings, 0 replies; 40+ messages in thread
From: Miklos Szeredi @ 2021-02-03 12:41 UTC (permalink / raw)
To: linux-fsdevel; +Cc: linux-kernel, Jan Kara
Use the miscattr API to let the VFS handle locking, permission checking and
conversion.
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Cc: Jan Kara <jack@suse.cz>
---
fs/reiserfs/file.c | 2 +
fs/reiserfs/ioctl.c | 120 +++++++++++++++++++----------------------
fs/reiserfs/namei.c | 2 +
fs/reiserfs/reiserfs.h | 6 +--
fs/reiserfs/super.c | 2 +-
5 files changed, 62 insertions(+), 70 deletions(-)
diff --git a/fs/reiserfs/file.c b/fs/reiserfs/file.c
index 0b641ae694f1..be86ddf898a9 100644
--- a/fs/reiserfs/file.c
+++ b/fs/reiserfs/file.c
@@ -258,4 +258,6 @@ const struct inode_operations reiserfs_file_inode_operations = {
.permission = reiserfs_permission,
.get_acl = reiserfs_get_acl,
.set_acl = reiserfs_set_acl,
+ .miscattr_get = reiserfs_miscattr_get,
+ .miscattr_set = reiserfs_miscattr_set,
};
diff --git a/fs/reiserfs/ioctl.c b/fs/reiserfs/ioctl.c
index adb21bea3d60..68f4cae19c59 100644
--- a/fs/reiserfs/ioctl.c
+++ b/fs/reiserfs/ioctl.c
@@ -10,6 +10,58 @@
#include <linux/uaccess.h>
#include <linux/pagemap.h>
#include <linux/compat.h>
+#include <linux/miscattr.h>
+
+int reiserfs_miscattr_get(struct dentry *dentry, struct miscattr *ma)
+{
+ struct inode *inode = d_inode(dentry);
+
+ if (!reiserfs_attrs(inode->i_sb))
+ return -ENOTTY;
+
+ miscattr_fill_flags(ma, REISERFS_I(inode)->i_attrs);
+
+ return 0;
+}
+
+int reiserfs_miscattr_set(struct dentry *dentry, struct miscattr *ma)
+{
+ struct inode *inode = d_inode(dentry);
+ unsigned int flags = ma->flags;
+ int err;
+
+ reiserfs_write_lock(inode->i_sb);
+
+ err = -ENOTTY;
+ if (!reiserfs_attrs(inode->i_sb))
+ goto unlock;
+
+ err = -EOPNOTSUPP;
+ if (miscattr_has_xattr(ma))
+ goto unlock;
+
+ /*
+ * Is it quota file? Do not allow user to mess with it
+ */
+ err = -EPERM;
+ if (IS_NOQUOTA(inode))
+ goto unlock;
+
+ if ((flags & REISERFS_NOTAIL_FL) && S_ISREG(inode->i_mode)) {
+ err = reiserfs_unpack(inode);
+ if (err)
+ goto unlock;
+ }
+ sd_attrs_to_i_attrs(flags, inode);
+ REISERFS_I(inode)->i_attrs = flags;
+ inode->i_ctime = current_time(inode);
+ mark_inode_dirty(inode);
+ err = 0;
+unlock:
+ reiserfs_write_unlock(inode->i_sb);
+
+ return err;
+}
/*
* reiserfs_ioctl - handler for ioctl for inode
@@ -23,7 +75,6 @@
long reiserfs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
{
struct inode *inode = file_inode(filp);
- unsigned int flags;
int err = 0;
reiserfs_write_lock(inode->i_sb);
@@ -32,7 +83,7 @@ long reiserfs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
case REISERFS_IOC_UNPACK:
if (S_ISREG(inode->i_mode)) {
if (arg)
- err = reiserfs_unpack(inode, filp);
+ err = reiserfs_unpack(inode);
} else
err = -ENOTTY;
break;
@@ -40,63 +91,6 @@ long reiserfs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
* following two cases are taken from fs/ext2/ioctl.c by Remy
* Card (card@masi.ibp.fr)
*/
- case REISERFS_IOC_GETFLAGS:
- if (!reiserfs_attrs(inode->i_sb)) {
- err = -ENOTTY;
- break;
- }
-
- flags = REISERFS_I(inode)->i_attrs;
- err = put_user(flags, (int __user *)arg);
- break;
- case REISERFS_IOC_SETFLAGS:{
- if (!reiserfs_attrs(inode->i_sb)) {
- err = -ENOTTY;
- break;
- }
-
- err = mnt_want_write_file(filp);
- if (err)
- break;
-
- if (!inode_owner_or_capable(inode)) {
- err = -EPERM;
- goto setflags_out;
- }
- if (get_user(flags, (int __user *)arg)) {
- err = -EFAULT;
- goto setflags_out;
- }
- /*
- * Is it quota file? Do not allow user to mess with it
- */
- if (IS_NOQUOTA(inode)) {
- err = -EPERM;
- goto setflags_out;
- }
- err = vfs_ioc_setflags_prepare(inode,
- REISERFS_I(inode)->i_attrs,
- flags);
- if (err)
- goto setflags_out;
- if ((flags & REISERFS_NOTAIL_FL) &&
- S_ISREG(inode->i_mode)) {
- int result;
-
- result = reiserfs_unpack(inode, filp);
- if (result) {
- err = result;
- goto setflags_out;
- }
- }
- sd_attrs_to_i_attrs(flags, inode);
- REISERFS_I(inode)->i_attrs = flags;
- inode->i_ctime = current_time(inode);
- mark_inode_dirty(inode);
-setflags_out:
- mnt_drop_write_file(filp);
- break;
- }
case REISERFS_IOC_GETVERSION:
err = put_user(inode->i_generation, (int __user *)arg);
break;
@@ -138,12 +132,6 @@ long reiserfs_compat_ioctl(struct file *file, unsigned int cmd,
case REISERFS_IOC32_UNPACK:
cmd = REISERFS_IOC_UNPACK;
break;
- case REISERFS_IOC32_GETFLAGS:
- cmd = REISERFS_IOC_GETFLAGS;
- break;
- case REISERFS_IOC32_SETFLAGS:
- cmd = REISERFS_IOC_SETFLAGS;
- break;
case REISERFS_IOC32_GETVERSION:
cmd = REISERFS_IOC_GETVERSION;
break;
@@ -165,7 +153,7 @@ int reiserfs_commit_write(struct file *f, struct page *page,
* Function try to convert tail from direct item into indirect.
* It set up nopack attribute in the REISERFS_I(inode)->nopack
*/
-int reiserfs_unpack(struct inode *inode, struct file *filp)
+int reiserfs_unpack(struct inode *inode)
{
int retval = 0;
int index;
diff --git a/fs/reiserfs/namei.c b/fs/reiserfs/namei.c
index 1594687582f0..d595b7f23fac 100644
--- a/fs/reiserfs/namei.c
+++ b/fs/reiserfs/namei.c
@@ -1657,6 +1657,8 @@ const struct inode_operations reiserfs_dir_inode_operations = {
.permission = reiserfs_permission,
.get_acl = reiserfs_get_acl,
.set_acl = reiserfs_set_acl,
+ .miscattr_get = reiserfs_miscattr_get,
+ .miscattr_set = reiserfs_miscattr_set,
};
/*
diff --git a/fs/reiserfs/reiserfs.h b/fs/reiserfs/reiserfs.h
index f69871516167..b7e06fbca65a 100644
--- a/fs/reiserfs/reiserfs.h
+++ b/fs/reiserfs/reiserfs.h
@@ -18,8 +18,6 @@
/* the 32 bit compat definitions with int argument */
#define REISERFS_IOC32_UNPACK _IOW(0xCD, 1, int)
-#define REISERFS_IOC32_GETFLAGS FS_IOC32_GETFLAGS
-#define REISERFS_IOC32_SETFLAGS FS_IOC32_SETFLAGS
#define REISERFS_IOC32_GETVERSION FS_IOC32_GETVERSION
#define REISERFS_IOC32_SETVERSION FS_IOC32_SETVERSION
@@ -3407,7 +3405,9 @@ __u32 r5_hash(const signed char *msg, int len);
#define SPARE_SPACE 500
/* prototypes from ioctl.c */
+int reiserfs_miscattr_get(struct dentry *dentry, struct miscattr *ma);
+int reiserfs_miscattr_set(struct dentry *dentry, struct miscattr *ma);
long reiserfs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);
long reiserfs_compat_ioctl(struct file *filp,
unsigned int cmd, unsigned long arg);
-int reiserfs_unpack(struct inode *inode, struct file *filp);
+int reiserfs_unpack(struct inode *inode);
diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c
index 1b9c7a387dc7..3ffafc73acf0 100644
--- a/fs/reiserfs/super.c
+++ b/fs/reiserfs/super.c
@@ -2408,7 +2408,7 @@ static int reiserfs_quota_on(struct super_block *sb, int type, int format_id,
* IO to work
*/
if (!(REISERFS_I(inode)->i_flags & i_nopack_mask)) {
- err = reiserfs_unpack(inode, NULL);
+ err = reiserfs_unpack(inode);
if (err) {
reiserfs_warning(sb, "super-6520",
"Unpacking tail of quota file failed"
--
2.26.2
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 17/18] ubifs: convert to miscattr
2021-02-03 12:40 [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR Miklos Szeredi
` (15 preceding siblings ...)
2021-02-03 12:41 ` [PATCH 16/18] reiserfs: " Miklos Szeredi
@ 2021-02-03 12:41 ` Miklos Szeredi
2021-02-03 12:41 ` [PATCH 18/18] vfs: remove unused ioctl helpers Miklos Szeredi
2021-02-03 13:05 ` [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR Matthew Wilcox
18 siblings, 0 replies; 40+ messages in thread
From: Miklos Szeredi @ 2021-02-03 12:41 UTC (permalink / raw)
To: linux-fsdevel; +Cc: linux-kernel, Richard Weinberger
Use the miscattr API to let the VFS handle locking, permission checking and
conversion.
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Cc: Richard Weinberger <richard@nod.at>
---
fs/ubifs/dir.c | 2 ++
fs/ubifs/file.c | 2 ++
fs/ubifs/ioctl.c | 73 ++++++++++++++++++++----------------------------
fs/ubifs/ubifs.h | 2 ++
4 files changed, 36 insertions(+), 43 deletions(-)
diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 9a6b8660425a..0e25e4c352eb 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -1635,6 +1635,8 @@ const struct inode_operations ubifs_dir_inode_operations = {
.listxattr = ubifs_listxattr,
.update_time = ubifs_update_time,
.tmpfile = ubifs_tmpfile,
+ .miscattr_get = ubifs_miscattr_get,
+ .miscattr_set = ubifs_miscattr_set,
};
const struct file_operations ubifs_dir_operations = {
diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 2bc7780d2963..3761bdc801bd 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -1647,6 +1647,8 @@ const struct inode_operations ubifs_file_inode_operations = {
.getattr = ubifs_getattr,
.listxattr = ubifs_listxattr,
.update_time = ubifs_update_time,
+ .miscattr_get = ubifs_miscattr_get,
+ .miscattr_set = ubifs_miscattr_set,
};
const struct inode_operations ubifs_symlink_inode_operations = {
diff --git a/fs/ubifs/ioctl.c b/fs/ubifs/ioctl.c
index 4363d85a3fd4..901530c56259 100644
--- a/fs/ubifs/ioctl.c
+++ b/fs/ubifs/ioctl.c
@@ -14,6 +14,7 @@
#include <linux/compat.h>
#include <linux/mount.h>
+#include <linux/miscattr.h>
#include "ubifs.h"
/* Need to be kept consistent with checked flags in ioctl2ubifs() */
@@ -103,7 +104,7 @@ static int ubifs2ioctl(int ubifs_flags)
static int setflags(struct inode *inode, int flags)
{
- int oldflags, err, release;
+ int err, release;
struct ubifs_inode *ui = ubifs_inode(inode);
struct ubifs_info *c = inode->i_sb->s_fs_info;
struct ubifs_budget_req req = { .dirtied_ino = 1,
@@ -114,11 +115,6 @@ static int setflags(struct inode *inode, int flags)
return err;
mutex_lock(&ui->ui_mutex);
- oldflags = ubifs2ioctl(ui->flags);
- err = vfs_ioc_setflags_prepare(inode, oldflags, flags);
- if (err)
- goto out_unlock;
-
ui->flags &= ~ioctl2ubifs(UBIFS_SETTABLE_IOCTL_FLAGS);
ui->flags |= ioctl2ubifs(flags);
ubifs_set_inode_flags(inode);
@@ -132,54 +128,45 @@ static int setflags(struct inode *inode, int flags)
if (IS_SYNC(inode))
err = write_inode_now(inode, 1);
return err;
-
-out_unlock:
- mutex_unlock(&ui->ui_mutex);
- ubifs_release_budget(c, &req);
- return err;
}
-long ubifs_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+int ubifs_miscattr_get(struct dentry *dentry, struct miscattr *ma)
{
- int flags, err;
- struct inode *inode = file_inode(file);
+ struct inode *inode = d_inode(dentry);
+ int flags = ubifs2ioctl(ubifs_inode(inode)->flags);
- switch (cmd) {
- case FS_IOC_GETFLAGS:
- flags = ubifs2ioctl(ubifs_inode(inode)->flags);
+ dbg_gen("get flags: %#x, i_flags %#x", flags, inode->i_flags);
+ miscattr_fill_flags(ma, flags);
- dbg_gen("get flags: %#x, i_flags %#x", flags, inode->i_flags);
- return put_user(flags, (int __user *) arg);
+ return 0;
+}
- case FS_IOC_SETFLAGS: {
- if (IS_RDONLY(inode))
- return -EROFS;
+int ubifs_miscattr_set(struct dentry *dentry, struct miscattr *ma)
+{
+ struct inode *inode = d_inode(dentry);
+ int flags = ma->flags;
- if (!inode_owner_or_capable(inode))
- return -EACCES;
+ if (miscattr_has_xattr(ma))
+ return -EOPNOTSUPP;
- if (get_user(flags, (int __user *) arg))
- return -EFAULT;
+ if (flags & ~UBIFS_GETTABLE_IOCTL_FLAGS)
+ return -EOPNOTSUPP;
- if (flags & ~UBIFS_GETTABLE_IOCTL_FLAGS)
- return -EOPNOTSUPP;
- flags &= UBIFS_SETTABLE_IOCTL_FLAGS;
+ flags &= UBIFS_SETTABLE_IOCTL_FLAGS;
- if (!S_ISDIR(inode->i_mode))
- flags &= ~FS_DIRSYNC_FL;
+ if (!S_ISDIR(inode->i_mode))
+ flags &= ~FS_DIRSYNC_FL;
- /*
- * Make sure the file-system is read-write and make sure it
- * will not become read-only while we are changing the flags.
- */
- err = mnt_want_write_file(file);
- if (err)
- return err;
- dbg_gen("set flags: %#x, i_flags %#x", flags, inode->i_flags);
- err = setflags(inode, flags);
- mnt_drop_write_file(file);
- return err;
- }
+ dbg_gen("set flags: %#x, i_flags %#x", flags, inode->i_flags);
+ return setflags(inode, flags);
+}
+
+long ubifs_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+ int err;
+ struct inode *inode = file_inode(file);
+
+ switch (cmd) {
case FS_IOC_SET_ENCRYPTION_POLICY: {
struct ubifs_info *c = inode->i_sb->s_fs_info;
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index fc2cdde3b549..404ec58254e9 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -2052,6 +2052,8 @@ int ubifs_recover_size(struct ubifs_info *c, bool in_place);
void ubifs_destroy_size_tree(struct ubifs_info *c);
/* ioctl.c */
+int ubifs_miscattr_get(struct dentry *dentry, struct miscattr *ma);
+int ubifs_miscattr_set(struct dentry *dentry, struct miscattr *ma);
long ubifs_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
void ubifs_set_inode_flags(struct inode *inode);
#ifdef CONFIG_COMPAT
--
2.26.2
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 18/18] vfs: remove unused ioctl helpers
2021-02-03 12:40 [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR Miklos Szeredi
` (16 preceding siblings ...)
2021-02-03 12:41 ` [PATCH 17/18] ubifs: " Miklos Szeredi
@ 2021-02-03 12:41 ` Miklos Szeredi
2021-02-03 13:05 ` [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR Matthew Wilcox
18 siblings, 0 replies; 40+ messages in thread
From: Miklos Szeredi @ 2021-02-03 12:41 UTC (permalink / raw)
To: linux-fsdevel; +Cc: linux-kernel
Remove vfs_ioc_setflags_prepare(), vfs_ioc_fssetxattr_check() and
simple_fill_fsxattr(), which are no longer used.
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
fs/inode.c | 87 ----------------------------------------------
include/linux/fs.h | 12 -------
2 files changed, 99 deletions(-)
diff --git a/fs/inode.c b/fs/inode.c
index 6442d97d9a4a..8771d3edbd0b 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -12,7 +12,6 @@
#include <linux/security.h>
#include <linux/cdev.h>
#include <linux/memblock.h>
-#include <linux/fscrypt.h>
#include <linux/fsnotify.h>
#include <linux/mount.h>
#include <linux/posix_acl.h>
@@ -2293,89 +2292,3 @@ struct timespec64 current_time(struct inode *inode)
return timestamp_truncate(now, inode);
}
EXPORT_SYMBOL(current_time);
-
-/*
- * Generic function to check FS_IOC_SETFLAGS values and reject any invalid
- * configurations.
- *
- * Note: the caller should be holding i_mutex, or else be sure that they have
- * exclusive access to the inode structure.
- */
-int vfs_ioc_setflags_prepare(struct inode *inode, unsigned int oldflags,
- unsigned int flags)
-{
- /*
- * The IMMUTABLE and APPEND_ONLY flags can only be changed by
- * the relevant capability.
- *
- * This test looks nicer. Thanks to Pauline Middelink
- */
- if ((flags ^ oldflags) & (FS_APPEND_FL | FS_IMMUTABLE_FL) &&
- !capable(CAP_LINUX_IMMUTABLE))
- return -EPERM;
-
- return fscrypt_prepare_setflags(inode, oldflags, flags);
-}
-EXPORT_SYMBOL(vfs_ioc_setflags_prepare);
-
-/*
- * Generic function to check FS_IOC_FSSETXATTR values and reject any invalid
- * configurations.
- *
- * Note: the caller should be holding i_mutex, or else be sure that they have
- * exclusive access to the inode structure.
- */
-int vfs_ioc_fssetxattr_check(struct inode *inode, const struct fsxattr *old_fa,
- struct fsxattr *fa)
-{
- /*
- * Can't modify an immutable/append-only file unless we have
- * appropriate permission.
- */
- if ((old_fa->fsx_xflags ^ fa->fsx_xflags) &
- (FS_XFLAG_IMMUTABLE | FS_XFLAG_APPEND) &&
- !capable(CAP_LINUX_IMMUTABLE))
- return -EPERM;
-
- /*
- * Project Quota ID state is only allowed to change from within the init
- * namespace. Enforce that restriction only if we are trying to change
- * the quota ID state. Everything else is allowed in user namespaces.
- */
- if (current_user_ns() != &init_user_ns) {
- if (old_fa->fsx_projid != fa->fsx_projid)
- return -EINVAL;
- if ((old_fa->fsx_xflags ^ fa->fsx_xflags) &
- FS_XFLAG_PROJINHERIT)
- return -EINVAL;
- }
-
- /* Check extent size hints. */
- if ((fa->fsx_xflags & FS_XFLAG_EXTSIZE) && !S_ISREG(inode->i_mode))
- return -EINVAL;
-
- if ((fa->fsx_xflags & FS_XFLAG_EXTSZINHERIT) &&
- !S_ISDIR(inode->i_mode))
- return -EINVAL;
-
- if ((fa->fsx_xflags & FS_XFLAG_COWEXTSIZE) &&
- !S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode))
- return -EINVAL;
-
- /*
- * It is only valid to set the DAX flag on regular files and
- * directories on filesystems.
- */
- if ((fa->fsx_xflags & FS_XFLAG_DAX) &&
- !(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode)))
- return -EINVAL;
-
- /* Extent size hints of zero turn off the flags. */
- if (fa->fsx_extsize == 0)
- fa->fsx_xflags &= ~(FS_XFLAG_EXTSIZE | FS_XFLAG_EXTSZINHERIT);
- if (fa->fsx_cowextsize == 0)
- fa->fsx_xflags &= ~FS_XFLAG_COWEXTSIZE;
-
- return 0;
-}
-EXPORT_SYMBOL(vfs_ioc_fssetxattr_check);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index ca3bab7d5f6e..fc1727003f3a 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3452,18 +3452,6 @@ extern int vfs_fadvise(struct file *file, loff_t offset, loff_t len,
extern int generic_fadvise(struct file *file, loff_t offset, loff_t len,
int advice);
-int vfs_ioc_setflags_prepare(struct inode *inode, unsigned int oldflags,
- unsigned int flags);
-
-int vfs_ioc_fssetxattr_check(struct inode *inode, const struct fsxattr *old_fa,
- struct fsxattr *fa);
-
-static inline void simple_fill_fsxattr(struct fsxattr *fa, __u32 xflags)
-{
- memset(fa, 0, sizeof(*fa));
- fa->fsx_xflags = xflags;
-}
-
/*
* Flush file data before changing attributes. Caller must hold any locks
* required to prevent further writes to this file until we're done setting
--
2.26.2
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR
2021-02-03 12:40 [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR Miklos Szeredi
` (17 preceding siblings ...)
2021-02-03 12:41 ` [PATCH 18/18] vfs: remove unused ioctl helpers Miklos Szeredi
@ 2021-02-03 13:05 ` Matthew Wilcox
2021-02-03 13:13 ` Miklos Szeredi
18 siblings, 1 reply; 40+ messages in thread
From: Matthew Wilcox @ 2021-02-03 13:05 UTC (permalink / raw)
To: Miklos Szeredi
Cc: linux-fsdevel, linux-kernel, Al Viro, Andreas Dilger,
Andreas Gruenbacher, Christoph Hellwig, Darrick J . Wong,
Dave Kleikamp, David Sterba, Jaegeuk Kim, Jan Kara, Joel Becker,
Matthew Garrett, Mike Marshall, Richard Weinberger,
Ryusuke Konishi, Theodore Ts'o, Tyler Hicks
On Wed, Feb 03, 2021 at 01:40:54PM +0100, Miklos Szeredi wrote:
> This series adds the infrastructure and conversion of filesystems to the
> new API.
>
> Two filesystems are not converted: FUSE and CIFS, as they behave
> differently from local filesystems (use the file pointer, don't perform
> permission checks). It's likely that these two can be supported with minor
> changes to the API, but this requires more thought.
Why not change the API now? ie pass the file instead of the dentry?
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR
2021-02-03 13:05 ` [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR Matthew Wilcox
@ 2021-02-03 13:13 ` Miklos Szeredi
2021-02-03 13:58 ` Matthew Wilcox
0 siblings, 1 reply; 40+ messages in thread
From: Miklos Szeredi @ 2021-02-03 13:13 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Miklos Szeredi, linux-fsdevel, linux-kernel, Al Viro,
Andreas Dilger, Andreas Gruenbacher, Christoph Hellwig,
Darrick J . Wong, Dave Kleikamp, David Sterba, Jaegeuk Kim,
Jan Kara, Joel Becker, Matthew Garrett, Mike Marshall,
Richard Weinberger, Ryusuke Konishi, Theodore Ts'o,
Tyler Hicks
On Wed, Feb 3, 2021 at 2:08 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Wed, Feb 03, 2021 at 01:40:54PM +0100, Miklos Szeredi wrote:
> > This series adds the infrastructure and conversion of filesystems to the
> > new API.
> >
> > Two filesystems are not converted: FUSE and CIFS, as they behave
> > differently from local filesystems (use the file pointer, don't perform
> > permission checks). It's likely that these two can be supported with minor
> > changes to the API, but this requires more thought.
>
> Why not change the API now? ie pass the file instead of the dentry?
These are inode attributes we are talking about, not much sense in
passing an open file to the filesystem. That was/is due to ioctl
being an fd based API.
It would make more sense to convert these filesystems to use a dentry
instead of a file pointer. Which is not trivial, unfortuantely.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR
2021-02-03 13:13 ` Miklos Szeredi
@ 2021-02-03 13:58 ` Matthew Wilcox
2021-02-03 14:13 ` Miklos Szeredi
0 siblings, 1 reply; 40+ messages in thread
From: Matthew Wilcox @ 2021-02-03 13:58 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Miklos Szeredi, linux-fsdevel, linux-kernel, Al Viro,
Andreas Dilger, Andreas Gruenbacher, Christoph Hellwig,
Darrick J . Wong, Dave Kleikamp, David Sterba, Jaegeuk Kim,
Jan Kara, Joel Becker, Matthew Garrett, Mike Marshall,
Richard Weinberger, Ryusuke Konishi, Theodore Ts'o,
Tyler Hicks
On Wed, Feb 03, 2021 at 02:13:27PM +0100, Miklos Szeredi wrote:
> On Wed, Feb 3, 2021 at 2:08 PM Matthew Wilcox <willy@infradead.org> wrote:
> >
> > On Wed, Feb 03, 2021 at 01:40:54PM +0100, Miklos Szeredi wrote:
> > > This series adds the infrastructure and conversion of filesystems to the
> > > new API.
> > >
> > > Two filesystems are not converted: FUSE and CIFS, as they behave
> > > differently from local filesystems (use the file pointer, don't perform
> > > permission checks). It's likely that these two can be supported with minor
> > > changes to the API, but this requires more thought.
> >
> > Why not change the API now? ie pass the file instead of the dentry?
>
> These are inode attributes we are talking about, not much sense in
> passing an open file to the filesystem. That was/is due to ioctl
> being an fd based API.
You might as well say "Not much point passing a dentry to the filesystem"
and just pass the inode.
> It would make more sense to convert these filesystems to use a dentry
> instead of a file pointer. Which is not trivial, unfortuantely.
Network filesystems frequently need to use the credentials attached to
a struct file in order to communicate with the server. There's no point
fighting this reality.
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR
2021-02-03 13:58 ` Matthew Wilcox
@ 2021-02-03 14:13 ` Miklos Szeredi
2021-02-03 14:28 ` Matthew Wilcox
0 siblings, 1 reply; 40+ messages in thread
From: Miklos Szeredi @ 2021-02-03 14:13 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Miklos Szeredi, linux-fsdevel, linux-kernel, Al Viro,
Andreas Dilger, Andreas Gruenbacher, Christoph Hellwig,
Darrick J . Wong, Dave Kleikamp, David Sterba, Jaegeuk Kim,
Jan Kara, Joel Becker, Matthew Garrett, Mike Marshall,
Richard Weinberger, Ryusuke Konishi, Theodore Ts'o,
Tyler Hicks
On Wed, Feb 3, 2021 at 2:58 PM Matthew Wilcox <willy@infradead.org> wrote:
> Network filesystems frequently need to use the credentials attached to
> a struct file in order to communicate with the server. There's no point
> fighting this reality.
IDGI. Credentials can be taken from the file and from the task. In
this case every filesystem except cifs looks at task creds. Why are
network filesystem special in this respect?
Thanks,
Miklos
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR
2021-02-03 14:13 ` Miklos Szeredi
@ 2021-02-03 14:28 ` Matthew Wilcox
2021-02-03 14:38 ` Miklos Szeredi
0 siblings, 1 reply; 40+ messages in thread
From: Matthew Wilcox @ 2021-02-03 14:28 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Miklos Szeredi, linux-fsdevel, linux-kernel, Al Viro,
Andreas Dilger, Andreas Gruenbacher, Christoph Hellwig,
Darrick J . Wong, Dave Kleikamp, David Sterba, Jaegeuk Kim,
Jan Kara, Joel Becker, Matthew Garrett, Mike Marshall,
Richard Weinberger, Ryusuke Konishi, Theodore Ts'o,
Tyler Hicks
On Wed, Feb 03, 2021 at 03:13:29PM +0100, Miklos Szeredi wrote:
> On Wed, Feb 3, 2021 at 2:58 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> > Network filesystems frequently need to use the credentials attached to
> > a struct file in order to communicate with the server. There's no point
> > fighting this reality.
>
> IDGI. Credentials can be taken from the file and from the task. In
> this case every filesystem except cifs looks at task creds. Why are
> network filesystem special in this respect?
I don't necessarily mean 'struct cred'. I mean "the authentication
that the client has performed to the server". Which is not a per-task
thing, it's stored in the struct file, which is why we have things like
int (*write_begin)(struct file *, struct address_space *mapping,
loff_t pos, unsigned len, unsigned flags,
struct page **pagep, void **fsdata);
disk filesystems ignore the struct file argument, but network filesystems
very much use it.
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR
2021-02-03 14:28 ` Matthew Wilcox
@ 2021-02-03 14:38 ` Miklos Szeredi
2021-02-03 14:56 ` Matthew Wilcox
0 siblings, 1 reply; 40+ messages in thread
From: Miklos Szeredi @ 2021-02-03 14:38 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Miklos Szeredi, linux-fsdevel, linux-kernel, Al Viro,
Andreas Dilger, Andreas Gruenbacher, Christoph Hellwig,
Darrick J . Wong, Dave Kleikamp, David Sterba, Jaegeuk Kim,
Jan Kara, Joel Becker, Matthew Garrett, Mike Marshall,
Richard Weinberger, Ryusuke Konishi, Theodore Ts'o,
Tyler Hicks
On Wed, Feb 3, 2021 at 3:28 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Wed, Feb 03, 2021 at 03:13:29PM +0100, Miklos Szeredi wrote:
> > On Wed, Feb 3, 2021 at 2:58 PM Matthew Wilcox <willy@infradead.org> wrote:
> >
> > > Network filesystems frequently need to use the credentials attached to
> > > a struct file in order to communicate with the server. There's no point
> > > fighting this reality.
> >
> > IDGI. Credentials can be taken from the file and from the task. In
> > this case every filesystem except cifs looks at task creds. Why are
> > network filesystem special in this respect?
>
> I don't necessarily mean 'struct cred'. I mean "the authentication
> that the client has performed to the server". Which is not a per-task
> thing, it's stored in the struct file, which is why we have things like
>
> int (*write_begin)(struct file *, struct address_space *mapping,
> loff_t pos, unsigned len, unsigned flags,
> struct page **pagep, void **fsdata);
>
> disk filesystems ignore the struct file argument, but network filesystems
> very much use it.
Fine for file I/O. That's authorized at open time for all
filesystems, not just network ones.
Not fine for metadata operations (IMO). I.e. ->[gs]etattr() don't
take a file argument either, even though on the uAPI there are plenty
of open file based variants.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR
2021-02-03 14:38 ` Miklos Szeredi
@ 2021-02-03 14:56 ` Matthew Wilcox
2021-02-03 15:03 ` Miklos Szeredi
0 siblings, 1 reply; 40+ messages in thread
From: Matthew Wilcox @ 2021-02-03 14:56 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Miklos Szeredi, linux-fsdevel, linux-kernel, Al Viro,
Andreas Dilger, Andreas Gruenbacher, Christoph Hellwig,
Darrick J . Wong, Dave Kleikamp, David Sterba, Jaegeuk Kim,
Jan Kara, Joel Becker, Matthew Garrett, Mike Marshall,
Richard Weinberger, Ryusuke Konishi, Theodore Ts'o,
Tyler Hicks
On Wed, Feb 03, 2021 at 03:38:54PM +0100, Miklos Szeredi wrote:
> On Wed, Feb 3, 2021 at 3:28 PM Matthew Wilcox <willy@infradead.org> wrote:
> >
> > On Wed, Feb 03, 2021 at 03:13:29PM +0100, Miklos Szeredi wrote:
> > > On Wed, Feb 3, 2021 at 2:58 PM Matthew Wilcox <willy@infradead.org> wrote:
> > >
> > > > Network filesystems frequently need to use the credentials attached to
> > > > a struct file in order to communicate with the server. There's no point
> > > > fighting this reality.
> > >
> > > IDGI. Credentials can be taken from the file and from the task. In
> > > this case every filesystem except cifs looks at task creds. Why are
> > > network filesystem special in this respect?
> >
> > I don't necessarily mean 'struct cred'. I mean "the authentication
> > that the client has performed to the server". Which is not a per-task
> > thing, it's stored in the struct file, which is why we have things like
> >
> > int (*write_begin)(struct file *, struct address_space *mapping,
> > loff_t pos, unsigned len, unsigned flags,
> > struct page **pagep, void **fsdata);
> >
> > disk filesystems ignore the struct file argument, but network filesystems
> > very much use it.
>
> Fine for file I/O. That's authorized at open time for all
> filesystems, not just network ones.
>
> Not fine for metadata operations (IMO). I.e. ->[gs]etattr() don't
> take a file argument either, even though on the uAPI there are plenty
> of open file based variants.
That's a fine statement of principle, but if the filesystem needs to
contact the server, then your principle must accede to reality.
But let's talk specifics. What does CIFS need to contact the server for?
Could it be cached earlier?
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR
2021-02-03 14:56 ` Matthew Wilcox
@ 2021-02-03 15:03 ` Miklos Szeredi
2021-02-08 2:00 ` Dave Chinner
0 siblings, 1 reply; 40+ messages in thread
From: Miklos Szeredi @ 2021-02-03 15:03 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Miklos Szeredi, linux-fsdevel, linux-kernel, Al Viro,
Andreas Dilger, Andreas Gruenbacher, Christoph Hellwig,
Darrick J . Wong, Dave Kleikamp, David Sterba, Jaegeuk Kim,
Jan Kara, Joel Becker, Matthew Garrett, Mike Marshall,
Richard Weinberger, Ryusuke Konishi, Theodore Ts'o,
Tyler Hicks
On Wed, Feb 3, 2021 at 3:56 PM Matthew Wilcox <willy@infradead.org> wrote:
> But let's talk specifics. What does CIFS need to contact the server for?
> Could it be cached earlier?
I don't understand what CIFS is doing, and I don't really care. This
is the sort of operation where adding a couple of network roundtrips
so that the client can obtain the credentials required to perform the
operation doesn't really matter. We won't have thousands of chattr(1)
calls per second.
So I think the principle is more important than the details of the
current implementation.
And I'm saying that knowing that fixing up FUSE will be my
responsibility and it won't be trivial either.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR
2021-02-03 15:03 ` Miklos Szeredi
@ 2021-02-08 2:00 ` Dave Chinner
2021-02-08 8:25 ` Miklos Szeredi
0 siblings, 1 reply; 40+ messages in thread
From: Dave Chinner @ 2021-02-08 2:00 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Matthew Wilcox, Miklos Szeredi, linux-fsdevel, linux-kernel,
Al Viro, Andreas Dilger, Andreas Gruenbacher, Christoph Hellwig,
Darrick J . Wong, Dave Kleikamp, David Sterba, Jaegeuk Kim,
Jan Kara, Joel Becker, Matthew Garrett, Mike Marshall,
Richard Weinberger, Ryusuke Konishi, Theodore Ts'o,
Tyler Hicks
On Wed, Feb 03, 2021 at 04:03:06PM +0100, Miklos Szeredi wrote:
> On Wed, Feb 3, 2021 at 3:56 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> > But let's talk specifics. What does CIFS need to contact the server for?
> > Could it be cached earlier?
>
> I don't understand what CIFS is doing, and I don't really care. This
> is the sort of operation where adding a couple of network roundtrips
> so that the client can obtain the credentials required to perform the
> operation doesn't really matter. We won't have thousands of chattr(1)
> calls per second.
Incorrect.
The xfs utilities can do recursive directory traversal to change
things like the project ID across an entire directory tree. Or to
change extent size hints.
We also have 'xfs_io -c "lsattr -R" ...' and 'lsprog -R' which will
do a recursive descent to list the requested attributes of all
directories and files in the tree...
So, yeah, we do indeed do thousands of these fsxattr based
operations a second, sometimes tens of thousands a second or more,
and sometimes they are issued in bulk in performance critical paths
for container build/deployment operations....
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR
2021-02-08 2:00 ` Dave Chinner
@ 2021-02-08 8:25 ` Miklos Szeredi
2021-02-08 14:02 ` Matthew Wilcox
0 siblings, 1 reply; 40+ messages in thread
From: Miklos Szeredi @ 2021-02-08 8:25 UTC (permalink / raw)
To: Dave Chinner
Cc: Matthew Wilcox, Miklos Szeredi, linux-fsdevel, linux-kernel,
Al Viro, Andreas Dilger, Andreas Gruenbacher, Christoph Hellwig,
Darrick J . Wong, Dave Kleikamp, David Sterba, Jaegeuk Kim,
Jan Kara, Joel Becker, Mike Marshall, Richard Weinberger,
Ryusuke Konishi, Theodore Ts'o, Tyler Hicks
On Mon, Feb 8, 2021 at 3:00 AM Dave Chinner <david@fromorbit.com> wrote:
>
> On Wed, Feb 03, 2021 at 04:03:06PM +0100, Miklos Szeredi wrote:
> > On Wed, Feb 3, 2021 at 3:56 PM Matthew Wilcox <willy@infradead.org> wrote:
> >
> > > But let's talk specifics. What does CIFS need to contact the server for?
> > > Could it be cached earlier?
> >
> > I don't understand what CIFS is doing, and I don't really care. This
> > is the sort of operation where adding a couple of network roundtrips
> > so that the client can obtain the credentials required to perform the
> > operation doesn't really matter. We won't have thousands of chattr(1)
> > calls per second.
>
> Incorrect.
Okay, I was wrong.
Still, CIFS may very well be able to perform these operations without
a struct file. But even if it can't, I'd still only add the file
pointer as an *optional hint* from the VFS, not as the primary object
as Matthew suggested.
I stand by my choice of /struct dentry/ as the object to pass to these
operations.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR
2021-02-08 8:25 ` Miklos Szeredi
@ 2021-02-08 14:02 ` Matthew Wilcox
2021-02-08 14:52 ` Miklos Szeredi
0 siblings, 1 reply; 40+ messages in thread
From: Matthew Wilcox @ 2021-02-08 14:02 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Dave Chinner, Miklos Szeredi, linux-fsdevel, linux-kernel,
Al Viro, Andreas Dilger, Andreas Gruenbacher, Christoph Hellwig,
Darrick J . Wong, Dave Kleikamp, David Sterba, Jaegeuk Kim,
Jan Kara, Joel Becker, Mike Marshall, Richard Weinberger,
Ryusuke Konishi, Theodore Ts'o, Tyler Hicks
On Mon, Feb 08, 2021 at 09:25:22AM +0100, Miklos Szeredi wrote:
> On Mon, Feb 8, 2021 at 3:00 AM Dave Chinner <david@fromorbit.com> wrote:
> >
> > On Wed, Feb 03, 2021 at 04:03:06PM +0100, Miklos Szeredi wrote:
> > > On Wed, Feb 3, 2021 at 3:56 PM Matthew Wilcox <willy@infradead.org> wrote:
> > >
> > > > But let's talk specifics. What does CIFS need to contact the server for?
> > > > Could it be cached earlier?
> > >
> > > I don't understand what CIFS is doing, and I don't really care. This
> > > is the sort of operation where adding a couple of network roundtrips
> > > so that the client can obtain the credentials required to perform the
> > > operation doesn't really matter. We won't have thousands of chattr(1)
> > > calls per second.
> >
> > Incorrect.
>
> Okay, I was wrong.
>
> Still, CIFS may very well be able to perform these operations without
> a struct file. But even if it can't, I'd still only add the file
> pointer as an *optional hint* from the VFS, not as the primary object
> as Matthew suggested.
>
> I stand by my choice of /struct dentry/ as the object to pass to these
> operations.
Why the dentry? This is an inode operation. Why doesn't it take an
inode as its primary object?
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 00/18] new API for FS_IOC_[GS]ETFLAGS/FS_IOC_FS[GS]ETXATTR
2021-02-08 14:02 ` Matthew Wilcox
@ 2021-02-08 14:52 ` Miklos Szeredi
0 siblings, 0 replies; 40+ messages in thread
From: Miklos Szeredi @ 2021-02-08 14:52 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Dave Chinner, Miklos Szeredi, linux-fsdevel, linux-kernel,
Al Viro, Andreas Dilger, Andreas Gruenbacher, Christoph Hellwig,
Darrick J . Wong, Dave Kleikamp, David Sterba, Jaegeuk Kim,
Jan Kara, Joel Becker, Mike Marshall, Richard Weinberger,
Ryusuke Konishi, Theodore Ts'o, Tyler Hicks
On Mon, Feb 8, 2021 at 3:02 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Mon, Feb 08, 2021 at 09:25:22AM +0100, Miklos Szeredi wrote:
> > On Mon, Feb 8, 2021 at 3:00 AM Dave Chinner <david@fromorbit.com> wrote:
> > >
> > > On Wed, Feb 03, 2021 at 04:03:06PM +0100, Miklos Szeredi wrote:
> > > > On Wed, Feb 3, 2021 at 3:56 PM Matthew Wilcox <willy@infradead.org> wrote:
> > > >
> > > > > But let's talk specifics. What does CIFS need to contact the server for?
> > > > > Could it be cached earlier?
> > > >
> > > > I don't understand what CIFS is doing, and I don't really care. This
> > > > is the sort of operation where adding a couple of network roundtrips
> > > > so that the client can obtain the credentials required to perform the
> > > > operation doesn't really matter. We won't have thousands of chattr(1)
> > > > calls per second.
> > >
> > > Incorrect.
> >
> > Okay, I was wrong.
> >
> > Still, CIFS may very well be able to perform these operations without
> > a struct file. But even if it can't, I'd still only add the file
> > pointer as an *optional hint* from the VFS, not as the primary object
> > as Matthew suggested.
> >
> > I stand by my choice of /struct dentry/ as the object to pass to these
> > operations.
>
> Why the dentry? This is an inode operation. Why doesn't it take an
> inode as its primary object?
If we pass struct file to the op, then that limits callers to those
that have an open file. E.g. it would be difficult to introduce a
path based userspace API for getting/changing these attributes.
Passing a dentry instead of an inode has no such problem, since the
dentry is always available, whether coming from an open file or a
path. Some inode operations do pass a dentry instead of an inode
(readlink, getattr, setattr) and some filesystems take advantage of
this.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 40+ messages in thread