* Re: [PATCH 03/25] vfs: Allow fsinfo() to query what's in an fs_context [ver #14]
From: Christian Brauner @ 2019-06-25 9:27 UTC (permalink / raw)
To: David Howells
Cc: viro, raven, mszeredi, linux-api, linux-fsdevel, linux-kernel
In-Reply-To: <156138535407.25627.15015993364565647650.stgit@warthog.procyon.org.uk>
On Mon, Jun 24, 2019 at 03:09:14PM +0100, David Howells wrote:
> Allow fsinfo() to be used to query the filesystem attached to an fs_context
> once a superblock has been created or if it comes from fspick().
>
> The caller must specify AT_FSINFO_FROM_FSOPEN in the parameters and must
Yeah, I like that better than how it was before.
> supply the fd from fsopen() as dfd and must set filename to NULL.
>
> This is done with something like:
>
> fd = fsopen("ext4", 0);
> ...
> struct fsinfo_params params = {
> .at_flags = AT_FSINFO_FROM_FSOPEN;
> ...
> };
> fsinfo(fd, NULL, ¶ms, ...);
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
>
> fs/fsinfo.c | 46 +++++++++++++++++++++++++++++++++++++++++++-
> fs/statfs.c | 2 +-
> include/uapi/linux/fcntl.h | 2 ++
> 3 files changed, 48 insertions(+), 2 deletions(-)
>
> diff --git a/fs/fsinfo.c b/fs/fsinfo.c
> index 49b46f96dda3..c24701f994d1 100644
> --- a/fs/fsinfo.c
> +++ b/fs/fsinfo.c
> @@ -8,6 +8,7 @@
> #include <linux/security.h>
> #include <linux/uaccess.h>
> #include <linux/fsinfo.h>
> +#include <linux/fs_context.h>
> #include <uapi/linux/mount.h>
> #include "internal.h"
>
> @@ -340,6 +341,42 @@ static int vfs_fsinfo_fd(unsigned int fd, struct fsinfo_kparams *params)
> return ret;
> }
>
> +/*
> + * Allow access to an fs_context object as created by fsopen() or fspick().
> + */
> +static int vfs_fsinfo_fscontext(int fd, struct fsinfo_kparams *params)
> +{
> + struct fs_context *fc;
> + struct fd f = fdget(fd);
> + int ret;
> +
> + if (!f.file)
> + return -EBADF;
> +
> + ret = -EINVAL;
> + if (f.file->f_op == &fscontext_fops)
Don't you mean != ?
if (f.file->f_op != &fscontext_fops)
> + goto out_f;
> + ret = -EOPNOTSUPP;
> + if (fc->ops == &legacy_fs_context_ops)
> + goto out_f;
> +
> + ret = mutex_lock_interruptible(&fc->uapi_mutex);
> + if (ret == 0) {
> + ret = -EIO;
Why EIO when there's no root dentry?
> + if (fc->root) {
> + struct path path = { .dentry = fc->root };
> +
> + ret = vfs_fsinfo(&path, params);
> + }
> +
> + mutex_unlock(&fc->uapi_mutex);
> + }
> +
> +out_f:
> + fdput(f);
> + return ret;
> +}
> +
> /*
> * Return buffer information by requestable attribute.
> *
> @@ -445,6 +482,9 @@ SYSCALL_DEFINE5(fsinfo,
> params.request = user_params.request;
> params.Nth = user_params.Nth;
> params.Mth = user_params.Mth;
> +
[1]:
> + if ((params.at_flags & AT_FSINFO_FROM_FSOPEN) && filename)
> + return -EINVAL;
> } else {
> params.request = FSINFO_ATTR_STATFS;
> }
> @@ -453,6 +493,8 @@ SYSCALL_DEFINE5(fsinfo,
> user_buf_size = 0;
> user_buffer = NULL;
> }
> + if ((params.at_flags & AT_FSINFO_FROM_FSOPEN) && filename)
> + return -EINVAL;
Sorry, why is this checked twice (see [1])? Or is the diff just
misleading here?
>
> /* Allocate an appropriately-sized buffer. We will truncate the
> * contents when we write the contents back to userspace.
> @@ -500,7 +542,9 @@ SYSCALL_DEFINE5(fsinfo,
> if (!params.buffer)
> goto error_scratch;
>
> - if (filename)
> + if (params.at_flags & AT_FSINFO_FROM_FSOPEN)
> + ret = vfs_fsinfo_fscontext(dfd, ¶ms);
> + else if (filename)
> ret = vfs_fsinfo_path(dfd, filename, ¶ms);
> else
> ret = vfs_fsinfo_fd(dfd, ¶ms);
> diff --git a/fs/statfs.c b/fs/statfs.c
> index eea7af6f2f22..b9b63d9f4f24 100644
> --- a/fs/statfs.c
> +++ b/fs/statfs.c
> @@ -86,7 +86,7 @@ int vfs_statfs(const struct path *path, struct kstatfs *buf)
> int error;
>
> error = statfs_by_dentry(path->dentry, buf);
> - if (!error)
> + if (!error && path->mnt)
> buf->f_flags = calculate_f_flags(path->mnt);
> return error;
> }
> diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h
> index 1d338357df8a..6a2402a8fa30 100644
> --- a/include/uapi/linux/fcntl.h
> +++ b/include/uapi/linux/fcntl.h
> @@ -91,6 +91,8 @@
> #define AT_STATX_FORCE_SYNC 0x2000 /* - Force the attributes to be sync'd with the server */
> #define AT_STATX_DONT_SYNC 0x4000 /* - Don't sync attributes with the server */
>
> +#define AT_FSINFO_FROM_FSOPEN 0x2000 /* Examine the fs_context attached to dfd by fsopen() */
> +
> #define AT_RECURSIVE 0x8000 /* Apply to the entire subtree */
>
>
>
^ permalink raw reply
* Re: [RFC PATCH 1/1] Revert "rseq/selftests: arm: use udf instruction for RSEQ_SIG"
From: Will Deacon @ 2019-06-25 9:15 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: shuah, linux-kernel, Peter Zijlstra, Thomas Gleixner,
Joel Fernandes, Catalin Marinas, Dave Watson, Andi Kleen,
linux-kselftest, H. Peter Anvin, Chris Lameter, Russell King,
Michael Kerrisk, Paul E . McKenney, Paul Turner, Boqun Feng,
Josh Triplett, rostedt, Ben Maurer, linux-api, Andy
In-Reply-To: <1620037196.377.1561400426591.JavaMail.zimbra@efficios.com>
On Mon, Jun 24, 2019 at 02:20:26PM -0400, Mathieu Desnoyers wrote:
> ----- On Jun 24, 2019, at 1:24 PM, Will Deacon will.deacon@arm.com wrote:
>
> > On Mon, Jun 17, 2019 at 05:23:04PM +0200, Mathieu Desnoyers wrote:
> >> This reverts commit 2b845d4b4acd9422bbb668989db8dc36dfc8f438.
> >>
> >> That commit introduces build issues for programs compiled in Thumb mode.
> >> Rather than try to be clever and emit a valid trap instruction on arm32,
> >> which requires special care about big/little endian handling on that
> >> architecture, just emit plain data. Data in the instruction stream is
> >> technically expected on arm32: this is how literal pools are
> >> implemented. Reverting to the prior behavior does exactly that.
> >>
> >> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> >> CC: Peter Zijlstra <peterz@infradead.org>
> >> CC: Thomas Gleixner <tglx@linutronix.de>
> >> CC: Joel Fernandes <joelaf@google.com>
> >> CC: Catalin Marinas <catalin.marinas@arm.com>
> >> CC: Dave Watson <davejwatson@fb.com>
> >> CC: Will Deacon <will.deacon@arm.com>
> >> CC: Shuah Khan <shuah@kernel.org>
> >> CC: Andi Kleen <andi@firstfloor.org>
> >> CC: linux-kselftest@vger.kernel.org
> >> CC: "H . Peter Anvin" <hpa@zytor.com>
> >> CC: Chris Lameter <cl@linux.com>
> >> CC: Russell King <linux@arm.linux.org.uk>
> >> CC: Michael Kerrisk <mtk.manpages@gmail.com>
> >> CC: "Paul E . McKenney" <paulmck@linux.vnet.ibm.com>
> >> CC: Paul Turner <pjt@google.com>
> >> CC: Boqun Feng <boqun.feng@gmail.com>
> >> CC: Josh Triplett <josh@joshtriplett.org>
> >> CC: Steven Rostedt <rostedt@goodmis.org>
> >> CC: Ben Maurer <bmaurer@fb.com>
> >> CC: linux-api@vger.kernel.org
> >> CC: Andy Lutomirski <luto@amacapital.net>
> >> CC: Andrew Morton <akpm@linux-foundation.org>
> >> CC: Linus Torvalds <torvalds@linux-foundation.org>
> >> CC: Carlos O'Donell <carlos@redhat.com>
> >> CC: Florian Weimer <fweimer@redhat.com>
> >> ---
> >> tools/testing/selftests/rseq/rseq-arm.h | 52 ++-------------------------------
> >> 1 file changed, 2 insertions(+), 50 deletions(-)
> >>
> >> diff --git a/tools/testing/selftests/rseq/rseq-arm.h
> >> b/tools/testing/selftests/rseq/rseq-arm.h
> >> index 84f28f147fb6..5f262c54364f 100644
> >> --- a/tools/testing/selftests/rseq/rseq-arm.h
> >> +++ b/tools/testing/selftests/rseq/rseq-arm.h
> >> @@ -5,54 +5,7 @@
> >> * (C) Copyright 2016-2018 - Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> >> */
> >>
> >> -/*
> >> - * RSEQ_SIG uses the udf A32 instruction with an uncommon immediate operand
> >> - * value 0x5de3. This traps if user-space reaches this instruction by mistake,
> >> - * and the uncommon operand ensures the kernel does not move the instruction
> >> - * pointer to attacker-controlled code on rseq abort.
> >> - *
> >> - * The instruction pattern in the A32 instruction set is:
> >> - *
> >> - * e7f5def3 udf #24035 ; 0x5de3
> >> - *
> >> - * This translates to the following instruction pattern in the T16 instruction
> >> - * set:
> >> - *
> >> - * little endian:
> >> - * def3 udf #243 ; 0xf3
> >> - * e7f5 b.n <7f5>
> >> - *
> >> - * pre-ARMv6 big endian code:
> >> - * e7f5 b.n <7f5>
> >> - * def3 udf #243 ; 0xf3
> >> - *
> >> - * ARMv6+ -mbig-endian generates mixed endianness code vs data: little-endian
> >> - * code and big-endian data. Ensure the RSEQ_SIG data signature matches code
> >> - * endianness. Prior to ARMv6, -mbig-endian generates big-endian code and data
> >> - * (which match), so there is no need to reverse the endianness of the data
> >> - * representation of the signature. However, the choice between BE32 and BE8
> >> - * is done by the linker, so we cannot know whether code and data endianness
> >> - * will be mixed before the linker is invoked.
> >> - */
> >> -
> >> -#define RSEQ_SIG_CODE 0xe7f5def3
> >> -
> >> -#ifndef __ASSEMBLER__
> >> -
> >> -#define RSEQ_SIG_DATA \
> >> - ({ \
> >> - int sig; \
> >> - asm volatile ("b 2f\n\t" \
> >> - "1: .inst " __rseq_str(RSEQ_SIG_CODE) "\n\t" \
> >> - "2:\n\t" \
> >> - "ldr %[sig], 1b\n\t" \
> >> - : [sig] "=r" (sig)); \
> >> - sig; \
> >> - })
> >> -
> >> -#define RSEQ_SIG RSEQ_SIG_DATA
> >> -
> >> -#endif
> >> +#define RSEQ_SIG 0x53053053
> >
> > I don't get why you're reverting back to this old signature value, when the
> > one we came up with will work well when interpreted as an instruction in the
> > *vast* majority of scenarios that people care about (A32/T32 little-endian).
> > I think you might be under-estimating just how dead things like BE32 really
> > are.
>
> My issue is that the current .instr approach is broken for programs or functions
> built in Thumb mode, and I received no feedback on the solutions I proposed for
> those issues, which led me to propose a patch reverting to a simple .word.
I understand why you're moving from .inst to .word, but I don't understand
why that necessitates a change in the value. Why not .word 0xe7f5def3 ? You
could also flip the bytes around in case of big-endian, which would keep the
instruction coding clean for BE8.
> > That said, when you ran into .inst.n/.inst.w issues, did you try something
> > along the lines of the WASM() macro we use in arch/arm/, which adds the ".w"
> > suffix when targetting Thumb?
>
> AFAIU, the WASM macros depend on CONFIG_THUMB2_KERNEL, which may be fine within
> the kernel, but for user-space things are a bit more complex.
>
> A compile-unit can be compiled as thumb, which will set a compiler define
> which we could use to detect thumb mode. However, unfortunately, a single
> function can also be compiled with an attribute selecting thumb mode, which
> AFAIU does not influence the preprocessor defines.
Thanks, I hadn't considered that case. I don't know the right way to handle
that in the toolchain, so using .word is probably the best bet in the
absence of any better suggestions from the tools folks.
Will
^ permalink raw reply
* Re: [PATCH 01/25] vfs: syscall: Add fsinfo() to query filesystem information [ver #14]
From: Christian Brauner @ 2019-06-25 8:28 UTC (permalink / raw)
To: David Howells
Cc: viro, raven, mszeredi, linux-api, linux-fsdevel, linux-kernel
In-Reply-To: <156138533403.25627.4606280739806094239.stgit@warthog.procyon.org.uk>
On Mon, Jun 24, 2019 at 03:08:54PM +0100, David Howells wrote:
> Add a system call to allow filesystem information to be queried. A request
> value can be given to indicate the desired attribute. Support is provided
> for enumerating multi-value attributes.
>
> ===============
> NEW SYSTEM CALL
> ===============
>
> The new system call looks like:
>
> int ret = fsinfo(int dfd,
> const char *filename,
> const struct fsinfo_params *params,
> void *buffer,
> size_t buf_size);
>
> The params parameter optionally points to a block of parameters:
>
> struct fsinfo_params {
> __u32 at_flags;
> __u32 request;
> __u32 Nth;
> __u32 Mth;
> __u32 __reserved[6];
> };
>
> If params is NULL, it is assumed params->request should be
> fsinfo_attr_statfs, params->Nth should be 0, params->Mth should be 0 and
> params->at_flags should be 0.
>
> If params is given, all of params->__reserved[] must be 0.
>
> dfd, filename and params->at_flags indicate the file to query. There is no
> equivalent of lstat() as that can be emulated with fsinfo() by setting
> AT_SYMLINK_NOFOLLOW in params->at_flags. There is also no equivalent of
> fstat() as that can be emulated by passing a NULL filename to fsinfo() with
> the fd of interest in dfd. AT_NO_AUTOMOUNT can also be used to an allow
> automount point to be queried without triggering it.
>
> params->request indicates the attribute/attributes to be queried. This can
> be one of:
>
> FSINFO_ATTR_STATFS - statfs-style info
> FSINFO_ATTR_FSINFO - Information about fsinfo()
> FSINFO_ATTR_IDS - Filesystem IDs
> FSINFO_ATTR_LIMITS - Filesystem limits
> FSINFO_ATTR_SUPPORTS - What's supported in statx(), IOC flags
> FSINFO_ATTR_CAPABILITIES - Filesystem capabilities
> FSINFO_ATTR_TIMESTAMP_INFO - Inode timestamp info
> FSINFO_ATTR_VOLUME_ID - Volume ID (string)
> FSINFO_ATTR_VOLUME_UUID - Volume UUID
> FSINFO_ATTR_VOLUME_NAME - Volume name (string)
> FSINFO_ATTR_NAME_ENCODING - Filename encoding (string)
> FSINFO_ATTR_NAME_CODEPAGE - Filename codepage (string)
>
> Some attributes (such as the servers backing a network filesystem) can have
> multiple values. These can be enumerated by setting params->Nth and
> params->Mth to 0, 1, ... until ENODATA is returned.
>
> buffer and buf_size point to the reply buffer. The buffer is filled up to
> the specified size, even if this means truncating the reply. The full size
> of the reply is returned. In future versions, this will allow extra fields
> to be tacked on to the end of the reply, but anyone not expecting them will
> only get the subset they're expecting. If either buffer of buf_size are 0,
> no copy will take place and the data size will be returned.
>
> At the moment, this will only work on x86_64 and i386 as it requires the
> system call to be wired up.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: linux-api@vger.kernel.org
> ---
>
> arch/x86/entry/syscalls/syscall_32.tbl | 1
> arch/x86/entry/syscalls/syscall_64.tbl | 1
> fs/Kconfig | 7
> fs/Makefile | 1
> fs/fsinfo.c | 537 +++++++++++++++++++++++++++++++
> include/linux/fs.h | 5
> include/linux/fsinfo.h | 66 ++++
> include/linux/syscalls.h | 4
> include/uapi/asm-generic/unistd.h | 4
> include/uapi/linux/fsinfo.h | 219 +++++++++++++
> kernel/sys_ni.c | 1
> samples/vfs/Makefile | 4
> samples/vfs/test-fsinfo.c | 551 ++++++++++++++++++++++++++++++++
> 13 files changed, 1400 insertions(+), 1 deletion(-)
> create mode 100644 fs/fsinfo.c
> create mode 100644 include/linux/fsinfo.h
> create mode 100644 include/uapi/linux/fsinfo.h
> create mode 100644 samples/vfs/test-fsinfo.c
>
> diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
> index ad968b7bac72..03decae51513 100644
> --- a/arch/x86/entry/syscalls/syscall_32.tbl
> +++ b/arch/x86/entry/syscalls/syscall_32.tbl
> @@ -438,3 +438,4 @@
> 431 i386 fsconfig sys_fsconfig __ia32_sys_fsconfig
> 432 i386 fsmount sys_fsmount __ia32_sys_fsmount
> 433 i386 fspick sys_fspick __ia32_sys_fspick
> +434 i386 fsinfo sys_fsinfo __ia32_sys_fsinfo
> diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
> index b4e6f9e6204a..ea63df9a1020 100644
> --- a/arch/x86/entry/syscalls/syscall_64.tbl
> +++ b/arch/x86/entry/syscalls/syscall_64.tbl
> @@ -355,6 +355,7 @@
> 431 common fsconfig __x64_sys_fsconfig
> 432 common fsmount __x64_sys_fsmount
> 433 common fspick __x64_sys_fspick
> +434 common fsinfo __x64_sys_fsinfo
>
> #
> # x32-specific system call numbers start at 512 to avoid cache impact
> diff --git a/fs/Kconfig b/fs/Kconfig
> index cbbffc8b9ef5..9e7d2f2c0111 100644
> --- a/fs/Kconfig
> +++ b/fs/Kconfig
> @@ -15,6 +15,13 @@ config VALIDATE_FS_PARSER
> Enable this to perform validation of the parameter description for a
> filesystem when it is registered.
>
> +config FSINFO
> + bool "Enable the fsinfo() system call"
> + help
> + Enable the file system information querying system call to allow
> + comprehensive information to be retrieved about a filesystem,
> + superblock or mount object.
> +
> if BLOCK
>
> config FS_IOMAP
> diff --git a/fs/Makefile b/fs/Makefile
> index c9aea23aba56..26eaeae4b9a1 100644
> --- a/fs/Makefile
> +++ b/fs/Makefile
> @@ -53,6 +53,7 @@ obj-$(CONFIG_SYSCTL) += drop_caches.o
>
> obj-$(CONFIG_FHANDLE) += fhandle.o
> obj-$(CONFIG_FS_IOMAP) += iomap.o
> +obj-$(CONFIG_FSINFO) += fsinfo.o
>
> obj-y += quota/
>
> diff --git a/fs/fsinfo.c b/fs/fsinfo.c
> new file mode 100644
> index 000000000000..49b46f96dda3
> --- /dev/null
> +++ b/fs/fsinfo.c
> @@ -0,0 +1,537 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <linux/syscalls.h>
> +#include <linux/fs.h>
> +#include <linux/file.h>
> +#include <linux/mount.h>
> +#include <linux/namei.h>
> +#include <linux/statfs.h>
> +#include <linux/security.h>
> +#include <linux/uaccess.h>
> +#include <linux/fsinfo.h>
> +#include <uapi/linux/mount.h>
> +#include "internal.h"
> +
> +static u32 calc_mount_attrs(u32 mnt_flags)
> +{
> + u32 attrs = 0;
> +
> + if (mnt_flags & MNT_READONLY)
> + attrs |= MOUNT_ATTR_RDONLY;
> + if (mnt_flags & MNT_NOSUID)
> + attrs |= MOUNT_ATTR_NOSUID;
> + if (mnt_flags & MNT_NODEV)
> + attrs |= MOUNT_ATTR_NODEV;
> + if (mnt_flags & MNT_NOEXEC)
> + attrs |= MOUNT_ATTR_NOEXEC;
> + if (mnt_flags & MNT_NODIRATIME)
> + attrs |= MOUNT_ATTR_NODIRATIME;
> +
> + if (mnt_flags & MNT_NOATIME)
> + attrs |= MOUNT_ATTR_NOATIME;
> + else if (mnt_flags & MNT_RELATIME)
> + attrs |= MOUNT_ATTR_RELATIME;
> + else
> + attrs |= MOUNT_ATTR_STRICTATIME;
> + return attrs;
> +}
> +
> +/*
> + * Get basic filesystem stats from statfs.
> + */
> +static int fsinfo_generic_statfs(struct path *path, struct fsinfo_statfs *p)
> +{
> + struct kstatfs buf;
> + int ret;
> +
> + ret = vfs_statfs(path, &buf);
> + if (ret < 0)
> + return ret;
> +
> + p->f_blocks.hi = 0;
> + p->f_blocks.lo = buf.f_blocks;
> + p->f_bfree.hi = 0;
> + p->f_bfree.lo = buf.f_bfree;
> + p->f_bavail.hi = 0;
> + p->f_bavail.lo = buf.f_bavail;
> + p->f_files.hi = 0;
> + p->f_files.lo = buf.f_files;
> + p->f_ffree.hi = 0;
> + p->f_ffree.lo = buf.f_ffree;
> + p->f_favail.hi = 0;
> + p->f_favail.lo = buf.f_ffree;
> + p->f_bsize = buf.f_bsize;
> + p->f_frsize = buf.f_frsize;
> +
> + p->mnt_attrs = calc_mount_attrs(path->mnt->mnt_flags);
> + return sizeof(*p);
Hm, the discrepancy between the function signature returning int and
the sizeof operator most likely being size_t is bothering me. It
probably doesn't matter but maybe we can avoid that.
> +}
> +
> +static int fsinfo_generic_ids(struct path *path, struct fsinfo_ids *p)
> +{
> + struct super_block *sb;
> + struct kstatfs buf;
> + int ret;
> +
> + ret = vfs_statfs(path, &buf);
> + if (ret < 0 && ret != -ENOSYS)
> + return ret;
> +
> + sb = path->dentry->d_sb;
> + p->f_fstype = sb->s_magic;
> + p->f_dev_major = MAJOR(sb->s_dev);
> + p->f_dev_minor = MINOR(sb->s_dev);
> +
> + memcpy(&p->f_fsid, &buf.f_fsid, sizeof(p->f_fsid));
> + strlcpy(p->f_fs_name, path->dentry->d_sb->s_type->name,
> + sizeof(p->f_fs_name));
Truncation is acceptable or impossible I assume?
> + return sizeof(*p);
> +}
> +
> +static int fsinfo_generic_limits(struct path *path, struct fsinfo_limits *lim)
> +{
> + struct super_block *sb = path->dentry->d_sb;
> +
> + lim->max_file_size.hi = 0;
> + lim->max_file_size.lo = sb->s_maxbytes;
> + lim->max_hard_links = sb->s_max_links;
> + lim->max_uid = UINT_MAX;
> + lim->max_gid = UINT_MAX;
> + lim->max_projid = UINT_MAX;
> + lim->max_filename_len = NAME_MAX;
> + lim->max_symlink_len = PAGE_SIZE;
> + lim->max_xattr_name_len = XATTR_NAME_MAX;
> + lim->max_xattr_body_len = XATTR_SIZE_MAX;
> + lim->max_dev_major = 0xffffff;
> + lim->max_dev_minor = 0xff;
> + return sizeof(*lim);
> +}
> +
> +static int fsinfo_generic_supports(struct path *path, struct fsinfo_supports *c)
> +{
> + struct super_block *sb = path->dentry->d_sb;
> +
> + c->stx_mask = STATX_BASIC_STATS;
> + if (sb->s_d_op && sb->s_d_op->d_automount)
> + c->stx_attributes |= STATX_ATTR_AUTOMOUNT;
> + return sizeof(*c);
> +}
> +
> +static int fsinfo_generic_capabilities(struct path *path,
> + struct fsinfo_capabilities *c)
> +{
> + struct super_block *sb = path->dentry->d_sb;
> +
> + if (sb->s_mtd)
> + fsinfo_set_cap(c, FSINFO_CAP_IS_FLASH_FS);
> + else if (sb->s_bdev)
> + fsinfo_set_cap(c, FSINFO_CAP_IS_BLOCK_FS);
> +
> + if (sb->s_quota_types & QTYPE_MASK_USR)
> + fsinfo_set_cap(c, FSINFO_CAP_USER_QUOTAS);
> + if (sb->s_quota_types & QTYPE_MASK_GRP)
> + fsinfo_set_cap(c, FSINFO_CAP_GROUP_QUOTAS);
> + if (sb->s_quota_types & QTYPE_MASK_PRJ)
> + fsinfo_set_cap(c, FSINFO_CAP_PROJECT_QUOTAS);
> + if (sb->s_d_op && sb->s_d_op->d_automount)
> + fsinfo_set_cap(c, FSINFO_CAP_AUTOMOUNTS);
> + if (sb->s_id[0])
> + fsinfo_set_cap(c, FSINFO_CAP_VOLUME_ID);
> +
> + fsinfo_set_cap(c, FSINFO_CAP_HAS_ATIME);
> + fsinfo_set_cap(c, FSINFO_CAP_HAS_CTIME);
> + fsinfo_set_cap(c, FSINFO_CAP_HAS_MTIME);
> + return sizeof(*c);
> +}
> +
> +static const struct fsinfo_timestamp_info fsinfo_default_timestamp_info = {
> + .atime = {
> + .minimum = S64_MIN,
> + .maximum = S64_MAX,
> + .gran_mantissa = 1,
> + .gran_exponent = 0,
> + },
> + .mtime = {
> + .minimum = S64_MIN,
> + .maximum = S64_MAX,
> + .gran_mantissa = 1,
> + .gran_exponent = 0,
> + },
> + .ctime = {
> + .minimum = S64_MIN,
> + .maximum = S64_MAX,
> + .gran_mantissa = 1,
> + .gran_exponent = 0,
> + },
> + .btime = {
> + .minimum = S64_MIN,
> + .maximum = S64_MAX,
> + .gran_mantissa = 1,
> + .gran_exponent = 0,
> + },
> +};
> +
> +static int fsinfo_generic_timestamp_info(struct path *path,
> + struct fsinfo_timestamp_info *ts)
> +{
> + struct super_block *sb = path->dentry->d_sb;
> + s8 exponent;
> +
> + *ts = fsinfo_default_timestamp_info;
> +
> +
> + if (sb->s_time_gran < 1000000000) {
> + if (sb->s_time_gran < 1000)
> + exponent = -9;
> + else if (sb->s_time_gran < 1000000)
> + exponent = -6;
> + else
> + exponent = -3;
> +
> + ts->atime.gran_exponent = exponent;
> + ts->mtime.gran_exponent = exponent;
> + ts->ctime.gran_exponent = exponent;
> + ts->btime.gran_exponent = exponent;
> + }
> +
> + return sizeof(*ts);
> +}
> +
> +static int fsinfo_generic_volume_uuid(struct path *path,
> + struct fsinfo_volume_uuid *vu)
> +{
> + struct super_block *sb = path->dentry->d_sb;
> +
> + memcpy(vu, &sb->s_uuid, sizeof(*vu));
> + return sizeof(*vu);
> +}
> +
> +static int fsinfo_generic_volume_id(struct path *path, char *buf)
> +{
> + struct super_block *sb = path->dentry->d_sb;
> + size_t len = strlen(sb->s_id);
> +
> + memcpy(buf, sb->s_id, len + 1);
> + return len;
> +}
> +
> +static int fsinfo_generic_name_encoding(struct path *path, char *buf)
> +{
> + static const char encoding[] = "utf8";
> +
> + memcpy(buf, encoding, sizeof(encoding) - 1);
> + return sizeof(encoding) - 1;
> +}
> +
> +/*
> + * Implement some queries generically from stuff in the superblock.
> + */
> +int generic_fsinfo(struct path *path, struct fsinfo_kparams *params)
> +{
> +#define _gen(X, Y) FSINFO_ATTR_##X: return fsinfo_generic_##Y(path, params->buffer)
I'm really not sure that this helps readability in the switch below... :)
> +
> + switch (params->request) {
> + case _gen(STATFS, statfs);
> + case _gen(IDS, ids);
> + case _gen(LIMITS, limits);
> + case _gen(SUPPORTS, supports);
> + case _gen(CAPABILITIES, capabilities);
> + case _gen(TIMESTAMP_INFO, timestamp_info);
> + case _gen(VOLUME_UUID, volume_uuid);
> + case _gen(VOLUME_ID, volume_id);
> + case _gen(NAME_ENCODING, name_encoding);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +EXPORT_SYMBOL(generic_fsinfo);
> +
> +/*
> + * Retrieve the filesystem info. We make some stuff up if the operation is not
> + * supported.
> + */
> +static int vfs_fsinfo(struct path *path, struct fsinfo_kparams *params)
> +{
> + struct dentry *dentry = path->dentry;
> + int (*fsinfo)(struct path *, struct fsinfo_kparams *);
> + int ret;
> +
> + if (params->request == FSINFO_ATTR_FSINFO) {
> + struct fsinfo_fsinfo *info = params->buffer;
> +
> + info->max_attr = FSINFO_ATTR__NR;
> + info->max_cap = FSINFO_CAP__NR;
> + return sizeof(*info);
> + }
> +
> + fsinfo = dentry->d_sb->s_op->fsinfo;
> + if (!fsinfo) {
> + if (!dentry->d_sb->s_op->statfs)
> + return -EOPNOTSUPP;
> + fsinfo = generic_fsinfo;
> + }
> +
> + ret = security_sb_statfs(dentry);
> + if (ret)
> + return ret;
> +
> + if (!params->overlarge)
> + return fsinfo(path, params);
> +
> + while (!signal_pending(current)) {
> + params->usage = 0;
> + ret = fsinfo(path, params);
> + if (ret <= (int)params->buf_size)
He, and this is where the return value discrepancy hits again. Just
doesn't look nice tbh. :)
> + return ret; /* Error or it fitted */
> + kvfree(params->buffer);
That means callers should always memset fsinfo_kparams or this is an
invalid free...
> + params->buffer = NULL;
> + params->buf_size = roundup(ret, PAGE_SIZE);
> + if (params->buf_size > INT_MAX)
> + return -ETOOSMALL;
> + params->buffer = kvmalloc(params->buf_size, GFP_KERNEL);
> + if (!params->buffer)
> + return -ENOMEM;
> + }
> +
> + return -ERESTARTSYS;
> +}
> +
> +static int vfs_fsinfo_path(int dfd, const char __user *filename,
> + struct fsinfo_kparams *params)
> +{
> + struct path path;
> + unsigned lookup_flags = LOOKUP_FOLLOW | LOOKUP_AUTOMOUNT;
> + int ret = -EINVAL;
> +
> + if ((params->at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_NO_AUTOMOUNT |
> + AT_EMPTY_PATH)) != 0)
> + return -EINVAL;
> +
> + if (params->at_flags & AT_SYMLINK_NOFOLLOW)
> + lookup_flags &= ~LOOKUP_FOLLOW;
> + if (params->at_flags & AT_NO_AUTOMOUNT)
> + lookup_flags &= ~LOOKUP_AUTOMOUNT;
> + if (params->at_flags & AT_EMPTY_PATH)
> + lookup_flags |= LOOKUP_EMPTY;
> +
> +retry:
> + ret = user_path_at(dfd, filename, lookup_flags, &path);
> + if (ret)
> + goto out;
> +
> + ret = vfs_fsinfo(&path, params);
> + path_put(&path);
> + if (retry_estale(ret, lookup_flags)) {
> + lookup_flags |= LOOKUP_REVAL;
> + goto retry;
> + }
> +out:
> + return ret;
> +}
> +
> +static int vfs_fsinfo_fd(unsigned int fd, struct fsinfo_kparams *params)
> +{
> + struct fd f = fdget_raw(fd);
> + int ret = -EBADF;
> +
> + if (f.file) {
> + ret = vfs_fsinfo(&f.file->f_path, params);
> + fdput(f);
> + }
> + return ret;
> +}
> +
> +/*
> + * Return buffer information by requestable attribute.
> + *
> + * STRUCT indicates a fixed-size structure with only one instance.
> + * STRUCT_N indicates a 1D array of STRUCT, indexed by Nth
> + * STRUCT_NM indicates a 2D-array of STRUCT, indexed by Nth, Mth
> + * STRING indicates a string with only one instance.
> + * STRING_N indicates a 1D array of STRING, indexed by Nth
> + * STRING_NM indicates a 2D-array of STRING, indexed by Nth, Mth
> + * OPAQUE indicates a blob that can be larger than 4K.
> + * STRUCT_ARRAY indicates an array of structs that can be larger than 4K
I honestly have a hard time following the documentation here and that
monster table/macro thing below. For example, STRUCT_NM corresponds to
__FSINFO_NM or what? And is this uapi as you're using this in your
samples/test below?
> + *
> + * If an entry is marked STRUCT, STRUCT_N or STRUCT_NM then if no buffer is
> + * supplied to sys_fsinfo(), sys_fsinfo() will handle returning the buffer size
> + * without calling vfs_fsinfo() and the filesystem.
> + *
> + * No struct may have more than 4K bytes.
> + */
> +struct fsinfo_attr_info {
> + u8 type;
> + u8 flags;
> + u16 size;
> +};
> +
> +#define __FSINFO_STRUCT 0
> +#define __FSINFO_STRING 1
> +#define __FSINFO_OPAQUE 2
> +#define __FSINFO_STRUCT_ARRAY 3
> +#define __FSINFO_0 0
> +#define __FSINFO_N 0x0001
> +#define __FSINFO_NM 0x0002
> +
> +#define _Z(T, F, S) { .type = __FSINFO_##T, .flags = __FSINFO_##F, .size = S }
> +#define FSINFO_STRING(X,Y) [FSINFO_ATTR_##X] = _Z(STRING, 0, 0)
> +#define FSINFO_STRUCT(X,Y) [FSINFO_ATTR_##X] = _Z(STRUCT, 0, sizeof(struct fsinfo_##Y))
> +#define FSINFO_STRING_N(X,Y) [FSINFO_ATTR_##X] = _Z(STRING, N, 0)
> +#define FSINFO_STRUCT_N(X,Y) [FSINFO_ATTR_##X] = _Z(STRUCT, N, sizeof(struct fsinfo_##Y))
> +#define FSINFO_STRING_NM(X,Y) [FSINFO_ATTR_##X] = _Z(STRING, NM, 0)
> +#define FSINFO_STRUCT_NM(X,Y) [FSINFO_ATTR_##X] = _Z(STRUCT, NM, sizeof(struct fsinfo_##Y))
> +#define FSINFO_OPAQUE(X,Y) [FSINFO_ATTR_##X] = _Z(OPAQUE, 0, 0)
> +#define FSINFO_STRUCT_ARRAY(X,Y) [FSINFO_ATTR_##X] = _Z(STRUCT_ARRAY, 0, sizeof(struct fsinfo_##Y))
> +
> +static const struct fsinfo_attr_info fsinfo_buffer_info[FSINFO_ATTR__NR] = {
> + FSINFO_STRUCT (STATFS, statfs),
> + FSINFO_STRUCT (FSINFO, fsinfo),
> + FSINFO_STRUCT (IDS, ids),
> + FSINFO_STRUCT (LIMITS, limits),
> + FSINFO_STRUCT (CAPABILITIES, capabilities),
> + FSINFO_STRUCT (SUPPORTS, supports),
> + FSINFO_STRUCT (TIMESTAMP_INFO, timestamp_info),
> + FSINFO_STRING (VOLUME_ID, volume_id),
> + FSINFO_STRUCT (VOLUME_UUID, volume_uuid),
> + FSINFO_STRING (VOLUME_NAME, -),
> + FSINFO_STRING (NAME_ENCODING, -),
> + FSINFO_STRING (NAME_CODEPAGE, -),
> +};
Can I complain again that this is really annoying to parse.
> +
> +/**
> + * sys_fsinfo - System call to get filesystem information
> + * @dfd: Base directory to pathwalk from or fd referring to filesystem.
> + * @filename: Filesystem to query or NULL.
> + * @_params: Parameters to define request (or NULL for enhanced statfs).
> + * @user_buffer: Result buffer.
> + * @user_buf_size: Size of result buffer.
> + *
> + * Get information on a filesystem. The filesystem attribute to be queried is
> + * indicated by @_params->request, and some of the attributes can have multiple
> + * values, indexed by @_params->Nth and @_params->Mth. If @_params is NULL,
> + * then the 0th fsinfo_attr_statfs attribute is queried. If an attribute does
> + * not exist, EOPNOTSUPP is returned; if the Nth,Mth value does not exist,
> + * ENODATA is returned.
> + *
> + * On success, the size of the attribute's value is returned. If
> + * @user_buf_size is 0 or @user_buffer is NULL, only the size is returned. If
> + * the size of the value is larger than @user_buf_size, it will be truncated by
> + * the copy. If the size of the value is smaller than @user_buf_size then the
> + * excess buffer space will be cleared. The full size of the value will be
> + * returned, irrespective of how much data is actually placed in the buffer.
> + */
> +SYSCALL_DEFINE5(fsinfo,
> + int, dfd, const char __user *, filename,
> + struct fsinfo_params __user *, _params,
> + void __user *, user_buffer, size_t, user_buf_size)
> +{
> + struct fsinfo_attr_info info;
> + struct fsinfo_params user_params;
> + struct fsinfo_kparams params;
> + unsigned int result_size;
> + int ret;
> +
> + memset(¶ms, 0, sizeof(params));
> +
> + if (_params) {
> + if (copy_from_user(&user_params, _params, sizeof(user_params)))
> + return -EFAULT;
> + if (user_params.__reserved[0] ||
> + user_params.__reserved[1] ||
> + user_params.__reserved[2])
> + return -EINVAL;
Hm, aren't there 6 reserved fields?
> + if (user_params.request >= FSINFO_ATTR__NR)
> + return -EOPNOTSUPP;
> + params.at_flags = user_params.at_flags;
> + params.request = user_params.request;
> + params.Nth = user_params.Nth;
> + params.Mth = user_params.Mth;
> + } else {
> + params.request = FSINFO_ATTR_STATFS;
> + }
> +
> + if (!user_buffer || !user_buf_size) {
> + user_buf_size = 0;
> + user_buffer = NULL;
> + }
> +
> + /* Allocate an appropriately-sized buffer. We will truncate the
> + * contents when we write the contents back to userspace.
> + */
> + info = fsinfo_buffer_info[params.request];
> + if (params.Nth != 0 && !(info.flags & (__FSINFO_N | __FSINFO_NM)))
> + return -ENODATA;
> + if (params.Mth != 0 && !(info.flags & __FSINFO_NM))
> + return -ENODATA;
> +
> + switch (info.type) {
> + case __FSINFO_STRUCT:
> + params.buf_size = info.size;
> + if (user_buf_size == 0)
> + return info.size; /* We know how big the buffer should be */
> + break;
> +
> + case __FSINFO_STRING:
> + params.buf_size = 4096;
> + break;
> +
> + case __FSINFO_OPAQUE:
> + case __FSINFO_STRUCT_ARRAY:
> + /* Opaque blob or array of struct elements. We also create a
> + * buffer that can be used for scratch space.
> + */
> + ret = -ENOMEM;
> + params.scratch_buffer = kmalloc(4096, GFP_KERNEL);
> + if (!params.scratch_buffer)
> + goto error;
> + params.overlarge = true;
> + params.buf_size = 4096;
All the 4096 could probably be macros, FSATTR_GOOD_SIZE or whatever name
you might like.
> + break;
> +
> + default:
> + return -ENOBUFS;
> + }
> +
> + /* We always allocate a buffer for a string, even if buf_size == 0 and
> + * we're not going to return any data. This means that the filesystem
> + * code needn't care about whether the buffer actually exists or not.
> + */
> + ret = -ENOMEM;
> + params.buffer = kvzalloc(params.buf_size, GFP_KERNEL);
> + if (!params.buffer)
> + goto error_scratch;
> +
> + if (filename)
> + ret = vfs_fsinfo_path(dfd, filename, ¶ms);
> + else
> + ret = vfs_fsinfo_fd(dfd, ¶ms);
> + if (ret < 0)
> + goto error_buffer;
> +
> + result_size = ret;
> + if (result_size > user_buf_size)
> + result_size = user_buf_size;
> +
> + if (result_size > 0 &&
> + copy_to_user(user_buffer, params.buffer, result_size)) {
> + ret = -EFAULT;
> + goto error_buffer;
> + }
> +
> + /* Clear any part of the buffer that we won't fill if we're putting a
> + * struct in there. Strings, opaque objects and arrays are expected to
> + * be variable length.
> + */
> + if (info.type == __FSINFO_STRUCT &&
> + user_buf_size > result_size &&
> + clear_user(user_buffer + result_size, user_buf_size - result_size) != 0) {
For consistency you could error check the clear_user() the same way as
copy_to_user(), i.e.
if (copy_to_user()) and if (clear_user()) and not if (clear_user() != 0)
> + ret = -EFAULT;
> + goto error_buffer;
> + }
> +
> +error_buffer:
> + kvfree(params.buffer);
> +error_scratch:
> + kfree(params.scratch_buffer);
> +error:
> + return ret;
> +}
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 6a38b7124143..71ce3b054c42 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -66,6 +66,8 @@ struct fscrypt_info;
> struct fscrypt_operations;
> struct fs_context;
> struct fs_parameter_description;
> +struct fsinfo_kparams;
> +enum fsinfo_attribute;
>
> extern void __init inode_init(void);
> extern void __init inode_init_early(void);
> @@ -1922,6 +1924,9 @@ struct super_operations {
> int (*thaw_super) (struct super_block *);
> int (*unfreeze_fs) (struct super_block *);
> int (*statfs) (struct dentry *, struct kstatfs *);
> +#ifdef CONFIG_FSINFO
> + int (*fsinfo) (struct path *, struct fsinfo_kparams *);
> +#endif
> int (*remount_fs) (struct super_block *, int *, char *);
> void (*umount_begin) (struct super_block *);
>
> diff --git a/include/linux/fsinfo.h b/include/linux/fsinfo.h
> new file mode 100644
> index 000000000000..e17e4f0bae18
> --- /dev/null
> +++ b/include/linux/fsinfo.h
> @@ -0,0 +1,66 @@
> +/* Filesystem information query
> + *
> + * Copyright (C) 2018 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@redhat.com)
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public Licence
> + * as published by the Free Software Foundation; either version
> + * 2 of the Licence, or (at your option) any later version.
> + */
That should be an SPDX line I would expect. Otherwise Thomas and Greg
can do another round of conversions right after this patchset. :)
> +
> +#ifndef _LINUX_FSINFO_H
> +#define _LINUX_FSINFO_H
> +
> +#ifdef CONFIG_FSINFO
> +
> +#include <uapi/linux/fsinfo.h>
> +
> +struct fsinfo_kparams {
> + __u32 at_flags; /* AT_SYMLINK_NOFOLLOW and similar */
> + enum fsinfo_attribute request; /* What is being asking for */
> + __u32 Nth; /* Instance of it (some may have multiple) */
> + __u32 Mth; /* Subinstance */
> + bool overlarge; /* T if the buffer may be resized */
> + unsigned int usage; /* Amount of buffer used (overlarge=T) */
> + unsigned int buf_size; /* Size of ->buffer[] */
> + void *buffer; /* Where to place the reply */
> + char *scratch_buffer; /* 4K scratch buffer (overlarge=T) */
> +};
> +
> +extern int generic_fsinfo(struct path *, struct fsinfo_kparams *);
> +
> +static inline void fsinfo_set_cap(struct fsinfo_capabilities *c,
> + enum fsinfo_capability cap)
> +{
> + c->capabilities[cap / 8] |= 1 << (cap % 8);
> +}
> +
> +static inline void fsinfo_clear_cap(struct fsinfo_capabilities *c,
> + enum fsinfo_capability cap)
> +{
> + c->capabilities[cap / 8] &= ~(1 << (cap % 8));
> +}
> +
> +/**
> + * fsinfo_set_unix_caps - Set standard UNIX capabilities.
> + * @c: The capabilities mask to alter
> + */
> +static inline void fsinfo_set_unix_caps(struct fsinfo_capabilities *caps)
> +{
> + fsinfo_set_cap(caps, FSINFO_CAP_UIDS);
> + fsinfo_set_cap(caps, FSINFO_CAP_GIDS);
> + fsinfo_set_cap(caps, FSINFO_CAP_DIRECTORIES);
> + fsinfo_set_cap(caps, FSINFO_CAP_SYMLINKS);
> + fsinfo_set_cap(caps, FSINFO_CAP_HARD_LINKS);
> + fsinfo_set_cap(caps, FSINFO_CAP_DEVICE_FILES);
> + fsinfo_set_cap(caps, FSINFO_CAP_UNIX_SPECIALS);
> + fsinfo_set_cap(caps, FSINFO_CAP_SPARSE);
> + fsinfo_set_cap(caps, FSINFO_CAP_HAS_ATIME);
> + fsinfo_set_cap(caps, FSINFO_CAP_HAS_CTIME);
> + fsinfo_set_cap(caps, FSINFO_CAP_HAS_MTIME);
> +}
> +
> +#endif /* CONFIG_FSINFO */
> +
> +#endif /* _LINUX_FSINFO_H */
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index e2870fe1be5b..217d25b62b4f 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -50,6 +50,7 @@ struct stat64;
> struct statfs;
> struct statfs64;
> struct statx;
> +struct fsinfo_params;
> struct __sysctl_args;
> struct sysinfo;
> struct timespec;
> @@ -997,6 +998,9 @@ asmlinkage long sys_fspick(int dfd, const char __user *path, unsigned int flags)
> asmlinkage long sys_pidfd_send_signal(int pidfd, int sig,
> siginfo_t __user *info,
> unsigned int flags);
> +asmlinkage long sys_fsinfo(int dfd, const char __user *path,
> + struct fsinfo_params __user *params,
> + void __user *buffer, size_t buf_size);
Nit: There's a bunch of name inconsistency for the arguments between the
stub and the definition:
SYSCALL_DEFINE5(fsinfo,
int, dfd, const char __user *, filename,
struct fsinfo_params __user *, _params,
void __user *, user_buffer, size_t, user_buf_size)
>
> /*
> * Architecture-specific system calls
> diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
> index a87904daf103..50ddf5f25122 100644
> --- a/include/uapi/asm-generic/unistd.h
> +++ b/include/uapi/asm-generic/unistd.h
> @@ -844,9 +844,11 @@ __SYSCALL(__NR_fsconfig, sys_fsconfig)
> __SYSCALL(__NR_fsmount, sys_fsmount)
> #define __NR_fspick 433
> __SYSCALL(__NR_fspick, sys_fspick)
> +#define __NR_fsinfo 434
> +__SYSCALL(__NR_fsinfo, sys_fsinfo)
>
> #undef __NR_syscalls
> -#define __NR_syscalls 434
> +#define __NR_syscalls 435
>
> /*
> * 32 bit systems traditionally used different
> diff --git a/include/uapi/linux/fsinfo.h b/include/uapi/linux/fsinfo.h
> new file mode 100644
> index 000000000000..a7a7c731d992
> --- /dev/null
> +++ b/include/uapi/linux/fsinfo.h
> @@ -0,0 +1,219 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/* fsinfo() definitions.
> + *
> + * Copyright (C) 2018 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@redhat.com)
> + */
Do we do SPDX that way? Or isn't this just supposed to be:
// <spdxy stuff>
> +#ifndef _UAPI_LINUX_FSINFO_H
> +#define _UAPI_LINUX_FSINFO_H
> +
> +#include <linux/types.h>
> +#include <linux/socket.h>
> +
> +/*
> + * The filesystem attributes that can be requested. Note that some attributes
> + * may have multiple instances which can be switched in the parameter block.
> + */
> +enum fsinfo_attribute {
> + FSINFO_ATTR_STATFS = 0, /* statfs()-style state */
> + FSINFO_ATTR_FSINFO = 1, /* Information about fsinfo() */
> + FSINFO_ATTR_IDS = 2, /* Filesystem IDs */
> + FSINFO_ATTR_LIMITS = 3, /* Filesystem limits */
> + FSINFO_ATTR_SUPPORTS = 4, /* What's supported in statx, iocflags, ... */
> + FSINFO_ATTR_CAPABILITIES = 5, /* Filesystem capabilities (bits) */
> + FSINFO_ATTR_TIMESTAMP_INFO = 6, /* Inode timestamp info */
> + FSINFO_ATTR_VOLUME_ID = 7, /* Volume ID (string) */
> + FSINFO_ATTR_VOLUME_UUID = 8, /* Volume UUID (LE uuid) */
> + FSINFO_ATTR_VOLUME_NAME = 9, /* Volume name (string) */
> + FSINFO_ATTR_NAME_ENCODING = 10, /* Filename encoding (string) */
> + FSINFO_ATTR_NAME_CODEPAGE = 11, /* Filename codepage (string) */
> + FSINFO_ATTR__NR
Nit/Bikeshed: FSINFO_ATTR_MAX? Seems more intuitive.
> +};
> +
> +/*
> + * Optional fsinfo() parameter structure.
> + *
> + * If this is not given, it is assumed that fsinfo_attr_statfs instance 0,0 is
> + * desired.
> + */
> +struct fsinfo_params {
> + __u32 at_flags; /* AT_SYMLINK_NOFOLLOW and similar flags */
> + __u32 request; /* What is being asking for (enum fsinfo_attribute) */
> + __u32 Nth; /* Instance of it (some may have multiple) */
> + __u32 Mth; /* Subinstance of Nth instance */
> + __u64 __reserved[3]; /* Reserved params; all must be 0 */
Oh, so your commit message uses __reserved[6] and here it's
__reserved[3] and your error check above also only validates
__reserved[3]. Should probably make this consistent. :)
> +};
> +
> +struct fsinfo_u128 {
> +#if defined(__BYTE_ORDER) ? __BYTE_ORDER == __BIG_ENDIAN : defined(__BIG_ENDIAN)
> + __u64 hi;
> + __u64 lo;
> +#elif defined(__BYTE_ORDER) ? __BYTE_ORDER == __LITTLE_ENDIAN : defined(__LITTLE_ENDIAN)
> + __u64 lo;
> + __u64 hi;
> +#endif
> +};
Hm, I know why you do this custom fsinfo_u128 thingy but for userspace
that is going to be annoying to operate with, e.g. comparing the
size/space of two filesystems etc.
> +
> +/*
> + * Information struct for fsinfo(fsinfo_attr_statfs).
> + * - This gives extended filesystem information.
> + */
> +struct fsinfo_statfs {
> + struct fsinfo_u128 f_blocks; /* Total number of blocks in fs */
> + struct fsinfo_u128 f_bfree; /* Total number of free blocks */
> + struct fsinfo_u128 f_bavail; /* Number of free blocks available to ordinary user */
> + struct fsinfo_u128 f_files; /* Total number of file nodes in fs */
> + struct fsinfo_u128 f_ffree; /* Number of free file nodes */
> + struct fsinfo_u128 f_favail; /* Number of file nodes available to ordinary user */
> + __u64 f_bsize; /* Optimal block size */
> + __u64 f_frsize; /* Fragment size */
> + __u64 mnt_attrs; /* Mount attributes (MOUNT_ATTR_*) */
> +};
> +
> +/*
> + * Information struct for fsinfo(fsinfo_attr_ids).
> + *
> + * List of basic identifiers as is normally found in statfs().
> + */
> +struct fsinfo_ids {
> + char f_fs_name[15 + 1]; /* Filesystem name */
You should probably make this a macro so userspace can use it in fs-name
length checks too.
> + __u64 f_fsid; /* Short 64-bit Filesystem ID (as statfs) */
> + __u64 f_sb_id; /* Internal superblock ID for sbnotify()/mntnotify() */
> + __u32 f_fstype; /* Filesystem type from linux/magic.h [uncond] */
> + __u32 f_dev_major; /* As st_dev_* from struct statx [uncond] */
> + __u32 f_dev_minor;
> + __u32 __reserved[1];
> +};
> +
> +/*
> + * Information struct for fsinfo(fsinfo_attr_limits).
> + *
> + * List of supported filesystem limits.
> + */
> +struct fsinfo_limits {
> + struct fsinfo_u128 max_file_size; /* Maximum file size */
> + struct fsinfo_u128 max_ino; /* Maximum inode number */
> + __u64 max_uid; /* Maximum UID supported */
> + __u64 max_gid; /* Maximum GID supported */
> + __u64 max_projid; /* Maximum project ID supported */
> + __u64 max_hard_links; /* Maximum number of hard links on a file */
> + __u64 max_xattr_body_len; /* Maximum xattr content length */
> + __u32 max_xattr_name_len; /* Maximum xattr name length */
> + __u32 max_filename_len; /* Maximum filename length */
> + __u32 max_symlink_len; /* Maximum symlink content length */
> + __u32 max_dev_major; /* Maximum device major representable */
> + __u32 max_dev_minor; /* Maximum device minor representable */
> + __u32 __reserved[1];
> +};
> +
> +/*
> + * Information struct for fsinfo(fsinfo_attr_supports).
> + *
> + * What's supported in various masks, such as statx() attribute and mask bits
> + * and IOC flags.
> + */
> +struct fsinfo_supports {
> + __u64 stx_attributes; /* What statx::stx_attributes are supported */
> + __u32 stx_mask; /* What statx::stx_mask bits are supported */
> + __u32 ioc_flags; /* What FS_IOC_* flags are supported */
> + __u32 win_file_attrs; /* What DOS/Windows FILE_* attributes are supported */
> + __u32 __reserved[1];
> +};
> +
> +/*
> + * Information struct for fsinfo(fsinfo_attr_capabilities).
> + *
> + * Bitmask indicating filesystem capabilities where renderable as single bits.
> + */
> +enum fsinfo_capability {
> + FSINFO_CAP_IS_KERNEL_FS = 0, /* fs is kernel-special filesystem */
> + FSINFO_CAP_IS_BLOCK_FS = 1, /* fs is block-based filesystem */
> + FSINFO_CAP_IS_FLASH_FS = 2, /* fs is flash filesystem */
> + FSINFO_CAP_IS_NETWORK_FS = 3, /* fs is network filesystem */
> + FSINFO_CAP_IS_AUTOMOUNTER_FS = 4, /* fs is automounter special filesystem */
> + FSINFO_CAP_IS_MEMORY_FS = 5, /* fs is memory-based filesystem */
> + FSINFO_CAP_AUTOMOUNTS = 6, /* fs supports automounts */
> + FSINFO_CAP_ADV_LOCKS = 7, /* fs supports advisory file locking */
> + FSINFO_CAP_MAND_LOCKS = 8, /* fs supports mandatory file locking */
> + FSINFO_CAP_LEASES = 9, /* fs supports file leases */
> + FSINFO_CAP_UIDS = 10, /* fs supports numeric uids */
> + FSINFO_CAP_GIDS = 11, /* fs supports numeric gids */
> + FSINFO_CAP_PROJIDS = 12, /* fs supports numeric project ids */
> + FSINFO_CAP_STRING_USER_IDS = 13, /* fs supports string user identifiers */
> + FSINFO_CAP_GUID_USER_IDS = 14, /* fs supports GUID user identifiers */
> + FSINFO_CAP_WINDOWS_ATTRS = 15, /* fs has windows attributes */
> + FSINFO_CAP_USER_QUOTAS = 16, /* fs has per-user quotas */
> + FSINFO_CAP_GROUP_QUOTAS = 17, /* fs has per-group quotas */
> + FSINFO_CAP_PROJECT_QUOTAS = 18, /* fs has per-project quotas */
> + FSINFO_CAP_XATTRS = 19, /* fs has xattrs */
> + FSINFO_CAP_JOURNAL = 20, /* fs has a journal */
> + FSINFO_CAP_DATA_IS_JOURNALLED = 21, /* fs is using data journalling */
> + FSINFO_CAP_O_SYNC = 22, /* fs supports O_SYNC */
> + FSINFO_CAP_O_DIRECT = 23, /* fs supports O_DIRECT */
> + FSINFO_CAP_VOLUME_ID = 24, /* fs has a volume ID */
> + FSINFO_CAP_VOLUME_UUID = 25, /* fs has a volume UUID */
> + FSINFO_CAP_VOLUME_NAME = 26, /* fs has a volume name */
> + FSINFO_CAP_VOLUME_FSID = 27, /* fs has a volume FSID */
> + FSINFO_CAP_IVER_ALL_CHANGE = 28, /* i_version represents data + meta changes */
> + FSINFO_CAP_IVER_DATA_CHANGE = 29, /* i_version represents data changes only */
> + FSINFO_CAP_IVER_MONO_INCR = 30, /* i_version incremented monotonically */
> + FSINFO_CAP_DIRECTORIES = 31, /* fs supports (sub)directories */
> + FSINFO_CAP_SYMLINKS = 32, /* fs supports symlinks */
> + FSINFO_CAP_HARD_LINKS = 33, /* fs supports hard links */
> + FSINFO_CAP_HARD_LINKS_1DIR = 34, /* fs supports hard links in same dir only */
> + FSINFO_CAP_DEVICE_FILES = 35, /* fs supports bdev, cdev */
> + FSINFO_CAP_UNIX_SPECIALS = 36, /* fs supports pipe, fifo, socket */
> + FSINFO_CAP_RESOURCE_FORKS = 37, /* fs supports resource forks/streams */
> + FSINFO_CAP_NAME_CASE_INDEP = 38, /* Filename case independence is mandatory */
> + FSINFO_CAP_NAME_NON_UTF8 = 39, /* fs has non-utf8 names */
> + FSINFO_CAP_NAME_HAS_CODEPAGE = 40, /* fs has a filename codepage */
> + FSINFO_CAP_SPARSE = 41, /* fs supports sparse files */
> + FSINFO_CAP_NOT_PERSISTENT = 42, /* fs is not persistent */
> + FSINFO_CAP_NO_UNIX_MODE = 43, /* fs does not support unix mode bits */
> + FSINFO_CAP_HAS_ATIME = 44, /* fs supports access time */
> + FSINFO_CAP_HAS_BTIME = 45, /* fs supports birth/creation time */
> + FSINFO_CAP_HAS_CTIME = 46, /* fs supports change time */
> + FSINFO_CAP_HAS_MTIME = 47, /* fs supports modification time */
> + FSINFO_CAP__NR
Hm, again, maybe better to use FSINFO_CAP_MAX?
> +};
> +
> +struct fsinfo_capabilities {
> + __u8 capabilities[(FSINFO_CAP__NR + 7) / 8];
> +};
> +
> +struct fsinfo_timestamp_one {
> + __s64 minimum; /* Minimum timestamp value in seconds */
> + __u64 maximum; /* Maximum timestamp value in seconds */
> + __u16 gran_mantissa; /* Granularity(secs) = mant * 10^exp */
> + __s8 gran_exponent;
> + __u8 reserved[5];
> +};
> +
> +/*
> + * Information struct for fsinfo(fsinfo_attr_timestamp_info).
> + */
> +struct fsinfo_timestamp_info {
> + struct fsinfo_timestamp_one atime; /* Access time */
> + struct fsinfo_timestamp_one mtime; /* Modification time */
> + struct fsinfo_timestamp_one ctime; /* Change time */
> + struct fsinfo_timestamp_one btime; /* Birth/creation time */
> +};
> +
> +/*
> + * Information struct for fsinfo(fsinfo_attr_volume_uuid).
> + */
> +struct fsinfo_volume_uuid {
> + __u8 uuid[16];
> +};
> +
> +/*
> + * Information struct for fsinfo(fsinfo_attr_fsinfo).
> + *
> + * This gives information about fsinfo() itself.
> + */
> +struct fsinfo_fsinfo {
> + __u32 max_attr; /* Number of supported attributes (fsinfo_attr__nr) */
> + __u32 max_cap; /* Number of supported capabilities (fsinfo_cap__nr) */
> +};
> +
> +#endif /* _UAPI_LINUX_FSINFO_H */
> diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
> index 4d9ae5ea6caf..93927072396c 100644
> --- a/kernel/sys_ni.c
> +++ b/kernel/sys_ni.c
> @@ -51,6 +51,7 @@ COND_SYSCALL_COMPAT(io_pgetevents);
> COND_SYSCALL(io_uring_setup);
> COND_SYSCALL(io_uring_enter);
> COND_SYSCALL(io_uring_register);
> +COND_SYSCALL(fsinfo);
>
> /* fs/xattr.c */
>
> diff --git a/samples/vfs/Makefile b/samples/vfs/Makefile
> index a3e4ffd4c773..d3cc8e9a4fd8 100644
> --- a/samples/vfs/Makefile
> +++ b/samples/vfs/Makefile
> @@ -1,10 +1,14 @@
> # List of programs to build
> hostprogs-y := \
> + test-fsinfo \
> test-fsmount \
> test-statx
>
> # Tell kbuild to always build the programs
> always := $(hostprogs-y)
>
> +HOSTCFLAGS_test-fsinfo.o += -I$(objtree)/usr/include
> +HOSTLDLIBS_test-fsinfo += -lm
> +
> HOSTCFLAGS_test-fsmount.o += -I$(objtree)/usr/include
> HOSTCFLAGS_test-statx.o += -I$(objtree)/usr/include
> diff --git a/samples/vfs/test-fsinfo.c b/samples/vfs/test-fsinfo.c
> new file mode 100644
> index 000000000000..8cce1986df7e
> --- /dev/null
> +++ b/samples/vfs/test-fsinfo.c
> @@ -0,0 +1,551 @@
> +/* Test the fsinfo() system call
> + *
> + * Copyright (C) 2018 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@redhat.com)
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public Licence
> + * as published by the Free Software Foundation; either version
> + * 2 of the Licence, or (at your option) any later version.
> + */
> +
> +#define _GNU_SOURCE
> +#define _ATFILE_SOURCE
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <stdint.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <ctype.h>
> +#include <errno.h>
> +#include <time.h>
> +#include <math.h>
> +#include <fcntl.h>
> +#include <sys/syscall.h>
> +#include <linux/fsinfo.h>
> +#include <linux/socket.h>
> +#include <sys/stat.h>
> +#include <arpa/inet.h>
> +
> +#ifndef __NR_fsinfo
> +#define __NR_fsinfo -1
> +#endif
> +
> +static bool debug = 0;
> +
> +static __attribute__((unused))
> +ssize_t fsinfo(int dfd, const char *filename, struct fsinfo_params *params,
> + void *buffer, size_t buf_size)
> +{
> + return syscall(__NR_fsinfo, dfd, filename, params, buffer, buf_size);
> +}
> +
> +struct fsinfo_attr_info {
> + unsigned char type;
> + unsigned char flags;
> + unsigned short size;
> +};
> +
> +#define __FSINFO_STRUCT 0
> +#define __FSINFO_STRING 1
> +#define __FSINFO_OVER 2
> +#define __FSINFO_STRUCT_ARRAY 3
> +#define __FSINFO_0 0
> +#define __FSINFO_N 0x0001
> +#define __FSINFO_NM 0x0002
> +
> +#define _Z(T, F, S) { .type = __FSINFO_##T, .flags = __FSINFO_##F, .size = S }
> +#define FSINFO_STRING(X,Y) [FSINFO_ATTR_##X] = _Z(STRING, 0, 0)
> +#define FSINFO_STRUCT(X,Y) [FSINFO_ATTR_##X] = _Z(STRUCT, 0, sizeof(struct fsinfo_##Y))
> +#define FSINFO_STRING_N(X,Y) [FSINFO_ATTR_##X] = _Z(STRING, N, 0)
> +#define FSINFO_STRUCT_N(X,Y) [FSINFO_ATTR_##X] = _Z(STRUCT, N, sizeof(struct fsinfo_##Y))
> +#define FSINFO_STRING_NM(X,Y) [FSINFO_ATTR_##X] = _Z(STRING, NM, 0)
> +#define FSINFO_STRUCT_NM(X,Y) [FSINFO_ATTR_##X] = _Z(STRUCT, NM, sizeof(struct fsinfo_##Y))
> +#define FSINFO_OVERLARGE(X,Y) [FSINFO_ATTR_##X] = _Z(OVER, 0, 0)
> +#define FSINFO_STRUCT_ARRAY(X,Y) [FSINFO_ATTR_##X] = _Z(STRUCT_ARRAY, 0, sizeof(struct fsinfo_##Y))
> +
> +static const struct fsinfo_attr_info fsinfo_buffer_info[FSINFO_ATTR__NR] = {
> + FSINFO_STRUCT (STATFS, statfs),
> + FSINFO_STRUCT (FSINFO, fsinfo),
> + FSINFO_STRUCT (IDS, ids),
> + FSINFO_STRUCT (LIMITS, limits),
> + FSINFO_STRUCT (CAPABILITIES, capabilities),
> + FSINFO_STRUCT (SUPPORTS, supports),
> + FSINFO_STRUCT (TIMESTAMP_INFO, timestamp_info),
> + FSINFO_STRING (VOLUME_ID, volume_id),
> + FSINFO_STRUCT (VOLUME_UUID, volume_uuid),
> + FSINFO_STRING (VOLUME_NAME, volume_name),
> + FSINFO_STRING (NAME_ENCODING, name_encoding),
> + FSINFO_STRING (NAME_CODEPAGE, name_codepage),
> +};
> +
> +#define FSINFO_NAME(X,Y) [FSINFO_ATTR_##X] = #Y
> +static const char *fsinfo_attr_names[FSINFO_ATTR__NR] = {
> + FSINFO_NAME (STATFS, statfs),
> + FSINFO_NAME (FSINFO, fsinfo),
> + FSINFO_NAME (IDS, ids),
> + FSINFO_NAME (LIMITS, limits),
> + FSINFO_NAME (CAPABILITIES, capabilities),
> + FSINFO_NAME (SUPPORTS, supports),
> + FSINFO_NAME (TIMESTAMP_INFO, timestamp_info),
> + FSINFO_NAME (VOLUME_ID, volume_id),
> + FSINFO_NAME (VOLUME_UUID, volume_uuid),
> + FSINFO_NAME (VOLUME_NAME, volume_name),
> + FSINFO_NAME (NAME_ENCODING, name_encoding),
> + FSINFO_NAME (NAME_CODEPAGE, name_codepage),
> +};
> +
> +union reply {
> + char buffer[4096];
> + struct fsinfo_statfs statfs;
> + struct fsinfo_fsinfo fsinfo;
> + struct fsinfo_ids ids;
> + struct fsinfo_limits limits;
> + struct fsinfo_supports supports;
> + struct fsinfo_capabilities caps;
> + struct fsinfo_timestamp_info timestamps;
> + struct fsinfo_volume_uuid uuid;
> +};
> +
> +static void dump_hex(unsigned int *data, int from, int to)
> +{
> + unsigned offset, print_offset = 1, col = 0;
> +
> + from /= 4;
> + to = (to + 3) / 4;
> +
> + for (offset = from; offset < to; offset++) {
> + if (print_offset) {
> + printf("%04x: ", offset * 8);
> + print_offset = 0;
> + }
> + printf("%08x", data[offset]);
> + col++;
> + if ((col & 3) == 0) {
> + printf("\n");
> + print_offset = 1;
> + } else {
> + printf(" ");
> + }
> + }
> +
> + if (!print_offset)
> + printf("\n");
> +}
> +
> +static void dump_attr_STATFS(union reply *r, int size)
> +{
> + struct fsinfo_statfs *f = &r->statfs;
> +
> + printf("\n");
> + printf("\tblocks: n=%llu fr=%llu av=%llu\n",
> + (unsigned long long)f->f_blocks.lo,
> + (unsigned long long)f->f_bfree.lo,
> + (unsigned long long)f->f_bavail.lo);
> +
> + printf("\tfiles : n=%llu fr=%llu av=%llu\n",
> + (unsigned long long)f->f_files.lo,
> + (unsigned long long)f->f_ffree.lo,
> + (unsigned long long)f->f_favail.lo);
> + printf("\tbsize : %llu\n", f->f_bsize);
> + printf("\tfrsize: %llu\n", f->f_frsize);
> + printf("\tmntfl : %llx\n", (unsigned long long)f->mnt_attrs);
> +}
> +
> +static void dump_attr_FSINFO(union reply *r, int size)
> +{
> + struct fsinfo_fsinfo *f = &r->fsinfo;
> +
> + printf("max_attr=%u max_cap=%u\n", f->max_attr, f->max_cap);
> +}
> +
> +static void dump_attr_IDS(union reply *r, int size)
> +{
> + struct fsinfo_ids *f = &r->ids;
> +
> + printf("\n");
> + printf("\tdev : %02x:%02x\n", f->f_dev_major, f->f_dev_minor);
> + printf("\tfs : type=%x name=%s\n", f->f_fstype, f->f_fs_name);
> + printf("\tfsid : %llx\n", (unsigned long long)f->f_fsid);
> +}
> +
> +static void dump_attr_LIMITS(union reply *r, int size)
> +{
> + struct fsinfo_limits *f = &r->limits;
> +
> + printf("\n");
> + printf("\tmax file size: %llx%016llx\n",
> + (unsigned long long)f->max_file_size.hi,
> + (unsigned long long)f->max_file_size.lo);
> + printf("\tmax ino: %llx%016llx\n",
> + (unsigned long long)f->max_ino.hi,
> + (unsigned long long)f->max_ino.lo);
> + printf("\tmax ids : u=%llx g=%llx p=%llx\n",
> + (unsigned long long)f->max_uid,
> + (unsigned long long)f->max_gid,
> + (unsigned long long)f->max_projid);
> + printf("\tmax dev : maj=%x min=%x\n",
> + f->max_dev_major, f->max_dev_minor);
> + printf("\tmax links : %llx\n",
> + (unsigned long long)f->max_hard_links);
> + printf("\tmax xattr : n=%x b=%llx\n",
> + f->max_xattr_name_len,
> + (unsigned long long)f->max_xattr_body_len);
> + printf("\tmax len : file=%x sym=%x\n",
> + f->max_filename_len, f->max_symlink_len);
> +}
> +
> +static void dump_attr_SUPPORTS(union reply *r, int size)
> +{
> + struct fsinfo_supports *f = &r->supports;
> +
> + printf("\n");
> + printf("\tstx_attr=%llx\n", (unsigned long long)f->stx_attributes);
> + printf("\tstx_mask=%x\n", f->stx_mask);
> + printf("\tioc_flags=%x\n", f->ioc_flags);
> + printf("\twin_fattrs=%x\n", f->win_file_attrs);
> +}
> +
> +#define FSINFO_CAP_NAME(C) [FSINFO_CAP_##C] = #C
> +static const char *fsinfo_cap_names[FSINFO_CAP__NR] = {
> + FSINFO_CAP_NAME(IS_KERNEL_FS),
> + FSINFO_CAP_NAME(IS_BLOCK_FS),
> + FSINFO_CAP_NAME(IS_FLASH_FS),
> + FSINFO_CAP_NAME(IS_NETWORK_FS),
> + FSINFO_CAP_NAME(IS_AUTOMOUNTER_FS),
> + FSINFO_CAP_NAME(IS_MEMORY_FS),
> + FSINFO_CAP_NAME(AUTOMOUNTS),
> + FSINFO_CAP_NAME(ADV_LOCKS),
> + FSINFO_CAP_NAME(MAND_LOCKS),
> + FSINFO_CAP_NAME(LEASES),
> + FSINFO_CAP_NAME(UIDS),
> + FSINFO_CAP_NAME(GIDS),
> + FSINFO_CAP_NAME(PROJIDS),
> + FSINFO_CAP_NAME(STRING_USER_IDS),
> + FSINFO_CAP_NAME(GUID_USER_IDS),
> + FSINFO_CAP_NAME(WINDOWS_ATTRS),
> + FSINFO_CAP_NAME(USER_QUOTAS),
> + FSINFO_CAP_NAME(GROUP_QUOTAS),
> + FSINFO_CAP_NAME(PROJECT_QUOTAS),
> + FSINFO_CAP_NAME(XATTRS),
> + FSINFO_CAP_NAME(JOURNAL),
> + FSINFO_CAP_NAME(DATA_IS_JOURNALLED),
> + FSINFO_CAP_NAME(O_SYNC),
> + FSINFO_CAP_NAME(O_DIRECT),
> + FSINFO_CAP_NAME(VOLUME_ID),
> + FSINFO_CAP_NAME(VOLUME_UUID),
> + FSINFO_CAP_NAME(VOLUME_NAME),
> + FSINFO_CAP_NAME(VOLUME_FSID),
> + FSINFO_CAP_NAME(IVER_ALL_CHANGE),
> + FSINFO_CAP_NAME(IVER_DATA_CHANGE),
> + FSINFO_CAP_NAME(IVER_MONO_INCR),
> + FSINFO_CAP_NAME(DIRECTORIES),
> + FSINFO_CAP_NAME(SYMLINKS),
> + FSINFO_CAP_NAME(HARD_LINKS),
> + FSINFO_CAP_NAME(HARD_LINKS_1DIR),
> + FSINFO_CAP_NAME(DEVICE_FILES),
> + FSINFO_CAP_NAME(UNIX_SPECIALS),
> + FSINFO_CAP_NAME(RESOURCE_FORKS),
> + FSINFO_CAP_NAME(NAME_CASE_INDEP),
> + FSINFO_CAP_NAME(NAME_NON_UTF8),
> + FSINFO_CAP_NAME(NAME_HAS_CODEPAGE),
> + FSINFO_CAP_NAME(SPARSE),
> + FSINFO_CAP_NAME(NOT_PERSISTENT),
> + FSINFO_CAP_NAME(NO_UNIX_MODE),
> + FSINFO_CAP_NAME(HAS_ATIME),
> + FSINFO_CAP_NAME(HAS_BTIME),
> + FSINFO_CAP_NAME(HAS_CTIME),
> + FSINFO_CAP_NAME(HAS_MTIME),
> +};
> +
> +static void dump_attr_CAPABILITIES(union reply *r, int size)
> +{
> + struct fsinfo_capabilities *f = &r->caps;
> + int i;
> +
> + for (i = 0; i < sizeof(f->capabilities); i++)
> + printf("%02x", f->capabilities[i]);
> + printf("\n");
> + for (i = 0; i < FSINFO_CAP__NR; i++)
> + if (f->capabilities[i / 8] & (1 << (i % 8)))
> + printf("\t- %s\n", fsinfo_cap_names[i]);
> +}
> +
> +static void print_time(struct fsinfo_timestamp_one *t, char stamp)
> +{
> + printf("\t%ctime : gran=%gs range=%llx-%llx\n",
> + stamp,
> + t->gran_mantissa * pow(10., t->gran_exponent),
> + (long long)t->minimum,
> + (long long)t->maximum);
> +}
> +
> +static void dump_attr_TIMESTAMP_INFO(union reply *r, int size)
> +{
> + struct fsinfo_timestamp_info *f = &r->timestamps;
> +
> + printf("\n");
> + print_time(&f->atime, 'a');
> + print_time(&f->mtime, 'm');
> + print_time(&f->ctime, 'c');
> + print_time(&f->btime, 'b');
> +}
> +
> +static void dump_attr_VOLUME_UUID(union reply *r, int size)
> +{
> + struct fsinfo_volume_uuid *f = &r->uuid;
> +
> + printf("%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x"
> + "-%02x%02x%02x%02x%02x%02x\n",
> + f->uuid[ 0], f->uuid[ 1],
> + f->uuid[ 2], f->uuid[ 3],
> + f->uuid[ 4], f->uuid[ 5],
> + f->uuid[ 6], f->uuid[ 7],
> + f->uuid[ 8], f->uuid[ 9],
> + f->uuid[10], f->uuid[11],
> + f->uuid[12], f->uuid[13],
> + f->uuid[14], f->uuid[15]);
> +}
> +
> +/*
> + *
> + */
> +typedef void (*dumper_t)(union reply *r, int size);
> +
> +#define FSINFO_DUMPER(N) [FSINFO_ATTR_##N] = dump_attr_##N
> +static const dumper_t fsinfo_attr_dumper[FSINFO_ATTR__NR] = {
> + FSINFO_DUMPER(STATFS),
> + FSINFO_DUMPER(FSINFO),
> + FSINFO_DUMPER(IDS),
> + FSINFO_DUMPER(LIMITS),
> + FSINFO_DUMPER(SUPPORTS),
> + FSINFO_DUMPER(CAPABILITIES),
> + FSINFO_DUMPER(TIMESTAMP_INFO),
> + FSINFO_DUMPER(VOLUME_UUID),
> +};
> +
> +static void dump_fsinfo(enum fsinfo_attribute attr,
> + struct fsinfo_attr_info about,
> + union reply *r, int size)
> +{
> + dumper_t dumper = fsinfo_attr_dumper[attr];
> + unsigned int len;
> +
> + if (!dumper) {
> + printf("<no dumper>\n");
> + return;
> + }
> +
> + len = about.size;
> + if (about.type == __FSINFO_STRUCT && size < len) {
> + printf("<short data %u/%u>\n", size, len);
> + return;
> + }
> +
> + dumper(r, size);
> +}
> +
> +/*
> + * Try one subinstance of an attribute.
> + */
> +static int try_one(const char *file, struct fsinfo_params *params, bool raw)
> +{
> + struct fsinfo_attr_info about;
> + union reply *r;
> + size_t buf_size = 4096;
> + char *p;
> + int ret;
> +
> + for (;;) {
> + r = malloc(buf_size);
> + if (!r) {
> + perror("malloc");
> + exit(1);
> + }
> + memset(r->buffer, 0xbd, buf_size);
> +
> + errno = 0;
> + ret = fsinfo(AT_FDCWD, file, params, r->buffer, buf_size);
> + if (params->request >= FSINFO_ATTR__NR) {
> + if (ret == -1 && errno == EOPNOTSUPP)
> + exit(0);
> + fprintf(stderr, "Unexpected error for too-large command %u: %m\n",
> + params->request);
> + exit(1);
> + }
> + if (ret == -1)
> + break;
> +
> + if (ret <= buf_size)
> + break;
> + buf_size = (ret + 4096 - 1) & ~(4096 - 1);
> + }
> +
> + if (debug)
> + printf("fsinfo(%s,%s,%u,%u) = %d: %m\n",
> + file, fsinfo_attr_names[params->request],
> + params->Nth, params->Mth, ret);
> +
> + about = fsinfo_buffer_info[params->request];
> + if (ret == -1) {
> + if (errno == ENODATA) {
> + if (!(about.flags & (__FSINFO_N | __FSINFO_NM)) &&
> + params->Nth == 0 && params->Mth == 0) {
> + fprintf(stderr,
> + "Unexpected ENODATA (%u[%u][%u])\n",
> + params->request, params->Nth, params->Mth);
> + exit(1);
> + }
> + return (params->Mth == 0) ? 2 : 1;
> + }
> + if (errno == EOPNOTSUPP) {
> + if (params->Nth > 0 || params->Mth > 0) {
> + fprintf(stderr,
> + "Should return -ENODATA (%u[%u][%u])\n",
> + params->request, params->Nth, params->Mth);
> + exit(1);
> + }
> + //printf("\e[33m%s\e[m: <not supported>\n",
> + // fsinfo_attr_names[attr]);
> + return 2;
> + }
> + perror(file);
> + exit(1);
> + }
> +
> + if (raw) {
> + if (ret > 4096)
> + ret = 4096;
> + dump_hex((unsigned int *)r->buffer, 0, ret);
> + return 0;
> + }
> +
> + switch (about.flags & (__FSINFO_N | __FSINFO_NM)) {
> + case 0:
> + printf("\e[33m%s\e[m: ",
> + fsinfo_attr_names[params->request]);
> + break;
> + case __FSINFO_N:
> + printf("\e[33m%s[%u]\e[m: ",
> + fsinfo_attr_names[params->request],
> + params->Nth);
> + break;
> + case __FSINFO_NM:
> + printf("\e[33m%s[%u][%u]\e[m: ",
> + fsinfo_attr_names[params->request],
> + params->Nth, params->Mth);
> + break;
> + }
> +
> + switch (about.type) {
> + case __FSINFO_STRUCT:
> + dump_fsinfo(params->request, about, r, ret);
> + return 0;
> +
> + case __FSINFO_STRING:
> + if (ret >= 4096) {
> + ret = 4096;
> + r->buffer[4092] = '.';
> + r->buffer[4093] = '.';
> + r->buffer[4094] = '.';
> + r->buffer[4095] = 0;
> + } else {
> + r->buffer[ret] = 0;
> + }
> + for (p = r->buffer; *p; p++) {
> + if (!isprint(*p)) {
> + printf("<non-printable>\n");
> + continue;
> + }
> + }
> + printf("%s\n", r->buffer);
> + return 0;
> +
> + case __FSINFO_OVER:
> + return 0;
> +
> + case __FSINFO_STRUCT_ARRAY:
> + dump_fsinfo(params->request, about, r, ret);
> + return 0;
> +
> + default:
> + fprintf(stderr, "Fishy about %u %u,%u,%u\n",
> + params->request, about.type, about.flags, about.size);
> + exit(1);
> + }
> +}
> +
> +/*
> + *
> + */
> +int main(int argc, char **argv)
> +{
> + struct fsinfo_params params = {
> + .at_flags = AT_SYMLINK_NOFOLLOW,
> + };
> + unsigned int attr;
> + int raw = 0, opt, Nth, Mth;
> +
> + while ((opt = getopt(argc, argv, "adlr"))) {
> + switch (opt) {
> + case 'a':
> + params.at_flags |= AT_NO_AUTOMOUNT;
> + continue;
> + case 'd':
> + debug = true;
> + continue;
> + case 'l':
> + params.at_flags &= ~AT_SYMLINK_NOFOLLOW;
> + continue;
> + case 'r':
> + raw = 1;
> + continue;
> + }
> + break;
> + }
> +
> + argc -= optind;
> + argv += optind;
> +
> + if (argc != 1) {
> + printf("Format: test-fsinfo [-alr] <file>\n");
> + exit(2);
> + }
> +
> + for (attr = 0; attr <= FSINFO_ATTR__NR; attr++) {
> + Nth = 0;
> + do {
> + Mth = 0;
> + do {
> + params.request = attr;
> + params.Nth = Nth;
> + params.Mth = Mth;
> +
> + switch (try_one(argv[0], ¶ms, raw)) {
> + case 0:
> + continue;
> + case 1:
> + goto done_M;
> + case 2:
> + goto done_N;
> + }
> + } while (++Mth < 100);
> +
> + done_M:
> + if (Mth >= 100) {
> + fprintf(stderr, "Fishy: Mth == %u\n", Mth);
> + break;
> + }
> +
> + } while (++Nth < 100);
> +
> + done_N:
> + if (Nth >= 100) {
> + fprintf(stderr, "Fishy: Nth == %u\n", Nth);
> + break;
> + }
> + }
> +
> + return 0;
> +}
>
^ permalink raw reply
* Re: [PATCH V34 00/29] Lockdown as an LSM
From: John Johansen @ 2019-06-25 8:16 UTC (permalink / raw)
To: James Morris, Matthew Garrett
Cc: linux-security-module, linux-kernel, linux-api, Stephen Smalley,
Andy Lutomirski, Casey Schaufler
In-Reply-To: <alpine.LRH.2.21.1906250853290.7826@namei.org>
On 6/24/19 4:01 PM, James Morris wrote:
> On Fri, 21 Jun 2019, Matthew Garrett wrote:
>
>> Minor updates over V33 - security_is_locked_down renamed to
>> security_locked_down, return value of security_locked_down is returned
>> in most cases, one unnecessary patch was dropped, couple of minor nits
>> fixed.
>
> Thanks for the respin.
>
> We are still not resolved on granularity. Stephen has said he's not sure
> if a useful policy can be constructed with just confidentiality and
> integrity settings. I'd be interested to know JJ and Casey's thoughts on
> lockdown policy flexibility wrt their respective LSMs.
>
> These are also "all or nothing" choices which may prevent deployment due
> to a user needing to allow (presumably controlled or mitigated) exceptions
> to the policy.
>
>
I haven't gotten a chance to play with this the way I want to so there is
still a lot of questions regarding its interaction with apparmor and its
policy, but from what I have seen so far it is looking good.
I expect the all or nothing choices may limit its deployments (we really
need to play with this more to say) but we already face similar issues.
There are options we provide at a distro level that we can't turn on by
default, but we do recommend to more security conscious users.
If lockdown was in kernel we would certainly make it available for our
users, we have even had a few people ask about it.
^ permalink raw reply
* Re: [PATCH v5 16/16] f2fs: add fs-verity support
From: Chao Yu @ 2019-06-25 7:55 UTC (permalink / raw)
To: Eric Biggers, linux-fscrypt
Cc: Theodore Y . Ts'o, Darrick J . Wong, linux-api, Dave Chinner,
linux-f2fs-devel, linux-fsdevel, Jaegeuk Kim, linux-integrity,
linux-ext4, Linus Torvalds, Christoph Hellwig, Victor Hsieh
In-Reply-To: <20190620205043.64350-17-ebiggers@kernel.org>
Hi Eric,
On 2019/6/21 4:50, Eric Biggers wrote:
> +static int f2fs_begin_enable_verity(struct file *filp)
> +{
> + struct inode *inode = file_inode(filp);
> + int err;
> +
I think we'd better add condition here (under inode lock) to disallow enabling
verity on atomic/volatile inode, as we may fail to write merkle tree data due to
atomic/volatile inode's special writeback method.
> + err = f2fs_convert_inline_inode(inode);
> + if (err)
> + return err;
> +
> + err = dquot_initialize(inode);
> + if (err)
> + return err;
We can get rid of dquot_initialize() here, since f2fs_file_open() ->
dquot_file_open() should has initialized quota entry previously, right?
Thanks,
> +
> + set_inode_flag(inode, FI_VERITY_IN_PROGRESS);
> + return 0;
> +}
> +
^ permalink raw reply
* Re: [PATCH V34 00/29] Lockdown as an LSM
From: James Morris @ 2019-06-25 6:04 UTC (permalink / raw)
To: Matthew Garrett
Cc: LSM List, Linux Kernel Mailing List, Linux API, Stephen Smalley,
Andy Lutomirski, John Johansen, Casey Schaufler
In-Reply-To: <CACdnJut2erF9ZKeJQ+uvWZeEnHh=stmEioi_P36DF9mN5i2RGQ@mail.gmail.com>
On Mon, 24 Jun 2019, Matthew Garrett wrote:
> > We are still not resolved on granularity. Stephen has said he's not sure
> > if a useful policy can be constructed with just confidentiality and
> > integrity settings. I'd be interested to know JJ and Casey's thoughts on
> > lockdown policy flexibility wrt their respective LSMs.
>
> This implementation provides arbitrary granularity at the LSM level,
> though the lockdown LSM itself only provides two levels. Other LSMs
> can choose an appropriate level of exposure.
Ahh, OK, I only looked at the patchset description and had not looked at
V33 yet.
This is looking good.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH V31 07/25] kexec_file: Restrict at runtime if the kernel is locked down
From: Dave Young @ 2019-06-25 2:51 UTC (permalink / raw)
To: Matthew Garrett
Cc: James Morris, Jiri Bohac, Linux API, kexec,
Linux Kernel Mailing List, David Howells, LSM List,
Andy Lutomirski
In-Reply-To: <CACdnJusPtYLdg7ZPhBo=Y5EsBz6B+5M2zYscBrLcc89oNnPkdQ@mail.gmail.com>
On 06/24/19 at 02:06pm, Matthew Garrett wrote:
> On Sun, Jun 23, 2019 at 6:52 PM Dave Young <dyoung@redhat.com> wrote:
> >
> > On 06/21/19 at 01:18pm, Matthew Garrett wrote:
> > > I don't think so - we want it to be possible to load images if they
> > > have a valid signature.
> >
> > I know it works like this way because of the previous patch. But from
> > the patch log "When KEXEC_SIG is not enabled, kernel should not load
> > images", it is simple to check it early for !IS_ENABLED(CONFIG_KEXEC_SIG) &&
> > kernel_is_locked_down(reason, LOCKDOWN_INTEGRITY) instead of depending
> > on the late code to verify signature. In that way, easier to
> > understand the logic, no?
>
> But that combination doesn't enforce signature validation? We can't
> depend on !IS_ENABLED(CONFIG_KEXEC_SIG_FORCE) because then it'll
> enforce signature validation even if lockdown is disabled.
Ok, got your point. still something could be improved though, in the switch
chunk, the errno, reason and IS_ENABLED(CONFIG_KEXEC_SIG_FORCE) etc is
not necessary for this -EPERM case.
/* add some comment to describe the behavior */
if (ret && security_is_locked_down(LOCKDOWN_KEXEC)) {
ret = -EPERM;
goto out;
}
Thanks
Dave
^ permalink raw reply
* Re: [PATCH V34 08/29] kexec_file: split KEXEC_VERIFY_SIG into KEXEC_SIG and KEXEC_SIG_FORCE
From: Dave Young @ 2019-06-25 2:35 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
Jiri Bohac, David Howells, Matthew Garrett, kexec
In-Reply-To: <20190624020106.GD2976@dhcp-128-65.nay.redhat.com>
On 06/24/19 at 10:01am, Dave Young wrote:
> On 06/21/19 at 05:03pm, Matthew Garrett wrote:
> > From: Jiri Bohac <jbohac@suse.cz>
> >
> > This is a preparatory patch for kexec_file_load() lockdown. A locked down
> > kernel needs to prevent unsigned kernel images from being loaded with
> > kexec_file_load(). Currently, the only way to force the signature
> > verification is compiling with KEXEC_VERIFY_SIG. This prevents loading
> > usigned images even when the kernel is not locked down at runtime.
> >
> > This patch splits KEXEC_VERIFY_SIG into KEXEC_SIG and KEXEC_SIG_FORCE.
> > Analogous to the MODULE_SIG and MODULE_SIG_FORCE for modules, KEXEC_SIG
> > turns on the signature verification but allows unsigned images to be
> > loaded. KEXEC_SIG_FORCE disallows images without a valid signature.
> >
> > [Modified by David Howells such that:
> >
> > (1) verify_pefile_signature() differentiates between no-signature and
> > sig-didn't-match in its returned errors.
> >
> > (2) kexec fails with EKEYREJECTED if there is a signature for which we
> > have a key, but signature doesn't match - even if in non-forcing mode.
> >
> > (3) kexec fails with EBADMSG or some other error if there is a signature
> > which cannot be parsed - even if in non-forcing mode.
> >
> > (4) kexec fails with ELIBBAD if the PE file cannot be parsed to extract
> > the signature - even if in non-forcing mode.
> >
> > ]
>
> Seems I do not see EBADMSG and ELIBBAD in this patch, also kexec fails
> with proper errno instead of EKEYREJECTED only.
>
> I may missed something? Other than the patch log issue:
>
> Reviewed-by: Dave Young <dyoung@redhat.com>
Hold on :) Noticed another issue, please see comment inline..
>
> >
> > Signed-off-by: Jiri Bohac <jbohac@suse.cz>
> > Signed-off-by: David Howells <dhowells@redhat.com>
> > Signed-off-by: Matthew Garrett <mjg59@google.com>
> > Reviewed-by: Jiri Bohac <jbohac@suse.cz>
> > cc: kexec@lists.infradead.org
> > ---
> > arch/x86/Kconfig | 20 ++++++++---
> > crypto/asymmetric_keys/verify_pefile.c | 4 ++-
> > include/linux/kexec.h | 4 +--
> > kernel/kexec_file.c | 47 ++++++++++++++++++++++----
> > 4 files changed, 60 insertions(+), 15 deletions(-)
> >
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index c1f9b3cf437c..84381dd60760 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -2012,20 +2012,30 @@ config KEXEC_FILE
> > config ARCH_HAS_KEXEC_PURGATORY
> > def_bool KEXEC_FILE
> >
> > -config KEXEC_VERIFY_SIG
> > +config KEXEC_SIG
> > bool "Verify kernel signature during kexec_file_load() syscall"
> > depends on KEXEC_FILE
> > ---help---
> > - This option makes kernel signature verification mandatory for
> > - the kexec_file_load() syscall.
> >
> > - In addition to that option, you need to enable signature
> > + This option makes the kexec_file_load() syscall check for a valid
> > + signature of the kernel image. The image can still be loaded without
> > + a valid signature unless you also enable KEXEC_SIG_FORCE, though if
> > + there's a signature that we can check, then it must be valid.
> > +
> > + In addition to this option, you need to enable signature
> > verification for the corresponding kernel image type being
> > loaded in order for this to work.
> >
> > +config KEXEC_SIG_FORCE
> > + bool "Require a valid signature in kexec_file_load() syscall"
> > + depends on KEXEC_SIG
> > + ---help---
> > + This option makes kernel signature verification mandatory for
> > + the kexec_file_load() syscall.
> > +
> > config KEXEC_BZIMAGE_VERIFY_SIG
> > bool "Enable bzImage signature verification support"
> > - depends on KEXEC_VERIFY_SIG
> > + depends on KEXEC_SIG
> > depends on SIGNED_PE_FILE_VERIFICATION
> > select SYSTEM_TRUSTED_KEYRING
> > ---help---
> > diff --git a/crypto/asymmetric_keys/verify_pefile.c b/crypto/asymmetric_keys/verify_pefile.c
> > index d178650fd524..4473cea1e877 100644
> > --- a/crypto/asymmetric_keys/verify_pefile.c
> > +++ b/crypto/asymmetric_keys/verify_pefile.c
> > @@ -100,7 +100,7 @@ static int pefile_parse_binary(const void *pebuf, unsigned int pelen,
> >
> > if (!ddir->certs.virtual_address || !ddir->certs.size) {
> > pr_debug("Unsigned PE binary\n");
> > - return -EKEYREJECTED;
> > + return -ENODATA;
> > }
> >
> > chkaddr(ctx->header_size, ddir->certs.virtual_address,
> > @@ -408,6 +408,8 @@ static int pefile_digest_pe(const void *pebuf, unsigned int pelen,
> > * (*) 0 if at least one signature chain intersects with the keys in the trust
> > * keyring, or:
> > *
> > + * (*) -ENODATA if there is no signature present.
> > + *
> > * (*) -ENOPKG if a suitable crypto module couldn't be found for a check on a
> > * chain.
> > *
> > diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> > index b9b1bc5f9669..58b27c7bdc2b 100644
> > --- a/include/linux/kexec.h
> > +++ b/include/linux/kexec.h
> > @@ -125,7 +125,7 @@ typedef void *(kexec_load_t)(struct kimage *image, char *kernel_buf,
> > unsigned long cmdline_len);
> > typedef int (kexec_cleanup_t)(void *loader_data);
> >
> > -#ifdef CONFIG_KEXEC_VERIFY_SIG
> > +#ifdef CONFIG_KEXEC_SIG
> > typedef int (kexec_verify_sig_t)(const char *kernel_buf,
> > unsigned long kernel_len);
> > #endif
> > @@ -134,7 +134,7 @@ struct kexec_file_ops {
> > kexec_probe_t *probe;
> > kexec_load_t *load;
> > kexec_cleanup_t *cleanup;
> > -#ifdef CONFIG_KEXEC_VERIFY_SIG
> > +#ifdef CONFIG_KEXEC_SIG
> > kexec_verify_sig_t *verify_sig;
> > #endif
> > };
> > diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> > index f1d0e00a3971..eec7e5bb2a08 100644
> > --- a/kernel/kexec_file.c
> > +++ b/kernel/kexec_file.c
> > @@ -90,7 +90,7 @@ int __weak arch_kimage_file_post_load_cleanup(struct kimage *image)
> > return kexec_image_post_load_cleanup_default(image);
> > }
> >
> > -#ifdef CONFIG_KEXEC_VERIFY_SIG
> > +#ifdef CONFIG_KEXEC_SIG
> > static int kexec_image_verify_sig_default(struct kimage *image, void *buf,
> > unsigned long buf_len)
> > {
> > @@ -188,7 +188,8 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
> > const char __user *cmdline_ptr,
> > unsigned long cmdline_len, unsigned flags)
> > {
> > - int ret = 0;
> > + const char *reason;
> > + int ret;
> > void *ldata;
> > loff_t size;
> >
> > @@ -207,15 +208,47 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
> > if (ret)
> > goto out;
> >
> > -#ifdef CONFIG_KEXEC_VERIFY_SIG
> > +#ifdef CONFIG_KEXEC_SIG
> > ret = arch_kexec_kernel_verify_sig(image, image->kernel_buf,
> > image->kernel_buf_len);
> > - if (ret) {
> > - pr_debug("kernel signature verification failed.\n");
> > +#else
> > + ret = -ENODATA;
Use -ENODATA for above case looks not correct, please just remove the #else and
move the #endif to the end of the switch chunk.
> > +#endif
> > +
> > + switch (ret) {
> > + case 0:
> > + break;
> > +
> > + /* Certain verification errors are non-fatal if we're not
> > + * checking errors, provided we aren't mandating that there
> > + * must be a valid signature.
> > + */
> > + case -ENODATA:
> > + reason = "kexec of unsigned image";
> > + goto decide;
> > + case -ENOPKG:
> > + reason = "kexec of image with unsupported crypto";
> > + goto decide;
> > + case -ENOKEY:
> > + reason = "kexec of image with unavailable key";
> > + decide:
> > + if (IS_ENABLED(CONFIG_KEXEC_SIG_FORCE)) {
> > + pr_notice("%s rejected\n", reason);
> > + goto out;
> > + }
> > +
> > + ret = 0;
> > + break;
> > +
> > + /* All other errors are fatal, including nomem, unparseable
> > + * signatures and signature check failures - even if signatures
> > + * aren't required.
> > + */
> > + default:
> > + pr_notice("kernel signature verification failed (%d).\n", ret);
> > goto out;
> > }
> > - pr_debug("kernel signature verification successful.\n");
> > -#endif
> > +
> > /* It is possible that there no initramfs is being loaded */
> > if (!(flags & KEXEC_FILE_NO_INITRAMFS)) {
> > ret = kernel_read_file_from_fd(initrd_fd, &image->initrd_buf,
> > --
> > 2.22.0.410.gd8fdbe21b5-goog
> >
Thanks
Dave
^ permalink raw reply
* Re: [PATCH V31 07/25] kexec_file: Restrict at runtime if the kernel is locked down
From: Mimi Zohar @ 2019-06-25 1:46 UTC (permalink / raw)
To: Matthew Garrett
Cc: Dave Young, James Morris, Jiri Bohac, Linux API, kexec,
Linux Kernel Mailing List, David Howells, LSM List,
Andy Lutomirski
In-Reply-To: <CACdnJuvE-MbD42AJTrio=0RaN8SaWo-RHHt21z=3an1vtjTFhA@mail.gmail.com>
On Mon, 2019-06-24 at 17:02 -0700, Matthew Garrett wrote:
> On Mon, Jun 24, 2019 at 2:27 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> > I agree with Dave. There should be a stub lockdown function to
> > prevent enforcing lockdown when it isn't enabled.
>
> Sorry, when what isn't enabled? If no LSMs are enforcing lockdown then
> the check will return 0. The goal here is for distributions to be able
> to ship a kernel that has CONFIG_KEXEC_SIG=y, CONFIG_KEXEC_SIG_FORCE=n
> and at runtime be able to enforce a policy that requires signatures on
> kexec payloads.
Never mind, the call can't be moved earlier.
^ permalink raw reply
* Re: [PATCH V31 07/25] kexec_file: Restrict at runtime if the kernel is locked down
From: Matthew Garrett @ 2019-06-25 0:02 UTC (permalink / raw)
To: Mimi Zohar
Cc: Dave Young, James Morris, Jiri Bohac, Linux API, kexec,
Linux Kernel Mailing List, David Howells, LSM List,
Andy Lutomirski
In-Reply-To: <1561411657.4340.70.camel@linux.ibm.com>
On Mon, Jun 24, 2019 at 2:27 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> I agree with Dave. There should be a stub lockdown function to
> prevent enforcing lockdown when it isn't enabled.
Sorry, when what isn't enabled? If no LSMs are enforcing lockdown then
the check will return 0. The goal here is for distributions to be able
to ship a kernel that has CONFIG_KEXEC_SIG=y, CONFIG_KEXEC_SIG_FORCE=n
and at runtime be able to enforce a policy that requires signatures on
kexec payloads.
^ permalink raw reply
* Re: [PATCH V34 00/29] Lockdown as an LSM
From: Matthew Garrett @ 2019-06-24 23:56 UTC (permalink / raw)
To: James Morris
Cc: LSM List, Linux Kernel Mailing List, Linux API, Stephen Smalley,
Andy Lutomirski, John Johansen, Casey Schaufler
In-Reply-To: <alpine.LRH.2.21.1906250853290.7826@namei.org>
On Mon, Jun 24, 2019 at 4:01 PM James Morris <jmorris@namei.org> wrote:
>
> On Fri, 21 Jun 2019, Matthew Garrett wrote:
>
> > Minor updates over V33 - security_is_locked_down renamed to
> > security_locked_down, return value of security_locked_down is returned
> > in most cases, one unnecessary patch was dropped, couple of minor nits
> > fixed.
>
> Thanks for the respin.
>
> We are still not resolved on granularity. Stephen has said he's not sure
> if a useful policy can be constructed with just confidentiality and
> integrity settings. I'd be interested to know JJ and Casey's thoughts on
> lockdown policy flexibility wrt their respective LSMs.
This implementation provides arbitrary granularity at the LSM level,
though the lockdown LSM itself only provides two levels. Other LSMs
can choose an appropriate level of exposure.
> These are also "all or nothing" choices which may prevent deployment due
> to a user needing to allow (presumably controlled or mitigated) exceptions
> to the policy.
Distributions have been deploying the "all or nothing" solution for
several years now, which implies that it's adequate for the common
case. I think it's reasonable to punt finer grained policies over to
other LSMs - people who want that are probably already using custom
LSM policy.
^ permalink raw reply
* Re: [PATCH V34 00/29] Lockdown as an LSM
From: Casey Schaufler @ 2019-06-24 23:47 UTC (permalink / raw)
To: James Morris, Matthew Garrett
Cc: linux-security-module, linux-kernel, linux-api, Stephen Smalley,
Andy Lutomirski, John Johansen, casey
In-Reply-To: <alpine.LRH.2.21.1906250853290.7826@namei.org>
On 6/24/2019 4:01 PM, James Morris wrote:
> On Fri, 21 Jun 2019, Matthew Garrett wrote:
>
>> Minor updates over V33 - security_is_locked_down renamed to
>> security_locked_down, return value of security_locked_down is returned
>> in most cases, one unnecessary patch was dropped, couple of minor nits
>> fixed.
> Thanks for the respin.
>
> We are still not resolved on granularity. Stephen has said he's not sure
> if a useful policy can be constructed with just confidentiality and
> integrity settings. I'd be interested to know JJ and Casey's thoughts on
> lockdown policy flexibility wrt their respective LSMs.
Smack is a mandatory access control mechanism on named
objects controlled by the system. Issues of administrative
control, like whether hibernation is allowed, are outside
the scope of what Smack controls. There may be some subject/object
implications, but I have not identified any yet.
> These are also "all or nothing" choices which may prevent deployment due
> to a user needing to allow (presumably controlled or mitigated) exceptions
> to the policy.
^ permalink raw reply
* Re: [PATCH V34 00/29] Lockdown as an LSM
From: James Morris @ 2019-06-24 23:01 UTC (permalink / raw)
To: Matthew Garrett
Cc: linux-security-module, linux-kernel, linux-api, Stephen Smalley,
Andy Lutomirski, John Johansen, Casey Schaufler
In-Reply-To: <20190622000358.19895-1-matthewgarrett@google.com>
On Fri, 21 Jun 2019, Matthew Garrett wrote:
> Minor updates over V33 - security_is_locked_down renamed to
> security_locked_down, return value of security_locked_down is returned
> in most cases, one unnecessary patch was dropped, couple of minor nits
> fixed.
Thanks for the respin.
We are still not resolved on granularity. Stephen has said he's not sure
if a useful policy can be constructed with just confidentiality and
integrity settings. I'd be interested to know JJ and Casey's thoughts on
lockdown policy flexibility wrt their respective LSMs.
These are also "all or nothing" choices which may prevent deployment due
to a user needing to allow (presumably controlled or mitigated) exceptions
to the policy.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH V34 23/29] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Matthew Garrett @ 2019-06-24 21:30 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Andy Lutomirski, James Morris, LSM List,
Linux Kernel Mailing List, Linux API, David Howells,
Alexei Starovoitov, Network Development, Chun-Yi Lee, Jann Horn,
bpf
In-Reply-To: <7f36edf7-3120-975e-b643-3c0fa470bafd@iogearbox.net>
On Mon, Jun 24, 2019 at 2:22 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
> Agree, for example, bpf_probe_write_user() can never write into
> kernel memory (only user one). Just thinking out loud, wouldn't it
> be cleaner and more generic to perform this check at the actual function
> which performs the kernel memory without faulting? All three of these
> are in mm/maccess.c, and the very few occasions that override the
> probe_kernel_read symbol are calling eventually into __probe_kernel_read(),
> so this would catch all of them wrt lockdown restrictions. Otherwise
> you'd need to keep tracking every bit of new code being merged that
> calls into one of these, no? That way you only need to do it once like
> below and are guaranteed that the check catches these in future as well.
Not all paths into probe_kernel_read/write are from entry points that
need to be locked down (eg, as far as I can tell ftrace can't leak
anything interesting here).
^ permalink raw reply
* Re: [PATCH V31 07/25] kexec_file: Restrict at runtime if the kernel is locked down
From: Mimi Zohar @ 2019-06-24 21:27 UTC (permalink / raw)
To: Matthew Garrett, Dave Young
Cc: James Morris, Jiri Bohac, Linux API, kexec,
Linux Kernel Mailing List, David Howells, LSM List,
Andy Lutomirski
In-Reply-To: <CACdnJusPtYLdg7ZPhBo=Y5EsBz6B+5M2zYscBrLcc89oNnPkdQ@mail.gmail.com>
Hi Matthew,
On Mon, 2019-06-24 at 14:06 -0700, Matthew Garrett wrote:
> On Sun, Jun 23, 2019 at 6:52 PM Dave Young <dyoung@redhat.com> wrote:
> >
> > On 06/21/19 at 01:18pm, Matthew Garrett wrote:
> > > I don't think so - we want it to be possible to load images if they
> > > have a valid signature.
> >
> > I know it works like this way because of the previous patch. But from
> > the patch log "When KEXEC_SIG is not enabled, kernel should not load
> > images", it is simple to check it early for !IS_ENABLED(CONFIG_KEXEC_SIG) &&
> > kernel_is_locked_down(reason, LOCKDOWN_INTEGRITY) instead of depending
> > on the late code to verify signature. In that way, easier to
> > understand the logic, no?
>
> But that combination doesn't enforce signature validation? We can't
> depend on !IS_ENABLED(CONFIG_KEXEC_SIG_FORCE) because then it'll
> enforce signature validation even if lockdown is disabled.
I agree with Dave. There should be a stub lockdown function to
prevent enforcing lockdown when it isn't enabled.
Mimi
^ permalink raw reply
* Re: [PATCH V31 07/25] kexec_file: Restrict at runtime if the kernel is locked down
From: Matthew Garrett @ 2019-06-24 21:06 UTC (permalink / raw)
To: Dave Young
Cc: James Morris, Jiri Bohac, Linux API, kexec,
Linux Kernel Mailing List, David Howells, LSM List,
Andy Lutomirski
In-Reply-To: <20190624015206.GB2976@dhcp-128-65.nay.redhat.com>
On Sun, Jun 23, 2019 at 6:52 PM Dave Young <dyoung@redhat.com> wrote:
>
> On 06/21/19 at 01:18pm, Matthew Garrett wrote:
> > I don't think so - we want it to be possible to load images if they
> > have a valid signature.
>
> I know it works like this way because of the previous patch. But from
> the patch log "When KEXEC_SIG is not enabled, kernel should not load
> images", it is simple to check it early for !IS_ENABLED(CONFIG_KEXEC_SIG) &&
> kernel_is_locked_down(reason, LOCKDOWN_INTEGRITY) instead of depending
> on the late code to verify signature. In that way, easier to
> understand the logic, no?
But that combination doesn't enforce signature validation? We can't
depend on !IS_ENABLED(CONFIG_KEXEC_SIG_FORCE) because then it'll
enforce signature validation even if lockdown is disabled.
^ permalink raw reply
* Re: [PATCH V34 23/29] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Daniel Borkmann @ 2019-06-24 20:59 UTC (permalink / raw)
To: Andy Lutomirski, Matthew Garrett
Cc: James Morris, LSM List, Linux Kernel Mailing List, Linux API,
David Howells, Alexei Starovoitov, Network Development,
Chun-Yi Lee, Jann Horn, bpf
In-Reply-To: <CALCETrWmZX3R1L88Gz9vLY68gcK8zSXL4cA4GqAzQoyqSR7rRQ@mail.gmail.com>
On 06/24/2019 10:08 PM, Andy Lutomirski wrote:
> On Mon, Jun 24, 2019 at 12:54 PM Matthew Garrett <mjg59@google.com> wrote:
>> On Mon, Jun 24, 2019 at 8:37 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>>> On 06/22/2019 02:03 AM, Matthew Garrett wrote:
>>>> From: David Howells <dhowells@redhat.com>
>>>>
>>>> There are some bpf functions can be used to read kernel memory:
>>>
>>> Nit: that
>>
>> Fixed.
>>
>>>> bpf_probe_read, bpf_probe_write_user and bpf_trace_printk. These allow
>>>
>>> Please explain how bpf_probe_write_user reads kernel memory ... ?!
>>
>> Ha.
>>
>>>> private keys in kernel memory (e.g. the hibernation image signing key) to
>>>> be read by an eBPF program and kernel memory to be altered without
>>>
>>> ... and while we're at it, also how they allow "kernel memory to be
>>> altered without restriction". I've been pointing this false statement
>>> out long ago.
>>
>> Yup. How's the following description:
>>
>> bpf: Restrict bpf when kernel lockdown is in confidentiality mode
>>
>> There are some bpf functions that can be used to read kernel memory and
>> exfiltrate it to userland: bpf_probe_read, bpf_probe_write_user and
>> bpf_trace_printk. These could be abused to (eg) allow private
>> keys in kernel
>> memory to be leaked. Disable them if the kernel has been locked
>> down in confidentiality
>> mode.
>
> I'm confused. I understand why we're restricting bpf_probe_read().
> Why are we restricting bpf_probe_write_user() and bpf_trace_printk(),
> though?
Agree, for example, bpf_probe_write_user() can never write into
kernel memory (only user one). Just thinking out loud, wouldn't it
be cleaner and more generic to perform this check at the actual function
which performs the kernel memory without faulting? All three of these
are in mm/maccess.c, and the very few occasions that override the
probe_kernel_read symbol are calling eventually into __probe_kernel_read(),
so this would catch all of them wrt lockdown restrictions. Otherwise
you'd need to keep tracking every bit of new code being merged that
calls into one of these, no? That way you only need to do it once like
below and are guaranteed that the check catches these in future as well.
Thanks,
Daniel
diff --git a/mm/maccess.c b/mm/maccess.c
index 482d4d6..2c8220f 100644
--- a/mm/maccess.c
+++ b/mm/maccess.c
@@ -29,6 +29,9 @@ long __probe_kernel_read(void *dst, const void *src, size_t size)
long ret;
mm_segment_t old_fs = get_fs();
+ if (security_locked_down(LOCKDOWN_KERNEL_READ))
+ return -EFAULT;
+
set_fs(KERNEL_DS);
pagefault_disable();
ret = __copy_from_user_inatomic(dst,
@@ -57,6 +60,9 @@ long __probe_kernel_write(void *dst, const void *src, size_t size)
long ret;
mm_segment_t old_fs = get_fs();
+ if (security_locked_down(LOCKDOWN_KERNEL_WRITE))
+ return -EFAULT;
+
set_fs(KERNEL_DS);
pagefault_disable();
ret = __copy_to_user_inatomic((__force void __user *)dst, src, size);
@@ -90,6 +96,9 @@ long strncpy_from_unsafe(char *dst, const void *unsafe_addr, long count)
const void *src = unsafe_addr;
long ret;
+ if (security_locked_down(LOCKDOWN_KERNEL_READ))
+ return -EFAULT;
+
if (unlikely(count <= 0))
return 0;
^ permalink raw reply related
* Re: [PATCH V34 23/29] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Matthew Garrett @ 2019-06-24 20:15 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Daniel Borkmann, James Morris, LSM List,
Linux Kernel Mailing List, Linux API, David Howells,
Alexei Starovoitov, Network Development, Chun-Yi Lee, Jann Horn,
bpf
In-Reply-To: <CALCETrWmZX3R1L88Gz9vLY68gcK8zSXL4cA4GqAzQoyqSR7rRQ@mail.gmail.com>
On Mon, Jun 24, 2019 at 1:09 PM Andy Lutomirski <luto@kernel.org> wrote:
> I'm confused. I understand why we're restricting bpf_probe_read().
> Why are we restricting bpf_probe_write_user() and bpf_trace_printk(),
> though?
Hmm. I think the thinking here was around exfiltration mechanisms, but
if the read is blocked then that seems less likely. This seems to
trace back to http://kernsec.org/pipermail/linux-security-module-archive/2017-October/003545.html
- Joey, do you know the reasoning here?
^ permalink raw reply
* Re: [PATCH V34 23/29] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Andy Lutomirski @ 2019-06-24 20:08 UTC (permalink / raw)
To: Matthew Garrett
Cc: Daniel Borkmann, James Morris, LSM List,
Linux Kernel Mailing List, Linux API, David Howells,
Alexei Starovoitov, Network Development, Chun-Yi Lee, Jann Horn,
bpf
In-Reply-To: <CACdnJuvR2bn3y3fYzg06GWXXgAGjgED2Dfa5g0oAwJ28qCCqBg@mail.gmail.com>
On Mon, Jun 24, 2019 at 12:54 PM Matthew Garrett <mjg59@google.com> wrote:
>
> On Mon, Jun 24, 2019 at 8:37 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
> >
> > On 06/22/2019 02:03 AM, Matthew Garrett wrote:
> > > From: David Howells <dhowells@redhat.com>
> > >
> > > There are some bpf functions can be used to read kernel memory:
> >
> > Nit: that
>
> Fixed.
>
> > > bpf_probe_read, bpf_probe_write_user and bpf_trace_printk. These allow
> >
> > Please explain how bpf_probe_write_user reads kernel memory ... ?!
>
> Ha.
>
> > > private keys in kernel memory (e.g. the hibernation image signing key) to
> > > be read by an eBPF program and kernel memory to be altered without
> >
> > ... and while we're at it, also how they allow "kernel memory to be
> > altered without restriction". I've been pointing this false statement
> > out long ago.
>
> Yup. How's the following description:
>
> bpf: Restrict bpf when kernel lockdown is in confidentiality mode
>
> There are some bpf functions that can be used to read kernel memory and
> exfiltrate it to userland: bpf_probe_read, bpf_probe_write_user and
> bpf_trace_printk. These could be abused to (eg) allow private
> keys in kernel
> memory to be leaked. Disable them if the kernel has been locked
> down in confidentiality
> mode.
I'm confused. I understand why we're restricting bpf_probe_read().
Why are we restricting bpf_probe_write_user() and bpf_trace_printk(),
though?
--Andy
^ permalink raw reply
* Re: [PATCH V34 23/29] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Matthew Garrett @ 2019-06-24 19:54 UTC (permalink / raw)
To: Daniel Borkmann
Cc: James Morris, LSM List, Linux Kernel Mailing List, Linux API,
David Howells, Alexei Starovoitov, Network Development,
Chun-Yi Lee, Jann Horn, bpf
In-Reply-To: <739e21b5-9559-d588-3542-bf0bc81de1b2@iogearbox.net>
On Mon, Jun 24, 2019 at 8:37 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 06/22/2019 02:03 AM, Matthew Garrett wrote:
> > From: David Howells <dhowells@redhat.com>
> >
> > There are some bpf functions can be used to read kernel memory:
>
> Nit: that
Fixed.
> > bpf_probe_read, bpf_probe_write_user and bpf_trace_printk. These allow
>
> Please explain how bpf_probe_write_user reads kernel memory ... ?!
Ha.
> > private keys in kernel memory (e.g. the hibernation image signing key) to
> > be read by an eBPF program and kernel memory to be altered without
>
> ... and while we're at it, also how they allow "kernel memory to be
> altered without restriction". I've been pointing this false statement
> out long ago.
Yup. How's the following description:
bpf: Restrict bpf when kernel lockdown is in confidentiality mode
There are some bpf functions that can be used to read kernel memory and
exfiltrate it to userland: bpf_probe_read, bpf_probe_write_user and
bpf_trace_printk. These could be abused to (eg) allow private
keys in kernel
memory to be leaked. Disable them if the kernel has been locked
down in confidentiality
mode.
> This whole thing is still buggy as has been pointed out before by
> Jann. For helpers like above and few others below, error conditions
> must clear the buffer ...
Sorry, yes. My fault.
^ permalink raw reply
* Re: [RFC PATCH 1/1] Revert "rseq/selftests: arm: use udf instruction for RSEQ_SIG"
From: Mathieu Desnoyers @ 2019-06-24 18:20 UTC (permalink / raw)
To: Will Deacon
Cc: shuah, linux-kernel, Peter Zijlstra, Thomas Gleixner,
Joel Fernandes, Catalin Marinas, Dave Watson, Andi Kleen,
linux-kselftest, H. Peter Anvin, Chris Lameter, Russell King,
Michael Kerrisk, Paul E . McKenney, Paul Turner, Boqun Feng,
Josh Triplett, rostedt, Ben Maurer, linux-api, Andy
In-Reply-To: <20190624172429.GA11133@fuggles.cambridge.arm.com>
----- On Jun 24, 2019, at 1:24 PM, Will Deacon will.deacon@arm.com wrote:
> On Mon, Jun 17, 2019 at 05:23:04PM +0200, Mathieu Desnoyers wrote:
>> This reverts commit 2b845d4b4acd9422bbb668989db8dc36dfc8f438.
>>
>> That commit introduces build issues for programs compiled in Thumb mode.
>> Rather than try to be clever and emit a valid trap instruction on arm32,
>> which requires special care about big/little endian handling on that
>> architecture, just emit plain data. Data in the instruction stream is
>> technically expected on arm32: this is how literal pools are
>> implemented. Reverting to the prior behavior does exactly that.
>>
>> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>> CC: Peter Zijlstra <peterz@infradead.org>
>> CC: Thomas Gleixner <tglx@linutronix.de>
>> CC: Joel Fernandes <joelaf@google.com>
>> CC: Catalin Marinas <catalin.marinas@arm.com>
>> CC: Dave Watson <davejwatson@fb.com>
>> CC: Will Deacon <will.deacon@arm.com>
>> CC: Shuah Khan <shuah@kernel.org>
>> CC: Andi Kleen <andi@firstfloor.org>
>> CC: linux-kselftest@vger.kernel.org
>> CC: "H . Peter Anvin" <hpa@zytor.com>
>> CC: Chris Lameter <cl@linux.com>
>> CC: Russell King <linux@arm.linux.org.uk>
>> CC: Michael Kerrisk <mtk.manpages@gmail.com>
>> CC: "Paul E . McKenney" <paulmck@linux.vnet.ibm.com>
>> CC: Paul Turner <pjt@google.com>
>> CC: Boqun Feng <boqun.feng@gmail.com>
>> CC: Josh Triplett <josh@joshtriplett.org>
>> CC: Steven Rostedt <rostedt@goodmis.org>
>> CC: Ben Maurer <bmaurer@fb.com>
>> CC: linux-api@vger.kernel.org
>> CC: Andy Lutomirski <luto@amacapital.net>
>> CC: Andrew Morton <akpm@linux-foundation.org>
>> CC: Linus Torvalds <torvalds@linux-foundation.org>
>> CC: Carlos O'Donell <carlos@redhat.com>
>> CC: Florian Weimer <fweimer@redhat.com>
>> ---
>> tools/testing/selftests/rseq/rseq-arm.h | 52 ++-------------------------------
>> 1 file changed, 2 insertions(+), 50 deletions(-)
>>
>> diff --git a/tools/testing/selftests/rseq/rseq-arm.h
>> b/tools/testing/selftests/rseq/rseq-arm.h
>> index 84f28f147fb6..5f262c54364f 100644
>> --- a/tools/testing/selftests/rseq/rseq-arm.h
>> +++ b/tools/testing/selftests/rseq/rseq-arm.h
>> @@ -5,54 +5,7 @@
>> * (C) Copyright 2016-2018 - Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>> */
>>
>> -/*
>> - * RSEQ_SIG uses the udf A32 instruction with an uncommon immediate operand
>> - * value 0x5de3. This traps if user-space reaches this instruction by mistake,
>> - * and the uncommon operand ensures the kernel does not move the instruction
>> - * pointer to attacker-controlled code on rseq abort.
>> - *
>> - * The instruction pattern in the A32 instruction set is:
>> - *
>> - * e7f5def3 udf #24035 ; 0x5de3
>> - *
>> - * This translates to the following instruction pattern in the T16 instruction
>> - * set:
>> - *
>> - * little endian:
>> - * def3 udf #243 ; 0xf3
>> - * e7f5 b.n <7f5>
>> - *
>> - * pre-ARMv6 big endian code:
>> - * e7f5 b.n <7f5>
>> - * def3 udf #243 ; 0xf3
>> - *
>> - * ARMv6+ -mbig-endian generates mixed endianness code vs data: little-endian
>> - * code and big-endian data. Ensure the RSEQ_SIG data signature matches code
>> - * endianness. Prior to ARMv6, -mbig-endian generates big-endian code and data
>> - * (which match), so there is no need to reverse the endianness of the data
>> - * representation of the signature. However, the choice between BE32 and BE8
>> - * is done by the linker, so we cannot know whether code and data endianness
>> - * will be mixed before the linker is invoked.
>> - */
>> -
>> -#define RSEQ_SIG_CODE 0xe7f5def3
>> -
>> -#ifndef __ASSEMBLER__
>> -
>> -#define RSEQ_SIG_DATA \
>> - ({ \
>> - int sig; \
>> - asm volatile ("b 2f\n\t" \
>> - "1: .inst " __rseq_str(RSEQ_SIG_CODE) "\n\t" \
>> - "2:\n\t" \
>> - "ldr %[sig], 1b\n\t" \
>> - : [sig] "=r" (sig)); \
>> - sig; \
>> - })
>> -
>> -#define RSEQ_SIG RSEQ_SIG_DATA
>> -
>> -#endif
>> +#define RSEQ_SIG 0x53053053
>
> I don't get why you're reverting back to this old signature value, when the
> one we came up with will work well when interpreted as an instruction in the
> *vast* majority of scenarios that people care about (A32/T32 little-endian).
> I think you might be under-estimating just how dead things like BE32 really
> are.
My issue is that the current .instr approach is broken for programs or functions
built in Thumb mode, and I received no feedback on the solutions I proposed for
those issues, which led me to propose a patch reverting to a simple .word.
>
> That said, when you ran into .inst.n/.inst.w issues, did you try something
> along the lines of the WASM() macro we use in arch/arm/, which adds the ".w"
> suffix when targetting Thumb?
AFAIU, the WASM macros depend on CONFIG_THUMB2_KERNEL, which may be fine within
the kernel, but for user-space things are a bit more complex.
A compile-unit can be compiled as thumb, which will set a compiler define
which we could use to detect thumb mode. However, unfortunately, a single
function can also be compiled with an attribute selecting thumb mode, which
AFAIU does not influence the preprocessor defines.
If we had the equivalent of pushsection and popsection for "arm" mode vs "thumb"
mode in asm, it would solve all our issues, but the only way I found to do
something equivalent was to use the size of a no-op in a asm conditional
to restore the previous state.
Any other ideas on how to handle this ?
Thanks,
Mathieu
>
> Will
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [RFC PATCH 1/1] Revert "rseq/selftests: arm: use udf instruction for RSEQ_SIG"
From: Will Deacon @ 2019-06-24 17:24 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Shuah Khan, linux-kernel, Peter Zijlstra, Thomas Gleixner,
Joel Fernandes, Catalin Marinas, Dave Watson, Andi Kleen,
linux-kselftest, H . Peter Anvin, Chris Lameter, Russell King,
Michael Kerrisk, Paul E . McKenney, Paul Turner, Boqun Feng,
Josh Triplett, Steven Rostedt, Ben Maurer, linux-api,
Andy Lutomirski
In-Reply-To: <20190617152304.23371-1-mathieu.desnoyers@efficios.com>
On Mon, Jun 17, 2019 at 05:23:04PM +0200, Mathieu Desnoyers wrote:
> This reverts commit 2b845d4b4acd9422bbb668989db8dc36dfc8f438.
>
> That commit introduces build issues for programs compiled in Thumb mode.
> Rather than try to be clever and emit a valid trap instruction on arm32,
> which requires special care about big/little endian handling on that
> architecture, just emit plain data. Data in the instruction stream is
> technically expected on arm32: this is how literal pools are
> implemented. Reverting to the prior behavior does exactly that.
>
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> CC: Peter Zijlstra <peterz@infradead.org>
> CC: Thomas Gleixner <tglx@linutronix.de>
> CC: Joel Fernandes <joelaf@google.com>
> CC: Catalin Marinas <catalin.marinas@arm.com>
> CC: Dave Watson <davejwatson@fb.com>
> CC: Will Deacon <will.deacon@arm.com>
> CC: Shuah Khan <shuah@kernel.org>
> CC: Andi Kleen <andi@firstfloor.org>
> CC: linux-kselftest@vger.kernel.org
> CC: "H . Peter Anvin" <hpa@zytor.com>
> CC: Chris Lameter <cl@linux.com>
> CC: Russell King <linux@arm.linux.org.uk>
> CC: Michael Kerrisk <mtk.manpages@gmail.com>
> CC: "Paul E . McKenney" <paulmck@linux.vnet.ibm.com>
> CC: Paul Turner <pjt@google.com>
> CC: Boqun Feng <boqun.feng@gmail.com>
> CC: Josh Triplett <josh@joshtriplett.org>
> CC: Steven Rostedt <rostedt@goodmis.org>
> CC: Ben Maurer <bmaurer@fb.com>
> CC: linux-api@vger.kernel.org
> CC: Andy Lutomirski <luto@amacapital.net>
> CC: Andrew Morton <akpm@linux-foundation.org>
> CC: Linus Torvalds <torvalds@linux-foundation.org>
> CC: Carlos O'Donell <carlos@redhat.com>
> CC: Florian Weimer <fweimer@redhat.com>
> ---
> tools/testing/selftests/rseq/rseq-arm.h | 52 ++-------------------------------
> 1 file changed, 2 insertions(+), 50 deletions(-)
>
> diff --git a/tools/testing/selftests/rseq/rseq-arm.h b/tools/testing/selftests/rseq/rseq-arm.h
> index 84f28f147fb6..5f262c54364f 100644
> --- a/tools/testing/selftests/rseq/rseq-arm.h
> +++ b/tools/testing/selftests/rseq/rseq-arm.h
> @@ -5,54 +5,7 @@
> * (C) Copyright 2016-2018 - Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> */
>
> -/*
> - * RSEQ_SIG uses the udf A32 instruction with an uncommon immediate operand
> - * value 0x5de3. This traps if user-space reaches this instruction by mistake,
> - * and the uncommon operand ensures the kernel does not move the instruction
> - * pointer to attacker-controlled code on rseq abort.
> - *
> - * The instruction pattern in the A32 instruction set is:
> - *
> - * e7f5def3 udf #24035 ; 0x5de3
> - *
> - * This translates to the following instruction pattern in the T16 instruction
> - * set:
> - *
> - * little endian:
> - * def3 udf #243 ; 0xf3
> - * e7f5 b.n <7f5>
> - *
> - * pre-ARMv6 big endian code:
> - * e7f5 b.n <7f5>
> - * def3 udf #243 ; 0xf3
> - *
> - * ARMv6+ -mbig-endian generates mixed endianness code vs data: little-endian
> - * code and big-endian data. Ensure the RSEQ_SIG data signature matches code
> - * endianness. Prior to ARMv6, -mbig-endian generates big-endian code and data
> - * (which match), so there is no need to reverse the endianness of the data
> - * representation of the signature. However, the choice between BE32 and BE8
> - * is done by the linker, so we cannot know whether code and data endianness
> - * will be mixed before the linker is invoked.
> - */
> -
> -#define RSEQ_SIG_CODE 0xe7f5def3
> -
> -#ifndef __ASSEMBLER__
> -
> -#define RSEQ_SIG_DATA \
> - ({ \
> - int sig; \
> - asm volatile ("b 2f\n\t" \
> - "1: .inst " __rseq_str(RSEQ_SIG_CODE) "\n\t" \
> - "2:\n\t" \
> - "ldr %[sig], 1b\n\t" \
> - : [sig] "=r" (sig)); \
> - sig; \
> - })
> -
> -#define RSEQ_SIG RSEQ_SIG_DATA
> -
> -#endif
> +#define RSEQ_SIG 0x53053053
I don't get why you're reverting back to this old signature value, when the
one we came up with will work well when interpreted as an instruction in the
*vast* majority of scenarios that people care about (A32/T32 little-endian).
I think you might be under-estimating just how dead things like BE32 really
are.
That said, when you ran into .inst.n/.inst.w issues, did you try something
along the lines of the WASM() macro we use in arch/arm/, which adds the ".w"
suffix when targetting Thumb?
Will
^ permalink raw reply
* Re: [PATCH V34 23/29] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Daniel Borkmann @ 2019-06-24 15:15 UTC (permalink / raw)
To: Matthew Garrett, jmorris
Cc: linux-security-module, linux-kernel, linux-api, David Howells,
Alexei Starovoitov, Matthew Garrett, netdev, Chun-Yi Lee, jannh,
bpf
In-Reply-To: <20190622000358.19895-24-matthewgarrett@google.com>
On 06/22/2019 02:03 AM, Matthew Garrett wrote:
> From: David Howells <dhowells@redhat.com>
>
> There are some bpf functions can be used to read kernel memory:
Nit: that
> bpf_probe_read, bpf_probe_write_user and bpf_trace_printk. These allow
Please explain how bpf_probe_write_user reads kernel memory ... ?!
> private keys in kernel memory (e.g. the hibernation image signing key) to
> be read by an eBPF program and kernel memory to be altered without
... and while we're at it, also how they allow "kernel memory to be
altered without restriction". I've been pointing this false statement
out long ago.
> restriction. Disable them if the kernel has been locked down in
> confidentiality mode.
>
> Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> cc: netdev@vger.kernel.org
> cc: Chun-Yi Lee <jlee@suse.com>
> cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
Nacked-by: Daniel Borkmann <daniel@iogearbox.net>
[...]
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index d64c00afceb5..638f9b00a8df 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -137,6 +137,10 @@ BPF_CALL_3(bpf_probe_read, void *, dst, u32, size, const void *, unsafe_ptr)
> {
> int ret;
>
> + ret = security_locked_down(LOCKDOWN_BPF_READ);
> + if (ret)
> + return ret;
This whole thing is still buggy as has been pointed out before by
Jann. For helpers like above and few others below, error conditions
must clear the buffer ...
> ret = probe_kernel_read(dst, unsafe_ptr, size);
> if (unlikely(ret < 0))
> memset(dst, 0, size);
> @@ -156,6 +160,12 @@ static const struct bpf_func_proto bpf_probe_read_proto = {
> BPF_CALL_3(bpf_probe_write_user, void *, unsafe_ptr, const void *, src,
> u32, size)
> {
> + int ret;
> +
> + ret = security_locked_down(LOCKDOWN_BPF_READ);
> + if (ret)
> + return ret;
> +
> /*
> * Ensure we're in user context which is safe for the helper to
> * run. This helper has no business in a kthread.
> @@ -205,7 +215,11 @@ BPF_CALL_5(bpf_trace_printk, char *, fmt, u32, fmt_size, u64, arg1,
> int fmt_cnt = 0;
> u64 unsafe_addr;
> char buf[64];
> - int i;
> + int i, ret;
> +
> + ret = security_locked_down(LOCKDOWN_BPF_READ);
> + if (ret)
> + return ret;
>
> /*
> * bpf_check()->check_func_arg()->check_stack_boundary()
> @@ -534,6 +548,10 @@ BPF_CALL_3(bpf_probe_read_str, void *, dst, u32, size,
> {
> int ret;
>
> + ret = security_locked_down(LOCKDOWN_BPF_READ);
> + if (ret)
> + return ret;
> +
> /*
> * The strncpy_from_unsafe() call will likely not fill the entire
> * buffer, but that's okay in this circumstance as we're probing
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 5a08c17f224d..2eea2cc13117 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -33,6 +33,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_KCORE] = "/proc/kcore access",
> [LOCKDOWN_KPROBES] = "use of kprobes",
> + [LOCKDOWN_BPF_READ] = "use of bpf to read kernel RAM",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
>
>
^ permalink raw reply
* [PATCH 25/25] fsinfo: ufs - add sb operation fsinfo() [ver #14]
From: David Howells @ 2019-06-24 14:12 UTC (permalink / raw)
To: viro; +Cc: dhowells, raven, mszeredi, linux-api, linux-fsdevel, linux-kernel
In-Reply-To: <156138532485.25627.7459410522109581052.stgit@warthog.procyon.org.uk>
From: Ian Kent <raven@themaw.net>
The new fsinfo() system call adds a new super block operation
->fsinfo() which is used by file systems to provide file
system specific information for fsinfo() requests.
The fsinfo() request FSINFO_ATTR_PARAMETERS provides the same
function as sb operation ->show_options() so it needs to be
implemented by any file system that provides ->show_options()
as a minimum.
Signed-off-by: Ian Kent <raven@themaw.net>
Signed-off-by: David Howells <dhowells@redhat.com>
---
fs/ufs/super.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 58 insertions(+)
diff --git a/fs/ufs/super.c b/fs/ufs/super.c
index 84c0c5178cd2..40a3e9db8ac7 100644
--- a/fs/ufs/super.c
+++ b/fs/ufs/super.c
@@ -89,6 +89,7 @@
#include <linux/mount.h>
#include <linux/seq_file.h>
#include <linux/iversion.h>
+#include <linux/fsinfo.h>
#include "ufs_fs.h"
#include "ufs.h"
@@ -1401,6 +1402,60 @@ static int ufs_show_options(struct seq_file *seq, struct dentry *root)
return 0;
}
+#ifdef CONFIG_FSINFO
+static int ufs_fsinfo_print_token(struct fsinfo_kparams *params, const char *token)
+{
+ char *new, *key, *value;
+
+ new = kstrdup(token, GFP_KERNEL);
+ if (!new)
+ return -ENOMEM;
+
+ key = new;
+ value = strchr(new, '=');
+ if (value)
+ *value++ = '\0';
+
+ fsinfo_note_param(params, key, value);
+
+ kfree(new);
+ return 0;
+}
+
+/*
+ * Get filesystem information.
+ */
+static int ufs_fsinfo(struct path *path, struct fsinfo_kparams *params)
+{
+ struct ufs_sb_info *sbi = UFS_SB(path->dentry->d_sb);
+ unsigned mval = sbi->s_mount_opt & UFS_MOUNT_UFSTYPE;
+ const struct match_token *tp = tokens;
+ int ret;
+
+ switch (params->request) {
+ case FSINFO_ATTR_PARAMETERS:
+ fsinfo_note_sb_params(params, path->dentry->d_sb->s_flags);
+ while (tp->token != Opt_onerror_panic && tp->token != mval)
+ ++tp;
+ BUG_ON(tp->token == Opt_onerror_panic);
+ ret = ufs_fsinfo_print_token(params, tp->pattern);
+ if (ret)
+ return ret;
+ mval = sbi->s_mount_opt & UFS_MOUNT_ONERROR;
+ while (tp->token != Opt_err && tp->token != mval)
+ ++tp;
+ BUG_ON(tp->token == Opt_err);
+ ret = ufs_fsinfo_print_token(params, tp->pattern);
+ if (ret)
+ return ret;
+ return params->usage;
+
+ default:
+ return generic_fsinfo(path, params);
+ }
+}
+#endif /* CONFIG_FSINFO */
+
static int ufs_statfs(struct dentry *dentry, struct kstatfs *buf)
{
struct super_block *sb = dentry->d_sb;
@@ -1496,6 +1551,9 @@ static const struct super_operations ufs_super_ops = {
.statfs = ufs_statfs,
.remount_fs = ufs_remount,
.show_options = ufs_show_options,
+#ifdef CONFIG_FSINFO
+ .fsinfo = ufs_fsinfo,
+#endif
};
static struct dentry *ufs_mount(struct file_system_type *fs_type,
^ permalink raw reply related
* [PATCH 24/25] fsinfo: bpf - add sb operation fsinfo() [ver #14]
From: David Howells @ 2019-06-24 14:12 UTC (permalink / raw)
To: viro; +Cc: dhowells, raven, mszeredi, linux-api, linux-fsdevel, linux-kernel
In-Reply-To: <156138532485.25627.7459410522109581052.stgit@warthog.procyon.org.uk>
From: Ian Kent <raven@themaw.net>
The new fsinfo() system call adds a new super block operation
->fsinfo() which is used by file systems to provide file
system specific information for fsinfo() requests.
The fsinfo() request FSINFO_ATTR_PARAMETERS provides the same
function as sb operation ->show_options() so it needs to be
implemented by any file system that provides ->show_options()
as a minimum.
Signed-off-by: Ian Kent <raven@themaw.net>
Signed-off-by: David Howells <dhowells@redhat.com>
---
kernel/bpf/inode.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/kernel/bpf/inode.c b/kernel/bpf/inode.c
index 6e22363054b1..8c3575eca976 100644
--- a/kernel/bpf/inode.c
+++ b/kernel/bpf/inode.c
@@ -23,6 +23,7 @@
#include <linux/filter.h>
#include <linux/bpf.h>
#include <linux/bpf_trace.h>
+#include <linux/fsinfo.h>
enum bpf_type {
BPF_TYPE_UNSPEC = 0,
@@ -567,6 +568,27 @@ static int bpf_show_options(struct seq_file *m, struct dentry *root)
return 0;
}
+#ifdef CONFIG_FSINFO
+/*
+ * Get filesystem information.
+ */
+static int bpf_fsinfo(struct path *path, struct fsinfo_kparams *params)
+{
+ umode_t mode = d_inode(path->dentry)->i_mode & S_IALLUGO & ~S_ISVTX;
+
+ switch (params->request) {
+ case FSINFO_ATTR_PARAMETERS:
+ fsinfo_note_sb_params(params, path->dentry->d_sb->s_flags);
+ if (mode != S_IRWXUGO)
+ fsinfo_note_paramf(params, "mode", "%o", mode);
+ return params->usage;
+
+ default:
+ return generic_fsinfo(path, params);
+ }
+}
+#endif /* CONFIG_FSINFO */
+
static void bpf_free_inode(struct inode *inode)
{
enum bpf_type type;
@@ -583,6 +605,9 @@ static const struct super_operations bpf_super_ops = {
.drop_inode = generic_delete_inode,
.show_options = bpf_show_options,
.free_inode = bpf_free_inode,
+#ifdef CONFIG_FSINFO
+ .fsinfo = bpf_fsinfo,
+#endif
};
enum {
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox