[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <202009301554.590642EBE@keescook>
Date: Wed, 30 Sep 2020 15:59:37 -0700
From: Kees Cook <keescook@...omium.org>
To: YiFei Zhu <zhuyifei1999@...il.com>
Cc: containers@...ts.linux-foundation.org,
YiFei Zhu <yifeifz2@...inois.edu>, bpf@...r.kernel.org,
linux-kernel@...r.kernel.org, Aleksa Sarai <cyphar@...har.com>,
Andrea Arcangeli <aarcange@...hat.com>,
Andy Lutomirski <luto@...capital.net>,
David Laight <David.Laight@...lab.com>,
Dimitrios Skarlatos <dskarlat@...cmu.edu>,
Giuseppe Scrivano <gscrivan@...hat.com>,
Hubertus Franke <frankeh@...ibm.com>,
Jack Chen <jianyan2@...inois.edu>,
Jann Horn <jannh@...gle.com>,
Josep Torrellas <torrella@...inois.edu>,
Tianyin Xu <tyxu@...inois.edu>,
Tobin Feldman-Fitzthum <tobin@....com>,
Tycho Andersen <tycho@...ho.pizza>,
Valentin Rothberg <vrothber@...hat.com>,
Will Drewry <wad@...omium.org>
Subject: Re: [PATCH v3 seccomp 5/5] seccomp/cache: Report cache data through
/proc/pid/seccomp_cache
On Wed, Sep 30, 2020 at 10:19:16AM -0500, YiFei Zhu wrote:
> From: YiFei Zhu <yifeifz2@...inois.edu>
>
> Currently the kernel does not provide an infrastructure to translate
> architecture numbers to a human-readable name. Translating syscall
> numbers to syscall names is possible through FTRACE_SYSCALL
> infrastructure but it does not provide support for compat syscalls.
>
> This will create a file for each PID as /proc/pid/seccomp_cache.
> The file will be empty when no seccomp filters are loaded, or be
> in the format of:
> <arch name> <decimal syscall number> <ALLOW | FILTER>
> where ALLOW means the cache is guaranteed to allow the syscall,
> and filter means the cache will pass the syscall to the BPF filter.
>
> For the docker default profile on x86_64 it looks like:
> x86_64 0 ALLOW
> x86_64 1 ALLOW
> x86_64 2 ALLOW
> x86_64 3 ALLOW
> [...]
> x86_64 132 ALLOW
> x86_64 133 ALLOW
> x86_64 134 FILTER
> x86_64 135 FILTER
> x86_64 136 FILTER
> x86_64 137 ALLOW
> x86_64 138 ALLOW
> x86_64 139 FILTER
> x86_64 140 ALLOW
> x86_64 141 ALLOW
> [...]
>
> This file is guarded by CONFIG_DEBUG_SECCOMP_CACHE with a default
> of N because I think certain users of seccomp might not want the
> application to know which syscalls are definitely usable. For
> the same reason, it is also guarded by CAP_SYS_ADMIN.
>
> Suggested-by: Jann Horn <jannh@...gle.com>
> Link: https://lore.kernel.org/lkml/CAG48ez3Ofqp4crXGksLmZY6=fGrF_tWyUCg7PBkAetvbbOPeOA@mail.gmail.com/
> Signed-off-by: YiFei Zhu <yifeifz2@...inois.edu>
> ---
> arch/Kconfig | 15 +++++++++++
> arch/x86/include/asm/seccomp.h | 3 +++
> fs/proc/base.c | 3 +++
> include/linux/seccomp.h | 5 ++++
> kernel/seccomp.c | 46 ++++++++++++++++++++++++++++++++++
> 5 files changed, 72 insertions(+)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index ca867b2a5d71..b840cadcc882 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -478,6 +478,7 @@ config HAVE_ARCH_SECCOMP_CACHE_NR_ONLY
> - all the requirements for HAVE_ARCH_SECCOMP_FILTER
> - SECCOMP_ARCH_DEFAULT
> - SECCOMP_ARCH_DEFAULT_NR
> + - SECCOMP_ARCH_DEFAULT_NAME
>
> config SECCOMP
> prompt "Enable seccomp to safely execute untrusted bytecode"
> @@ -532,6 +533,20 @@ config SECCOMP_CACHE_NR_ONLY
>
> endchoice
>
> +config DEBUG_SECCOMP_CACHE
naming nit: I prefer where what how order, so SECCOMP_CACHE_DEBUG.
> + bool "Show seccomp filter cache status in /proc/pid/seccomp_cache"
> + depends on SECCOMP_CACHE_NR_ONLY
> + depends on PROC_FS
> + help
> + This is enables /proc/pid/seccomp_cache interface to monitor
> + seccomp cache data. The file format is subject to change. Reading
> + the file requires CAP_SYS_ADMIN.
> +
> + This option is for debugging only. Enabling present the risk that
> + an adversary may be able to infer the seccomp filter logic.
> +
> + If unsure, say N.
> +
> config HAVE_ARCH_STACKLEAK
> bool
> help
> diff --git a/arch/x86/include/asm/seccomp.h b/arch/x86/include/asm/seccomp.h
> index 7b3a58271656..33ccc074be7a 100644
> --- a/arch/x86/include/asm/seccomp.h
> +++ b/arch/x86/include/asm/seccomp.h
> @@ -19,13 +19,16 @@
> #ifdef CONFIG_X86_64
> # define SECCOMP_ARCH_DEFAULT AUDIT_ARCH_X86_64
> # define SECCOMP_ARCH_DEFAULT_NR NR_syscalls
> +# define SECCOMP_ARCH_DEFAULT_NAME "x86_64"
> # ifdef CONFIG_COMPAT
> # define SECCOMP_ARCH_COMPAT AUDIT_ARCH_I386
> # define SECCOMP_ARCH_COMPAT_NR IA32_NR_syscalls
> +# define SECCOMP_ARCH_COMPAT_NAME "x86_32"
I think this should be "ia32"? Is there a good definitive guide on this
naming convention?
--
Kees Cook
Powered by blists - more mailing lists