On Wed, Oct 24, 2018 at 8:35 AM, Michael Sammler <msammler@xxxxxxxxxxx> wrote:
> Add the current value of the PKRU register to data available for
> seccomp-bpf programs to work on. This allows filters based on the
> currently enabled protection keys.
As mentioned in the thread, this commit log needs to be expanded greatly. :)
>
> Cc: Kees Cook <keescook@xxxxxxxxxxxx>
> Cc: Andy Lutomirski <luto@xxxxxxxxxxxxxx>
> Cc: Will Drewry <wad@xxxxxxxxxxxx>
> Cc: linux-api@xxxxxxxxxxxxxxx
> Signed-off-by: Michael Sammler <msammler@xxxxxxxxxxx>
> ---
> arch/mips/kernel/ptrace.c | 2 ++
> arch/x86/entry/common.c | 2 ++
> include/uapi/linux/seccomp.h | 4 ++++
> kernel/seccomp.c | 2 ++
> tools/testing/selftests/seccomp/seccomp_bpf.c | 4 +++-
> 5 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/arch/mips/kernel/ptrace.c b/arch/mips/kernel/ptrace.c
> index e5ba56c0..07d55955 100644
> --- a/arch/mips/kernel/ptrace.c
> +++ b/arch/mips/kernel/ptrace.c
> @@ -1277,6 +1277,8 @@ asmlinkage long syscall_trace_enter(struct pt_regs *regs, long syscall)
> for (i = 0; i < 6; i++)
> sd.args[i] = args[i];
> sd.instruction_pointer = KSTK_EIP(current);
> + sd.pkru = 0;
> + sd.reserved = 0;
>
> ret = __secure_computing(&sd);
> if (ret == -1)
> diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
> index 3b2490b8..2afa85d7 100644
> --- a/arch/x86/entry/common.c
> +++ b/arch/x86/entry/common.c
> @@ -98,6 +98,8 @@ static long syscall_trace_enter(struct pt_regs *regs)
> sd.arch = arch;
> sd.nr = regs->orig_ax;
> sd.instruction_pointer = regs->ip;
> + sd.pkru = read_pkru();
> + sd.reserved = 0;
> #ifdef CONFIG_X86_64
> if (arch == AUDIT_ARCH_X86_64) {
> sd.args[0] = regs->di;
> diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
> index 9efc0e73..e8b9ecfc 100644
> --- a/include/uapi/linux/seccomp.h
> +++ b/include/uapi/linux/seccomp.h
> @@ -52,12 +52,16 @@
> * @instruction_pointer: at the time of the system call.
> * @args: up to 6 system call arguments always stored as 64-bit values
> * regardless of the architecture.
> + * @pkru: value of the pkru register
> + * @reserved: pad the structure to a multiple of eight bytes
> */
> struct seccomp_data {
> int nr;
> __u32 arch;
> __u64 instruction_pointer;
> __u64 args[6];
> + __u32 pkru;
> + __u32 reserved;
> };
struct seccomp_data only needs to be 32-bit aligned. And since I think
old kernels will correctly reject filters that use the new, larger,
seccomp_data (via the sizeof() checks in seccomp_check_filter(), it
would be better to leave off the "reserved" field, since it has no
meaning right now, and it would be best to "version" the seccomp_data
entirely by size.
>
> #endif /* _UAPI_LINUX_SECCOMP_H */
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index fd023ac2..b193c26e 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -91,6 +91,8 @@ static void populate_seccomp_data(struct seccomp_data *sd)
> sd->args[4] = args[4];
> sd->args[5] = args[5];
> sd->instruction_pointer = KSTK_EIP(task);
> + sd->pkru = read_pkru();
In some recent MPK usage[1] it sounded like the MPK instructions were
extremely expensive to run. I'm strongly against adding any global
performance regressions to seccomp, so if this field is slow to
populate, I think we'll need some kind of flag to have a filter opt in
to using it (and have it set to some impossible value otherwise).
[1] https://github.com/AndroidHardening/hardened_malloc/commit/e985afe0e16f4e1f0d178476342de10bce5d2c0c
> + sd->reserved = 0;
> }
>
> /**
> diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
> index e1473234..359249cd 100644
> --- a/tools/testing/selftests/seccomp/seccomp_bpf.c
> +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
> @@ -732,7 +732,9 @@ TEST(KILL_process)
> TEST(arg_out_of_range)
> {
> struct sock_filter filter[] = {
> - BPF_STMT(BPF_LD|BPF_W|BPF_ABS, syscall_arg(6)),
> + BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
> + offsetof(struct seccomp_data, reserved)
This should probably just be sizeof(struct seccomp_data).
> + + sizeof(__u32)),
> BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
> };
> struct sock_fprog prog = {
> --
> 2.11.0
>
-Kees
--
Kees Cook
