Discussion:
[PATCH 0/2] RISC-V: Add support for SECCOMP
Palmer Dabbelt
2018-10-24 20:40:34 UTC
Permalink
I'm pretty sure this is our largest patch set since the original kernel
contribution, and it's certainly the one with the most contributors.
While I don't have anything else I know I'm going to submit for the
merge window, I would be somewhat surprised if I didn't screw anything
up.
Hi Palmer,
Do you plan to land wip-seccomp in 4.20?
http://lists.infradead.org/pipermail/linux-riscv/2018-August/001182.html
david
I've updated the patches to live on top of 4.19 as well as cleaning up
the Kconfig entry. Unless anyone has any comments I'll add them to
for-next and submit a PR next week.

Thanks for the reminder!
Palmer Dabbelt
2018-10-24 20:40:36 UTC
Permalink
From: "Wesley W. Terpstra" <***@sifive.com>

This is a fairly straight-forward implementation of seccomp for RISC-V
systems.

Signed-off-by: Wesley W. Terpstra <***@sifive.com>
Signed-off-by: Palmer Dabbelt <***@sifive.com>
---
arch/riscv/Kconfig | 18 ++++++++++++++++++
arch/riscv/include/asm/seccomp.h | 10 ++++++++++
arch/riscv/include/asm/syscall.h | 6 ++++++
arch/riscv/include/asm/thread_info.h | 1 +
include/uapi/linux/audit.h | 1 +
5 files changed, 36 insertions(+)
create mode 100644 arch/riscv/include/asm/seccomp.h

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index a344980287a5..28abe47602a1 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -28,6 +28,7 @@ config RISCV
select GENERIC_STRNLEN_USER
select GENERIC_SMP_IDLE_THREAD
select GENERIC_ATOMIC64 if !64BIT || !RISCV_ISA_A
+ select HAVE_ARCH_SECCOMP_FILTER
select HAVE_MEMBLOCK
select HAVE_MEMBLOCK_NODE_MAP
select HAVE_DMA_CONTIGUOUS
@@ -214,6 +215,22 @@ menu "Kernel type"

source "kernel/Kconfig.hz"

+config SECCOMP
+ bool "Enable seccomp to safely compute untrusted bytecode"
+
+ help
+ This kernel feature is useful for number crunching applications
+ that may need to compute untrusted bytecode during their
+ execution. By using pipes or other transports made available to
+ the process as file descriptors supporting the read/write
+ syscalls, it's possible to isolate those applications in
+ their own address space using seccomp. Once seccomp is
+ enabled via prctl(PR_SET_SECCOMP), it cannot be disabled
+ and the task is only allowed to execute a few safe syscalls
+ defined by each seccomp mode.
+
+ If unsure, say Y. Only embedded should say N here.
+
endmenu

menu "Bus support"
@@ -243,3 +260,4 @@ menu "Power management options"
source kernel/power/Kconfig

endmenu
+
diff --git a/arch/riscv/include/asm/seccomp.h b/arch/riscv/include/asm/seccomp.h
new file mode 100644
index 000000000000..c1b4407f1038
--- /dev/null
+++ b/arch/riscv/include/asm/seccomp.h
@@ -0,0 +1,10 @@
+/* Copyright 2018 SiFive, Inc. */
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_RISCV_SECCOMP_H
+#define _ASM_RISCV_SECCOMP_H
+
+#include <asm/unistd.h>
+
+#include <asm-generic/seccomp.h>
+
+#endif /* _ASM_RISCV_SECCOMP_H */
diff --git a/arch/riscv/include/asm/syscall.h b/arch/riscv/include/asm/syscall.h
index 8d25f8904c00..d24f774f39df 100644
--- a/arch/riscv/include/asm/syscall.h
+++ b/arch/riscv/include/asm/syscall.h
@@ -19,6 +19,7 @@
#define _ASM_RISCV_SYSCALL_H

#include <linux/sched.h>
+#include <uapi/linux/audit.h>
#include <linux/err.h>

/* The array of function pointers for syscalls. */
@@ -99,4 +100,9 @@ static inline void syscall_set_arguments(struct task_struct *task,
memcpy(&regs->a1 + i * sizeof(regs->a1), args, n * sizeof(regs->a0));
}

