RE: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection

[Schaufler, Casey]

> -----Original Message-----
> From: Jiri Kosina [mailto:jikos@xxxxxxxxxx]
> Sent: Wednesday, September 12, 2018 2:05 AM
> To: Thomas Gleixner <tglx@xxxxxxxxxxxxx>; Ingo Molnar <mingo@xxxxxxxxxx>;
> Peter Zijlstra <peterz@xxxxxxxxxxxxx>; Josh Poimboeuf
> <jpoimboe@xxxxxxxxxx>; Andrea Arcangeli <aarcange@xxxxxxxxxx>;
> Woodhouse, David <dwmw@xxxxxxxxxxxx>; Andi Kleen <ak@xxxxxxxxxxxxxxx>;
> Tim Chen <tim.c.chen@xxxxxxxxxxxxxxx>; Schaufler, Casey
> <casey.schaufler@xxxxxxxxx>
> Cc: linux-kernel@xxxxxxxxxxxxxxx; x86@xxxxxxxxxx
> Subject: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection
> 
> Currently, linux kernel is basically not preventing userspace-userspace
> spectrev2 attack, because:
> 
> - IBPB is basically unused (issued only for tasks that marked themselves
>   explicitly non-dumpable, which is absolutely negligible minority of all
>   software out there), therefore cross-process branch buffer posioning
>   using spectrev2 is possible
> 
> - STIBP is completely unused, therefore cross-process branch buffer
>   poisoning using spectrev2 between processess running on two HT siblings
>   thread s is possible
> 
> This patchset changes IBPB semantics, so that it's now applied whenever
> context-switching between processess that can't use ptrace() to achieve
> the same. This admittedly comes with extra overhad on a context switch;
> systems that don't care about could disable the mitigation using
> nospectre_v2 boot option.
> The IBPB implementaion is heavily based on original patches by Tim Chen.
> 
> In addition to that, we unconditionally turn STIBP on so that HT siblings
> always have separate branch buffers.
> 
> We've been carrying IBPB implementation with the same semantics in our
> (SUSE) trees since january disclosure; STIBP was more or less ignored up
> to today.
> 
> v1->v2:
>         include IBPB changes
> v2->v3:
>         fix IBPB 'who can trace who' semantics
>         wire up STIBP flipping to SMT hotplug
> v3->v4:
> 	dropped ___ptrace_may_access(), as it's not needed
> 	fixed deadlock with LSM/audit/selinux (Andrea Arcangeli)
> 	statically patch out the ptrace check if !IBPB
> 
> v4->v5:
> 	fix MSR writing logic (Thomas Gleixner, Josh Poimboeuf)
> 
> v5->v6:
> 	propagate X86_FEATURE_RSB_CTXSW setting to sysfs
> 	propagate STIBP setting to sysfs (Thomas Gleixner)
> 	simplify arch_smt_update() (Thomas Gleixner)
> 
> Jiri Kosina (3):
>       x86/speculation: apply IBPB more strictly to avoid cross-process data leak
>       x86/speculation: Enable cross-hyperthread spectre v2 STIBP mitigation
>       x86/speculation: Propagate information about RSB filling mitigation to sysfs
> 
>  arch/x86/kernel/cpu/bugs.c | 60
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++------
>  arch/x86/mm/tlb.c          | 31 ++++++++++++++++++++-----------
>  include/linux/ptrace.h     |  4 ++++
>  kernel/cpu.c               | 11 ++++++++++-
>  kernel/ptrace.c            | 12 ++++++++----
>  5 files changed, 96 insertions(+), 22 deletions(-)
> 
> --
> Jiri Kosina
> SUSE Labs

The locking issue with SELinux has a simple fix as below.
The other LSMs don't manifest this issue. With the change to
SELinux the call to security_ptrace_access_check() can and
should be made unconditionally.

Patch is attached, whitespace damaged (known problem) patch:

SELinux: Handle audit locking for PTRACE_MODE_IBPB

The SELinux audit code locking cannot be used from the
task switching code, which is where PTRACE_MODE_IBPB comes
from. As this is a system check, not a user action, audit
is not needed, and would generate noise. Use the unaudited
check for this case.

Signed-off-by: Casey Schaufler <casey.schaufler@xxxxxxxxx>
---
 kernel/ptrace.c          | 4 +---
 security/selinux/hooks.c | 5 +++++
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 5c5e7cb597cd..202a4d9c2af7 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -330,9 +330,7 @@ int __ptrace_may_access(struct task_struct *task, unsigned int mode)
               !ptrace_has_cap(mm->user_ns, mode))))
            return -EPERM;

-       if (!(mode & PTRACE_MODE_NOACCESS_CHK))
-               return security_ptrace_access_check(task, mode);
-       return 0;
+       return security_ptrace_access_check(task, mode);
 }

 bool ptrace_may_access(struct task_struct *task, unsigned int mode)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 161a4f29f860..30d21142e9fe 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2215,7 +2215,12 @@ static int selinux_ptrace_access_check(struct task_struct *child,
 {
        u32 sid = current_sid();
        u32 csid = task_sid(child);
+       struct av_decision avd;

+       if (mode == PTRACE_MODE_IBPB)
+               return avc_has_perm_noaudit(&selinux_state, sid, csid,
+                                           SECCLASS_PROCESS, PROCESS__PTRACE,
+                                           0, &avd);
        if (mode & PTRACE_MODE_READ)
                return avc_has_perm(&selinux_state,
                                    sid, csid, SECCLASS_FILE, FILE__READ, NULL);

Attachment: casey-jiri-v6.patch
Description: casey-jiri-v6.patch