+static inline int syscall_get_arch(void)
+{
+ return AUDIT_ARCH_RISCV;
+}
+
#endif /* _ASM_RISCV_SYSCALL_H */
diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h
index f8fa1cd2dad9..374973dc05c6 100644
--- a/arch/riscv/include/asm/thread_info.h
+++ b/arch/riscv/include/asm/thread_info.h
@@ -80,6 +80,7 @@ struct thread_info {
#define TIF_RESTORE_SIGMASK 4 /* restore signal mask in do_signal() */
#define TIF_MEMDIE 5 /* is terminating due to OOM killer */
#define TIF_SYSCALL_TRACEPOINT 6 /* syscall tracepoint instrumentation */
+#define TIF_SECCOMP 7 /* seccomp syscall filtering active */

#define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE)
#define _TIF_NOTIFY_RESUME (1 << TIF_NOTIFY_RESUME)
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 818ae690ab79..c16fa1a76659 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -399,6 +399,7 @@ enum {
/* do not define AUDIT_ARCH_PPCLE since it is not supported by audit */
#define AUDIT_ARCH_PPC64 (EM_PPC64|__AUDIT_ARCH_64BIT)
#define AUDIT_ARCH_PPC64LE (EM_PPC64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
+#define AUDIT_ARCH_RISCV (EM_RISCV)
#define AUDIT_ARCH_S390 (EM_S390)
#define AUDIT_ARCH_S390X (EM_S390|__AUDIT_ARCH_64BIT)
#define AUDIT_ARCH_SH (EM_SH)
--
2.18.1
Kees Cook
2018-10-24 21:42:03 UTC
Permalink
Post by Palmer Dabbelt
This is a fairly straight-forward implementation of seccomp for RISC-V
systems.
---
arch/riscv/Kconfig | 18 ++++++++++++++++++
arch/riscv/include/asm/seccomp.h | 10 ++++++++++
arch/riscv/include/asm/syscall.h | 6 ++++++
arch/riscv/include/asm/thread_info.h | 1 +
include/uapi/linux/audit.h | 1 +
5 files changed, 36 insertions(+)
create mode 100644 arch/riscv/include/asm/seccomp.h
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index a344980287a5..28abe47602a1 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -28,6 +28,7 @@ config RISCV
select GENERIC_STRNLEN_USER
select GENERIC_SMP_IDLE_THREAD
select GENERIC_ATOMIC64 if !64BIT || !RISCV_ISA_A
+ select HAVE_ARCH_SECCOMP_FILTER
I think this patch is missing most of the actual seccomp glue?

config HAVE_ARCH_SECCOMP_FILTER
bool
help
An arch should select this symbol if it provides all of these things:
- syscall_get_arch()
- syscall_get_arguments()
- syscall_rollback()
- syscall_set_return_value()
- SIGSYS siginfo_t support
- secure_computing is called from a ptrace_event()-safe context
- secure_computing return value is checked and a return value of -1
results in the system call being skipped immediately.
- seccomp syscall wired up

I only see syscall_get_arch(). Nothing is using TIF_SECCOMP (I'd
expect a masked check in entry.S -- it seems like tracepoints are
getting missed too? I see it handled in ptrace.c but not checked in
entry.S?) There's no checking for seccomp in ptrace.c, etc.

At the very least, I think the Kconfigs should not be included in this
patch. The other things are needed, but without everything else,
seccomp isn't actually available. :)

Reading the per-arch Kconfigs, I am reminded I still need to move
CONFIG_SECCOMP up into arch/Kconfig. :P

-Kees
--
Kees Cook
Kees Cook
2018-10-24 22:34:45 UTC
Permalink
Post by Kees Cook
config HAVE_ARCH_SECCOMP_FILTER
bool
help
- syscall_get_arch()
- syscall_get_arguments()
- syscall_rollback()
- syscall_set_return_value()
- SIGSYS siginfo_t support
- secure_computing is called from a ptrace_event()-safe context
- secure_computing return value is checked and a return value of -1
results in the system call being skipped immediately.
- seccomp syscall wired up
Oh, and I should add to this list, "passes
tools/testing/selftests/seccomp/seccomp_bpf test". :)
--
Kees Cook
Andy Lutomirski
2018-10-25 21:02:20 UTC
Permalink
Post by Kees Cook
Post by Palmer Dabbelt
This is a fairly straight-forward implementation of seccomp for RISC-V
systems.
---
arch/riscv/Kconfig | 18 ++++++++++++++++++
arch/riscv/include/asm/seccomp.h | 10 ++++++++++
arch/riscv/include/asm/syscall.h | 6 ++++++
arch/riscv/include/asm/thread_info.h | 1 +
include/uapi/linux/audit.h | 1 +
5 files changed, 36 insertions(+)
create mode 100644 arch/riscv/include/asm/seccomp.h
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index a344980287a5..28abe47602a1 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -28,6 +28,7 @@ config RISCV
select GENERIC_STRNLEN_USER
select GENERIC_SMP_IDLE_THREAD
select GENERIC_ATOMIC64 if !64BIT || !RISCV_ISA_A
+ select HAVE_ARCH_SECCOMP_FILTER
I think this patch is missing most of the actual seccomp glue?
config HAVE_ARCH_SECCOMP_FILTER
bool
help
- syscall_get_arch()
- syscall_get_arguments()
- syscall_rollback()
- syscall_set_return_value()
- SIGSYS siginfo_t support
- secure_computing is called from a ptrace_event()-safe context
- secure_computing return value is checked and a return value of -1
results in the system call being skipped immediately.
- seccomp syscall wired up
I only see syscall_get_arch(). Nothing is using TIF_SECCOMP (I'd
expect a masked check in entry.S -- it seems like tracepoints are
getting missed too? I see it handled in ptrace.c but not checked in
entry.S?) There's no checking for seccomp in ptrace.c, etc.
Hi RISC-V people:

I strongly, strongly suggest that you rewrite your asm to work the way
that x86's does: have a function called prepare_exit_to_usermode() and
make it work more or less like x86's. Doing all the exit work in asm
like you are is just setting you up for a world of pain.

--Andy
Paul Moore
2018-10-25 20:36:41 UTC
Permalink
On Thu, Oct 25, 2018 at 2:31 PM David Abdurachmanov
...
Palmer,
Half of the patch seems to touch audit parts. I started working on audit
support this morning, and I can boot Fedora with audit traces.
[ 0.312000] audit: initializing netlink subsys (disabled)
[ 0.316000] audit: type=2000 audit(0.316:1): state=initialized
audit_enabled=0 res=1
[ 7.288000] audit: type=1130 audit(1529665913.772:2): pid=1 uid=0
auid=4294967295 ses=4294967295 msg='unit=systemd-remount-fs
comm="systemd" exe="/usr/lib/systemd/systemd" hostname=? addr=?
terminal=? res=success'
[ 7.684000] audit: type=1130 audit(1529665914.176:3): pid=1 uid=0
auid=4294967295 ses=4294967295 msg='unit=systemd-sysctl comm="systemd"
exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=?
res=success'
[..]
I am still working on audit user-space support for better testing.
I suggest we first implement audit and then seccomp.
FYI, while small and far from comprehensive, we do have a test suite
we use for basic validation of the audit kernel bits which may be
helpful while you're working on the audit enablement:

* https://github.com/linux-audit/audit-testsuite
--
paul moore
www.paul-moore.com
David Abdurachmanov
2018-10-28 11:07:55 UTC
Permalink
Post by Paul Moore
On Thu, Oct 25, 2018 at 2:31 PM David Abdurachmanov
...
Palmer,
Half of the patch seems to touch audit parts. I started working on audit
support this morning, and I can boot Fedora with audit traces.
[ 0.312000] audit: initializing netlink subsys (disabled)
[ 0.316000] audit: type=2000 audit(0.316:1): state=initialized
audit_enabled=0 res=1
[ 7.288000] audit: type=1130 audit(1529665913.772:2): pid=1 uid=0
auid=4294967295 ses=4294967295 msg='unit=systemd-remount-fs
comm="systemd" exe="/usr/lib/systemd/systemd" hostname=? addr=?
terminal=? res=success'
[ 7.684000] audit: type=1130 audit(1529665914.176:3): pid=1 uid=0
auid=4294967295 ses=4294967295 msg='unit=systemd-sysctl comm="systemd"
exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=?
res=success'
[..]
I am still working on audit user-space support for better testing.
I suggest we first implement audit and then seccomp.
FYI, while small and far from comprehensive, we do have a test suite
we use for basic validation of the audit kernel bits which may be
* https://github.com/linux-audit/audit-testsuite
Currently I checked the following to work:
- /proc/self/loginuid (required by DNF [package manager])
- auditctl (checked several different example rules from internet)
- aulast
- aulastlog
- ausearch
- ausyscall
- aureport
- autrace (compared some syscalls to strace: order and
return value/input arguments seems to be correct)

I checked audit-testsuite yesterday and it seems to be only for
x86-64 / x86-32. After adjusting it (MODE, syscalls) I am at:

Failed 4/14 test programs. 19/88 subtests failed.

I don't plan to look further in the failure, e.g.:
- syscall_socketcall: that's an old stuff and not relevant to
new arches
- syscall_module: Fedora kernel currently is not compiled
with kernel loadable module support
- filter_exclude: two tests fail because id -Z doesn't print
any categories, but "semanage login -l" output is identical
between x86_64 and riscv64
- netfilter_pkt: don't have CONFIG_IP_NF_MANGLE enabled

Fedora kernel currently has minimal CONFIG_* options
and is built without loadable module support.

I will send the patches for review soon.

david
Christoph Hellwig
2018-10-27 07:55:59 UTC
Permalink
Post by Palmer Dabbelt
+ bool "Enable seccomp to safely compute untrusted bytecode"
+
+ help
The empty line above is odd, please drop it.
Post by Palmer Dabbelt
+++ b/arch/riscv/include/asm/seccomp.h
@@ -0,0 +1,10 @@
+/* Copyright 2018 SiFive, Inc. */
+/* SPDX-License-Identifier: GPL-2.0 */
The SPDX tag should go into the first line.
Post by Palmer Dabbelt
+#ifndef _ASM_RISCV_SECCOMP_H
+#define _ASM_RISCV_SECCOMP_H
+
+#include <asm/unistd.h>
+
+#include <asm-generic/seccomp.h>
And while at it I'd drop this empty line as well.
Palmer Dabbelt
2018-10-24 20:40:35 UTC
Permalink
This should never have been inside our arch port to begin with, it's
just a relic from when we were maintaining out of tree patches.

Signed-off-by: Palmer Dabbelt <***@sifive.com>
---
arch/riscv/include/asm/elf.h | 3 ---
include/uapi/linux/elf-em.h | 1 +
2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/riscv/include/asm/elf.h b/arch/riscv/include/asm/elf.h
index a1ef503d616e..697fc23b0d5a 100644
--- a/arch/riscv/include/asm/elf.h
+++ b/arch/riscv/include/asm/elf.h
@@ -16,9 +16,6 @@
#include <asm/auxvec.h>
#include <asm/byteorder.h>

-/* TODO: Move definition into include/uapi/linux/elf-em.h */
-#define EM_RISCV 0xF3
-
/*
* These are used to set parameters in the core dumps.
*/
diff --git a/include/uapi/linux/elf-em.h b/include/uapi/linux/elf-em.h
index 31aa10178335..93722e60204c 100644
--- a/include/uapi/linux/elf-em.h
+++ b/include/uapi/linux/elf-em.h
@@ -41,6 +41,7 @@
#define EM_TILEPRO 188 /* Tilera TILEPro */
#define EM_MICROBLAZE 189 /* Xilinx MicroBlaze */
#define EM_TILEGX 191 /* Tilera TILE-Gx */
+#define EM_RISCV 243 /* RISC-V */
#define EM_BPF 247 /* Linux BPF - in-kernel virtual machine */
#define EM_FRV 0x5441 /* Fujitsu FR-V */
--
2.18.1
Kees Cook
2018-10-24 21:26:05 UTC
Permalink
Post by Palmer Dabbelt
This should never have been inside our arch port to begin with, it's
just a relic from when we were maintaining out of tree patches.
Reviewed-by: Kees Cook <***@chromium.org>

-Kees
Post by Palmer Dabbelt
---
arch/riscv/include/asm/elf.h | 3 ---
include/uapi/linux/elf-em.h | 1 +
2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/arch/riscv/include/asm/elf.h b/arch/riscv/include/asm/elf.h
index a1ef503d616e..697fc23b0d5a 100644
--- a/arch/riscv/include/asm/elf.h
+++ b/arch/riscv/include/asm/elf.h
@@ -16,9 +16,6 @@
#include <asm/auxvec.h>
#include <asm/byteorder.h>
-/* TODO: Move definition into include/uapi/linux/elf-em.h */
-#define EM_RISCV 0xF3
-
/*
* These are used to set parameters in the core dumps.
*/
diff --git a/include/uapi/linux/elf-em.h b/include/uapi/linux/elf-em.h
index 31aa10178335..93722e60204c 100644
--- a/include/uapi/linux/elf-em.h
+++ b/include/uapi/linux/elf-em.h
@@ -41,6 +41,7 @@
#define EM_TILEPRO 188 /* Tilera TILEPro */
#define EM_MICROBLAZE 189 /* Xilinx MicroBlaze */
#define EM_TILEGX 191 /* Tilera TILE-Gx */
+#define EM_RISCV 243 /* RISC-V */
#define EM_BPF 247 /* Linux BPF - in-kernel virtual machine */
#define EM_FRV 0x5441 /* Fujitsu FR-V */
--
2.18.1
--
Kees Cook
David Abdurachmanov
2018-10-27 09:10:34 UTC
Permalink
Post by Palmer Dabbelt
This should never have been inside our arch port to begin with, it's
just a relic from when we were maintaining out of tree patches.
Looks good, and probably harmless enought that we should pick it up
That would be nice. The audit parts I am working on depends on this patch.

Tested-by: David Abdurachmanov <***@gmail.com>
Palmer Dabbelt
2018-10-27 06:07:11 UTC
Permalink
Post by Palmer Dabbelt
This is a fairly straight-forward implementation of seccomp for RISC-V
systems.
---
arch/riscv/Kconfig | 18 ++++++++++++++++++
arch/riscv/include/asm/seccomp.h | 10 ++++++++++
arch/riscv/include/asm/syscall.h | 6 ++++++
arch/riscv/include/asm/thread_info.h | 1 +
include/uapi/linux/audit.h | 1 +
5 files changed, 36 insertions(+)
create mode 100644 arch/riscv/include/asm/seccomp.h
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index a344980287a5..28abe47602a1 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -28,6 +28,7 @@ config RISCV
select GENERIC_STRNLEN_USER
select GENERIC_SMP_IDLE_THREAD
select GENERIC_ATOMIC64 if !64BIT || !RISCV_ISA_A
+ select HAVE_ARCH_SECCOMP_FILTER
select HAVE_MEMBLOCK
select HAVE_MEMBLOCK_NODE_MAP
select HAVE_DMA_CONTIGUOUS
@@ -214,6 +215,22 @@ menu "Kernel type"
source "kernel/Kconfig.hz"
+config SECCOMP
+ bool "Enable seccomp to safely compute untrusted bytecode"
+
+ help
+ This kernel feature is useful for number crunching applications
+ that may need to compute untrusted bytecode during their
+ execution. By using pipes or other transports made available to
+ the process as file descriptors supporting the read/write
+ syscalls, it's possible to isolate those applications in
+ their own address space using seccomp. Once seccomp is
+ enabled via prctl(PR_SET_SECCOMP), it cannot be disabled
+ and the task is only allowed to execute a few safe syscalls
+ defined by each seccomp mode.
+
+ If unsure, say Y. Only embedded should say N here.
+
endmenu
menu "Bus support"
@@ -243,3 +260,4 @@ menu "Power management options"
source kernel/power/Kconfig
endmenu
+
diff --git a/arch/riscv/include/asm/seccomp.h b/arch/riscv/include/asm/seccomp.h
new file mode 100644
index 000000000000..c1b4407f1038
--- /dev/null
+++ b/arch/riscv/include/asm/seccomp.h
@@ -0,0 +1,10 @@
+/* Copyright 2018 SiFive, Inc. */
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_RISCV_SECCOMP_H
+#define _ASM_RISCV_SECCOMP_H
+
+#include <asm/unistd.h>
+
+#include <asm-generic/seccomp.h>
+
+#endif /* _ASM_RISCV_SECCOMP_H */
diff --git a/arch/riscv/include/asm/syscall.h b/arch/riscv/include/asm/syscall.h
index 8d25f8904c00..d24f774f39df 100644
--- a/arch/riscv/include/asm/syscall.h
+++ b/arch/riscv/include/asm/syscall.h
@@ -19,6 +19,7 @@
#define _ASM_RISCV_SYSCALL_H
#include <linux/sched.h>
+#include <uapi/linux/audit.h>
#include <linux/err.h>
/* The array of function pointers for syscalls. */
@@ -99,4 +100,9 @@ static inline void syscall_set_arguments(struct task_struct *task,
memcpy(&regs->a1 + i * sizeof(regs->a1), args, n * sizeof(regs->a0));
}
+static inline int syscall_get_arch(void)
+{
+ return AUDIT_ARCH_RISCV;
+}
+
#endif /* _ASM_RISCV_SYSCALL_H */
diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h
index f8fa1cd2dad9..374973dc05c6 100644
--- a/arch/riscv/include/asm/thread_info.h
+++ b/arch/riscv/include/asm/thread_info.h
@@ -80,6 +80,7 @@ struct thread_info {
#define TIF_RESTORE_SIGMASK 4 /* restore signal mask in do_signal() */
#define TIF_MEMDIE 5 /* is terminating due to OOM killer */
#define TIF_SYSCALL_TRACEPOINT 6 /* syscall tracepoint instrumentation */
+#define TIF_SECCOMP 7 /* seccomp syscall filtering active */
#define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE)
#define _TIF_NOTIFY_RESUME (1 << TIF_NOTIFY_RESUME)
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 818ae690ab79..c16fa1a76659 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -399,6 +399,7 @@ enum {
/* do not define AUDIT_ARCH_PPCLE since it is not supported by audit */
#define AUDIT_ARCH_PPC64 (EM_PPC64|__AUDIT_ARCH_64BIT)
#define AUDIT_ARCH_PPC64LE (EM_PPC64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
+#define AUDIT_ARCH_RISCV (EM_RISCV)
#define AUDIT_ARCH_S390 (EM_S390)
#define AUDIT_ARCH_S390X (EM_S390|__AUDIT_ARCH_64BIT)
#define AUDIT_ARCH_SH (EM_SH)
Palmer,
Half of the patch seems to touch audit parts. I started working on audit
support this morning, and I can boot Fedora with audit traces.
[ 0.312000] audit: initializing netlink subsys (disabled)
[ 0.316000] audit: type=2000 audit(0.316:1): state=initialized
audit_enabled=0 res=1
[ 7.288000] audit: type=1130 audit(1529665913.772:2): pid=1 uid=0
auid=4294967295 ses=4294967295 msg='unit=systemd-remount-fs
comm="systemd" exe="/usr/lib/systemd/systemd" hostname=? addr=?
terminal=? res=success'
[ 7.684000] audit: type=1130 audit(1529665914.176:3): pid=1 uid=0
auid=4294967295 ses=4294967295 msg='unit=systemd-sysctl comm="systemd"
exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=?
res=success'
[..]
I am still working on audit user-space support for better testing.
I suggest we first implement audit and then seccomp.
Works for me. I'll drop my patch set for now.

Thanks!
Palmer Dabbelt
2018-10-27 06:07:20 UTC
Permalink
Post by Andy Lutomirski
Post by Kees Cook
Post by Palmer Dabbelt
This is a fairly straight-forward implementation of seccomp for RISC-V
systems.
---
arch/riscv/Kconfig | 18 ++++++++++++++++++
arch/riscv/include/asm/seccomp.h | 10 ++++++++++
arch/riscv/include/asm/syscall.h | 6 ++++++
arch/riscv/include/asm/thread_info.h | 1 +
include/uapi/linux/audit.h | 1 +
5 files changed, 36 insertions(+)
create mode 100644 arch/riscv/include/asm/seccomp.h
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index a344980287a5..28abe47602a1 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -28,6 +28,7 @@ config RISCV
select GENERIC_STRNLEN_USER
select GENERIC_SMP_IDLE_THREAD
select GENERIC_ATOMIC64 if !64BIT || !RISCV_ISA_A
+ select HAVE_ARCH_SECCOMP_FILTER
I think this patch is missing most of the actual seccomp glue?
config HAVE_ARCH_SECCOMP_FILTER
bool
help
- syscall_get_arch()
- syscall_get_arguments()
- syscall_rollback()
- syscall_set_return_value()
- SIGSYS siginfo_t support
- secure_computing is called from a ptrace_event()-safe context
- secure_computing return value is checked and a return value of -1
results in the system call being skipped immediately.
- seccomp syscall wired up
I only see syscall_get_arch(). Nothing is using TIF_SECCOMP (I'd
expect a masked check in entry.S -- it seems like tracepoints are
getting missed too? I see it handled in ptrace.c but not checked in
entry.S?) There's no checking for seccomp in ptrace.c, etc.
I strongly, strongly suggest that you rewrite your asm to work the way
that x86's does: have a function called prepare_exit_to_usermode() and
make it work more or less like x86's. Doing all the exit work in asm
like you are is just setting you up for a world of pain.
OK, thanks for the suggestion. Next time we have to change it I'll try to take
a look and figure out something sane.
Palmer Dabbelt
2018-10-29 20:27:55 UTC
Permalink
Post by David Abdurachmanov
Post by Paul Moore
On Thu, Oct 25, 2018 at 2:31 PM David Abdurachmanov
...
Palmer,
Half of the patch seems to touch audit parts. I started working on audit
support this morning, and I can boot Fedora with audit traces.
[ 0.312000] audit: initializing netlink subsys (disabled)
[ 0.316000] audit: type=2000 audit(0.316:1): state=initialized
audit_enabled=0 res=1
[ 7.288000] audit: type=1130 audit(1529665913.772:2): pid=1 uid=0
auid=4294967295 ses=4294967295 msg='unit=systemd-remount-fs
comm="systemd" exe="/usr/lib/systemd/systemd" hostname=? addr=?
terminal=? res=success'
[ 7.684000] audit: type=1130 audit(1529665914.176:3): pid=1 uid=0
auid=4294967295 ses=4294967295 msg='unit=systemd-sysctl comm="systemd"
exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=?
res=success'
[..]
I am still working on audit user-space support for better testing.
I suggest we first implement audit and then seccomp.
FYI, while small and far from comprehensive, we do have a test suite
we use for basic validation of the audit kernel bits which may be
* https://github.com/linux-audit/audit-testsuite
- /proc/self/loginuid (required by DNF [package manager])
- auditctl (checked several different example rules from internet)
- aulast
- aulastlog
- ausearch
- ausyscall
- aureport
- autrace (compared some syscalls to strace: order and
return value/input arguments seems to be correct)
I checked audit-testsuite yesterday and it seems to be only for
Failed 4/14 test programs. 19/88 subtests failed.
- syscall_socketcall: that's an old stuff and not relevant to
new arches
- syscall_module: Fedora kernel currently is not compiled
with kernel loadable module support
- filter_exclude: two tests fail because id -Z doesn't print
any categories, but "semanage login -l" output is identical
between x86_64 and riscv64
- netfilter_pkt: don't have CONFIG_IP_NF_MANGLE enabled
Fedora kernel currently has minimal CONFIG_* options
and is built without loadable module support.
I will send the patches for review soon.
Thanks!
David Abdurachmanov
2018-11-02 13:32:47 UTC
Permalink
Post by Palmer Dabbelt
Post by David Abdurachmanov
Post by Paul Moore
On Thu, Oct 25, 2018 at 2:31 PM David Abdurachmanov
...
Palmer,
Half of the patch seems to touch audit parts. I started working on audit
support this morning, and I can boot Fedora with audit traces.
[ 0.312000] audit: initializing netlink subsys (disabled)
[ 0.316000] audit: type=2000 audit(0.316:1): state=initialized
audit_enabled=0 res=1
[ 7.288000] audit: type=1130 audit(1529665913.772:2): pid=1 uid=0
auid=4294967295 ses=4294967295 msg='unit=systemd-remount-fs
comm="systemd" exe="/usr/lib/systemd/systemd" hostname=? addr=?
terminal=? res=success'
[ 7.684000] audit: type=1130 audit(1529665914.176:3): pid=1 uid=0
auid=4294967295 ses=4294967295 msg='unit=systemd-sysctl comm="systemd"
exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=?
res=success'
[..]
I am still working on audit user-space support for better testing.
I suggest we first implement audit and then seccomp.
FYI, while small and far from comprehensive, we do have a test suite
we use for basic validation of the audit kernel bits which may be
* https://github.com/linux-audit/audit-testsuite
- /proc/self/loginuid (required by DNF [package manager])
- auditctl (checked several different example rules from internet)
- aulast
- aulastlog
- ausearch
- ausyscall
- aureport
- autrace (compared some syscalls to strace: order and
return value/input arguments seems to be correct)
I checked audit-testsuite yesterday and it seems to be only for
Failed 4/14 test programs. 19/88 subtests failed.
- syscall_socketcall: that's an old stuff and not relevant to
new arches
- syscall_module: Fedora kernel currently is not compiled
with kernel loadable module support
- filter_exclude: two tests fail because id -Z doesn't print
any categories, but "semanage login -l" output is identical
between x86_64 and riscv64
- netfilter_pkt: don't have CONFIG_IP_NF_MANGLE enabled
Fedora kernel currently has minimal CONFIG_* options
and is built without loadable module support.
I will send the patches for review soon.
Thanks!
I fixed the last issue I see with SECCOMP this morning.
I also have patch on top of libseccomp-2.3.3.

Testsuite results for SIM:

Regression Test Summary
tests run: 4434
tests skipped: 88
tests passed: 4434
tests failed: 0
tests errored: 0

Testsuite results for LIVE:

Regression Test Summary
tests run: 6
tests skipped: 0
tests passed: 6
tests failed: 0
tests errored: 0

Then tested a couple examples manually w/ and w/o BPF and it
performed the same as on x86_64 (also checked exit codes &
strace output).

Upstream libseccomp has now more tests. Once I rebase & re-test
with master of libseccomp, I will send both.

david
Kees Cook
2018-11-02 15:51:55 UTC
Permalink
On Fri, Nov 2, 2018 at 6:32 AM, David Abdurachmanov
Post by David Abdurachmanov
Post by Palmer Dabbelt
Post by David Abdurachmanov
Post by Paul Moore
On Thu, Oct 25, 2018 at 2:31 PM David Abdurachmanov
...
Palmer,
Half of the patch seems to touch audit parts. I started working on audit
support this morning, and I can boot Fedora with audit traces.
[ 0.312000] audit: initializing netlink subsys (disabled)
[ 0.316000] audit: type=2000 audit(0.316:1): state=initialized
audit_enabled=0 res=1
[ 7.288000] audit: type=1130 audit(1529665913.772:2): pid=1 uid=0
auid=4294967295 ses=4294967295 msg='unit=systemd-remount-fs
comm="systemd" exe="/usr/lib/systemd/systemd" hostname=? addr=?
terminal=? res=success'
[ 7.684000] audit: type=1130 audit(1529665914.176:3): pid=1 uid=0
auid=4294967295 ses=4294967295 msg='unit=systemd-sysctl comm="systemd"
exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=?
res=success'
[..]
I am still working on audit user-space support for better testing.
I suggest we first implement audit and then seccomp.
FYI, while small and far from comprehensive, we do have a test suite
we use for basic validation of the audit kernel bits which may be
* https://github.com/linux-audit/audit-testsuite
- /proc/self/loginuid (required by DNF [package manager])
- auditctl (checked several different example rules from internet)
- aulast
- aulastlog
- ausearch
- ausyscall
- aureport
- autrace (compared some syscalls to strace: order and
return value/input arguments seems to be correct)
I checked audit-testsuite yesterday and it seems to be only for
Failed 4/14 test programs. 19/88 subtests failed.
- syscall_socketcall: that's an old stuff and not relevant to
new arches
- syscall_module: Fedora kernel currently is not compiled
with kernel loadable module support
- filter_exclude: two tests fail because id -Z doesn't print
any categories, but "semanage login -l" output is identical
between x86_64 and riscv64
- netfilter_pkt: don't have CONFIG_IP_NF_MANGLE enabled
Fedora kernel currently has minimal CONFIG_* options
and is built without loadable module support.
I will send the patches for review soon.
Thanks!
I fixed the last issue I see with SECCOMP this morning.
Can you CC me on the series? I'd love to take a look.
Post by David Abdurachmanov
I also have patch on top of libseccomp-2.3.3.
Nice! If you toss it up on github I can review that too. :)

-Kees
Post by David Abdurachmanov
Regression Test Summary
tests run: 4434
tests skipped: 88
tests passed: 4434
tests failed: 0
tests errored: 0
Regression Test Summary
tests run: 6
tests skipped: 0
tests passed: 6
tests failed: 0
tests errored: 0
Then tested a couple examples manually w/ and w/o BPF and it
performed the same as on x86_64 (also checked exit codes &
strace output).
Upstream libseccomp has now more tests. Once I rebase & re-test
with master of libseccomp, I will send both.
david
--
Kees Cook
Loading...