Re: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection
On Mon, 24 Sep 2018, Jiri Kosina wrote: > On Sat, 22 Sep 2018, Thomas Gleixner wrote: > > > Lunch and coffee indeed made brain work better. The simple solution was way > > too obvious. > > Ah, cool, I like it a lot. > > Do you want me to fold this into v7, or are you on it already? Please do so. I'm traveling/conferencing and only spotty online. Thanks, tglx
Re: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection
On Mon, 24 Sep 2018, Jiri Kosina wrote: > On Sat, 22 Sep 2018, Thomas Gleixner wrote: > > > Lunch and coffee indeed made brain work better. The simple solution was way > > too obvious. > > Ah, cool, I like it a lot. > > Do you want me to fold this into v7, or are you on it already? Please do so. I'm traveling/conferencing and only spotty online. Thanks, tglx
Re: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection
On Sat, 22 Sep 2018, Thomas Gleixner wrote: > Lunch and coffee indeed made brain work better. The simple solution was way > too obvious. Ah, cool, I like it a lot. Do you want me to fold this into v7, or are you on it already? Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection
On Sat, 22 Sep 2018, Thomas Gleixner wrote: > Lunch and coffee indeed made brain work better. The simple solution was way > too obvious. Ah, cool, I like it a lot. Do you want me to fold this into v7, or are you on it already? Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection
On Sat, Sep 22, 2018 at 03:30:07PM +0200, Thomas Gleixner wrote:
> On Sat, 22 Sep 2018, Thomas Gleixner wrote:
> > On Sat, 22 Sep 2018, Peter Zijlstra wrote:
> > > This has some unfortunate duplication.
> > >
> > > Lets go with it for now, but I'll see if I can do something about that
> > > later.
> >
> > Yes, I know. I tried to make the duplication smaller, but all attempts
> > ended up being a convoluted mess. I'll try again after applying more
> > coffee.
>
> Lunch and coffee indeed made brain work better. The simple solution was way
> too obvious.
> --- a/kernel/ptrace.c
> +++ b/kernel/ptrace.c
> @@ -261,6 +261,9 @@ static int ptrace_check_attach(struct ta
>
> static int ptrace_has_cap(struct user_namespace *ns, unsigned int mode)
> {
> + if (mode & PTRACE_MODE_SCHED)
> + return false;
> +
> if (mode & PTRACE_MODE_NOAUDIT)
> return has_ns_capability_noaudit(current, ns, CAP_SYS_PTRACE);
> else
> @@ -328,9 +331,16 @@ static int __ptrace_may_access(struct ta
>!ptrace_has_cap(mm->user_ns, mode)))
> return -EPERM;
>
> + if (mode & PTRACE_MODE_SCHED)
> + return 0;
> return security_ptrace_access_check(task, mode);
> }
>
> +bool ptrace_may_access_sched(struct task_struct *task, unsigned int mode)
> +{
> + return __ptrace_may_access(task, mode | PTRACE_MODE_SCHED);
> +}
Ha!, much nicer. Thanks!
Re: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection
On Sat, Sep 22, 2018 at 03:30:07PM +0200, Thomas Gleixner wrote:
> On Sat, 22 Sep 2018, Thomas Gleixner wrote:
> > On Sat, 22 Sep 2018, Peter Zijlstra wrote:
> > > This has some unfortunate duplication.
> > >
> > > Lets go with it for now, but I'll see if I can do something about that
> > > later.
> >
> > Yes, I know. I tried to make the duplication smaller, but all attempts
> > ended up being a convoluted mess. I'll try again after applying more
> > coffee.
>
> Lunch and coffee indeed made brain work better. The simple solution was way
> too obvious.
> --- a/kernel/ptrace.c
> +++ b/kernel/ptrace.c
> @@ -261,6 +261,9 @@ static int ptrace_check_attach(struct ta
>
> static int ptrace_has_cap(struct user_namespace *ns, unsigned int mode)
> {
> + if (mode & PTRACE_MODE_SCHED)
> + return false;
> +
> if (mode & PTRACE_MODE_NOAUDIT)
> return has_ns_capability_noaudit(current, ns, CAP_SYS_PTRACE);
> else
> @@ -328,9 +331,16 @@ static int __ptrace_may_access(struct ta
>!ptrace_has_cap(mm->user_ns, mode)))
> return -EPERM;
>
> + if (mode & PTRACE_MODE_SCHED)
> + return 0;
> return security_ptrace_access_check(task, mode);
> }
>
> +bool ptrace_may_access_sched(struct task_struct *task, unsigned int mode)
> +{
> + return __ptrace_may_access(task, mode | PTRACE_MODE_SCHED);
> +}
Ha!, much nicer. Thanks!
Re: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection
On Sat, 22 Sep 2018, Thomas Gleixner wrote:
> On Sat, 22 Sep 2018, Peter Zijlstra wrote:
> > This has some unfortunate duplication.
> >
> > Lets go with it for now, but I'll see if I can do something about that
> > later.
>
> Yes, I know. I tried to make the duplication smaller, but all attempts
> ended up being a convoluted mess. I'll try again after applying more
> coffee.
Lunch and coffee indeed made brain work better. The simple solution was way
too obvious.
Thanks,
tglx
8<
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -7,6 +7,7 @@
#include
#include
#include
+#include
#include
#include
@@ -180,6 +181,19 @@ static void sync_current_stack_to_mm(str
}
}
+static bool ibpb_needed(struct task_struct *tsk, u64 last_ctx_id)
+{
+ /*
+* Check if the current (previous) task has access to the memory
+* of the @tsk (next) task. If access is denied, make sure to
+* issue a IBPB to stop user->user Spectre-v2 attacks.
+*
+* Note: __ptrace_may_access() returns 0 or -ERRNO.
+*/
+ return (tsk && tsk->mm && tsk->mm->context.ctx_id != last_ctx_id &&
+ ptrace_may_access_sched(tsk, PTRACE_MODE_SPEC_IBPB));
+}
+
void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
struct task_struct *tsk)
{
@@ -262,18 +276,13 @@ void switch_mm_irqs_off(struct mm_struct
* one process from doing Spectre-v2 attacks on another.
*
* As an optimization, flush indirect branches only when
-* switching into processes that disable dumping. This
-* protects high value processes like gpg, without having
-* too high performance overhead. IBPB is *expensive*!
-*
-* This will not flush branches when switching into kernel
-* threads. It will also not flush if we switch to idle
-* thread and back to the same process. It will flush if we
-* switch to a different non-dumpable process.
+* switching into a processes that can't be ptrace by the
+* current one (as in such case, attacker has much more
+* convenient way how to tamper with the next process than
+* branch buffer poisoning).
*/
- if (tsk && tsk->mm &&
- tsk->mm->context.ctx_id != last_ctx_id &&
- get_dumpable(tsk->mm) != SUID_DUMP_USER)
+ if (static_cpu_has(X86_FEATURE_USE_IBPB) &&
+ ibpb_needed(tsk, last_ctx_id))
indirect_branch_prediction_barrier();
if (IS_ENABLED(CONFIG_VMAP_STACK)) {
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -62,14 +62,17 @@ extern void exit_ptrace(struct task_stru
#define PTRACE_MODE_READ 0x01
#define PTRACE_MODE_ATTACH 0x02
#define PTRACE_MODE_NOAUDIT0x04
-#define PTRACE_MODE_FSCREDS 0x08
-#define PTRACE_MODE_REALCREDS 0x10
+#define PTRACE_MODE_FSCREDS0x08
+#define PTRACE_MODE_REALCREDS 0x10
+#define PTRACE_MODE_SCHED 0x20
+#define PTRACE_MODE_IBPB 0x40
/* shorthands for READ/ATTACH and FSCREDS/REALCREDS combinations */
#define PTRACE_MODE_READ_FSCREDS (PTRACE_MODE_READ | PTRACE_MODE_FSCREDS)
#define PTRACE_MODE_READ_REALCREDS (PTRACE_MODE_READ | PTRACE_MODE_REALCREDS)
#define PTRACE_MODE_ATTACH_FSCREDS (PTRACE_MODE_ATTACH | PTRACE_MODE_FSCREDS)
#define PTRACE_MODE_ATTACH_REALCREDS (PTRACE_MODE_ATTACH |
PTRACE_MODE_REALCREDS)
+#define PTRACE_MODE_SPEC_IBPB (PTRACE_MODE_ATTACH_REALCREDS | PTRACE_MOD_IBPB)
/**
* ptrace_may_access - check whether the caller is permitted to access
@@ -87,6 +90,20 @@ extern void exit_ptrace(struct task_stru
*/
extern bool ptrace_may_access(struct task_struct *task, unsigned int mode);
+/**
+ * ptrace_may_access - check whether the caller is permitted to access
+ * a target task.
+ * @task: target task
+ * @mode: selects type of access and caller credentials
+ *
+ * Returns true on success, false on denial.
+ *
+ * Similar to ptrace_may_access(). Only to be called from context switch
+ * code. Does not call into audit and the regular LSM hooks due to locking
+ * constraints.
+ */
+extern bool ptrace_may_access_sched(struct task_struct *task, unsigned int
mode);
+
static inline int ptrace_reparented(struct task_struct *child)
{
return !same_thread_group(child->real_parent, child->parent);
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -261,6 +261,9 @@ static int ptrace_check_attach(struct ta
static int ptrace_has_cap(struct user_namespace *ns, unsigned int mode)
{
+ if (mode & PTRACE_MODE_SCHED)
+ return false;
+
if (mode & PTRACE_MODE_NOAUDIT)
return has_ns_capability_noaudit(current, ns, CAP_SYS_PTRACE);
else
@@ -328,9 +331,16 @@ static int
Re: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection
On Sat, 22 Sep 2018, Thomas Gleixner wrote:
> On Sat, 22 Sep 2018, Peter Zijlstra wrote:
> > This has some unfortunate duplication.
> >
> > Lets go with it for now, but I'll see if I can do something about that
> > later.
>
> Yes, I know. I tried to make the duplication smaller, but all attempts
> ended up being a convoluted mess. I'll try again after applying more
> coffee.
Lunch and coffee indeed made brain work better. The simple solution was way
too obvious.
Thanks,
tglx
8<
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -7,6 +7,7 @@
#include
#include
#include
+#include
#include
#include
@@ -180,6 +181,19 @@ static void sync_current_stack_to_mm(str
}
}
+static bool ibpb_needed(struct task_struct *tsk, u64 last_ctx_id)
+{
+ /*
+* Check if the current (previous) task has access to the memory
+* of the @tsk (next) task. If access is denied, make sure to
+* issue a IBPB to stop user->user Spectre-v2 attacks.
+*
+* Note: __ptrace_may_access() returns 0 or -ERRNO.
+*/
+ return (tsk && tsk->mm && tsk->mm->context.ctx_id != last_ctx_id &&
+ ptrace_may_access_sched(tsk, PTRACE_MODE_SPEC_IBPB));
+}
+
void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
struct task_struct *tsk)
{
@@ -262,18 +276,13 @@ void switch_mm_irqs_off(struct mm_struct
* one process from doing Spectre-v2 attacks on another.
*
* As an optimization, flush indirect branches only when
-* switching into processes that disable dumping. This
-* protects high value processes like gpg, without having
-* too high performance overhead. IBPB is *expensive*!
-*
-* This will not flush branches when switching into kernel
-* threads. It will also not flush if we switch to idle
-* thread and back to the same process. It will flush if we
-* switch to a different non-dumpable process.
+* switching into a processes that can't be ptrace by the
+* current one (as in such case, attacker has much more
+* convenient way how to tamper with the next process than
+* branch buffer poisoning).
*/
- if (tsk && tsk->mm &&
- tsk->mm->context.ctx_id != last_ctx_id &&
- get_dumpable(tsk->mm) != SUID_DUMP_USER)
+ if (static_cpu_has(X86_FEATURE_USE_IBPB) &&
+ ibpb_needed(tsk, last_ctx_id))
indirect_branch_prediction_barrier();
if (IS_ENABLED(CONFIG_VMAP_STACK)) {
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -62,14 +62,17 @@ extern void exit_ptrace(struct task_stru
#define PTRACE_MODE_READ 0x01
#define PTRACE_MODE_ATTACH 0x02
#define PTRACE_MODE_NOAUDIT0x04
-#define PTRACE_MODE_FSCREDS 0x08
-#define PTRACE_MODE_REALCREDS 0x10
+#define PTRACE_MODE_FSCREDS0x08
+#define PTRACE_MODE_REALCREDS 0x10
+#define PTRACE_MODE_SCHED 0x20
+#define PTRACE_MODE_IBPB 0x40
/* shorthands for READ/ATTACH and FSCREDS/REALCREDS combinations */
#define PTRACE_MODE_READ_FSCREDS (PTRACE_MODE_READ | PTRACE_MODE_FSCREDS)
#define PTRACE_MODE_READ_REALCREDS (PTRACE_MODE_READ | PTRACE_MODE_REALCREDS)
#define PTRACE_MODE_ATTACH_FSCREDS (PTRACE_MODE_ATTACH | PTRACE_MODE_FSCREDS)
#define PTRACE_MODE_ATTACH_REALCREDS (PTRACE_MODE_ATTACH |
PTRACE_MODE_REALCREDS)
+#define PTRACE_MODE_SPEC_IBPB (PTRACE_MODE_ATTACH_REALCREDS | PTRACE_MOD_IBPB)
/**
* ptrace_may_access - check whether the caller is permitted to access
@@ -87,6 +90,20 @@ extern void exit_ptrace(struct task_stru
*/
extern bool ptrace_may_access(struct task_struct *task, unsigned int mode);
+/**
+ * ptrace_may_access - check whether the caller is permitted to access
+ * a target task.
+ * @task: target task
+ * @mode: selects type of access and caller credentials
+ *
+ * Returns true on success, false on denial.
+ *
+ * Similar to ptrace_may_access(). Only to be called from context switch
+ * code. Does not call into audit and the regular LSM hooks due to locking
+ * constraints.
+ */
+extern bool ptrace_may_access_sched(struct task_struct *task, unsigned int
mode);
+
static inline int ptrace_reparented(struct task_struct *child)
{
return !same_thread_group(child->real_parent, child->parent);
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -261,6 +261,9 @@ static int ptrace_check_attach(struct ta
static int ptrace_has_cap(struct user_namespace *ns, unsigned int mode)
{
+ if (mode & PTRACE_MODE_SCHED)
+ return false;
+
if (mode & PTRACE_MODE_NOAUDIT)
return has_ns_capability_noaudit(current, ns, CAP_SYS_PTRACE);
else
@@ -328,9 +331,16 @@ static int
Re: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection
On Sat, 22 Sep 2018, Peter Zijlstra wrote:
> On Sat, Sep 22, 2018 at 11:53:14AM +0200, Thomas Gleixner wrote:
> > +bool ptrace_may_access_sched(struct task_struct *task, unsigned int mode)
> > +{
> > + struct mm_struct *mm;
> > + int res;
> > +
> > + res = __ptrace_may_access_basic(task, mode);
> > + if (res <= 0)
> > + return !res;
> > +
> > + rcu_read_lock();
> > + res = __ptrace_may_access_cred(__task_cred(task), mode);
> > rcu_read_unlock();
> > + if (res)
> > + return false;
> > +
> > + mm = task->mm;
> > + if (mm && get_dumpable(mm) != SUID_DUMP_USER)
> > + return false;
> > + return true;
> > +}
> > +
> > +/* Returns 0 on success, -errno on denial. */
> > +static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
> > +{
> > + const struct cred *tcred;
> > + struct mm_struct *mm;
> > + int res;
> > +
> > + res = __ptrace_may_access_basic(task, mode);
> > + if (res <= 0)
> > + return res;
> > +
> > + rcu_read_lock();
> > + tcred = __task_cred(task);
> > + res = __ptrace_may_access_cred(tcred, mode);
> > + if (res > 0)
> > + res = ptrace_has_cap(tcred->user_ns, mode) ? 0 : -EPERM;
> > rcu_read_unlock();
> > + if (res < 0)
> > + return res;
> > +
> > mm = task->mm;
> > + if (mm && (get_dumpable(mm) != SUID_DUMP_USER &&
> > + !ptrace_has_cap(mm->user_ns, mode)))
> > + return -EPERM;
> >
> > return security_ptrace_access_check(task, mode);
> > }
>
> This has some unfortunate duplication.
>
> Lets go with it for now, but I'll see if I can do something about that
> later.
Yes, I know. I tried to make the duplication smaller, but all attempts
ended up being a convoluted mess. I'll try again after applying more
coffee.
Thanks,
tglx
Re: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection
On Sat, 22 Sep 2018, Peter Zijlstra wrote:
> On Sat, Sep 22, 2018 at 11:53:14AM +0200, Thomas Gleixner wrote:
> > +bool ptrace_may_access_sched(struct task_struct *task, unsigned int mode)
> > +{
> > + struct mm_struct *mm;
> > + int res;
> > +
> > + res = __ptrace_may_access_basic(task, mode);
> > + if (res <= 0)
> > + return !res;
> > +
> > + rcu_read_lock();
> > + res = __ptrace_may_access_cred(__task_cred(task), mode);
> > rcu_read_unlock();
> > + if (res)
> > + return false;
> > +
> > + mm = task->mm;
> > + if (mm && get_dumpable(mm) != SUID_DUMP_USER)
> > + return false;
> > + return true;
> > +}
> > +
> > +/* Returns 0 on success, -errno on denial. */
> > +static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
> > +{
> > + const struct cred *tcred;
> > + struct mm_struct *mm;
> > + int res;
> > +
> > + res = __ptrace_may_access_basic(task, mode);
> > + if (res <= 0)
> > + return res;
> > +
> > + rcu_read_lock();
> > + tcred = __task_cred(task);
> > + res = __ptrace_may_access_cred(tcred, mode);
> > + if (res > 0)
> > + res = ptrace_has_cap(tcred->user_ns, mode) ? 0 : -EPERM;
> > rcu_read_unlock();
> > + if (res < 0)
> > + return res;
> > +
> > mm = task->mm;
> > + if (mm && (get_dumpable(mm) != SUID_DUMP_USER &&
> > + !ptrace_has_cap(mm->user_ns, mode)))
> > + return -EPERM;
> >
> > return security_ptrace_access_check(task, mode);
> > }
>
> This has some unfortunate duplication.
>
> Lets go with it for now, but I'll see if I can do something about that
> later.
Yes, I know. I tried to make the duplication smaller, but all attempts
ended up being a convoluted mess. I'll try again after applying more
coffee.
Thanks,
tglx
Re: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection
On Sat, Sep 22, 2018 at 11:53:14AM +0200, Thomas Gleixner wrote:
> @@ -86,6 +88,7 @@ extern void exit_ptrace(struct task_stru
> * process_vm_writev or ptrace (and should use the real credentials).
> */
> extern bool ptrace_may_access(struct task_struct *task, unsigned int mode);
> +extern bool ptrace_may_access_sched(struct task_struct *task, unsigned int
> mode);
I like that..
> static inline int ptrace_reparented(struct task_struct *child)
> {
> --- a/kernel/ptrace.c
> +++ b/kernel/ptrace.c
> +bool ptrace_may_access_sched(struct task_struct *task, unsigned int mode)
> +{
> + struct mm_struct *mm;
> + int res;
> +
> + res = __ptrace_may_access_basic(task, mode);
> + if (res <= 0)
> + return !res;
> +
> + rcu_read_lock();
> + res = __ptrace_may_access_cred(__task_cred(task), mode);
> rcu_read_unlock();
> + if (res)
> + return false;
> +
> + mm = task->mm;
> + if (mm && get_dumpable(mm) != SUID_DUMP_USER)
> + return false;
> + return true;
> +}
> +
> +/* Returns 0 on success, -errno on denial. */
> +static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
> +{
> + const struct cred *tcred;
> + struct mm_struct *mm;
> + int res;
> +
> + res = __ptrace_may_access_basic(task, mode);
> + if (res <= 0)
> + return res;
> +
> + rcu_read_lock();
> + tcred = __task_cred(task);
> + res = __ptrace_may_access_cred(tcred, mode);
> + if (res > 0)
> + res = ptrace_has_cap(tcred->user_ns, mode) ? 0 : -EPERM;
> rcu_read_unlock();
> + if (res < 0)
> + return res;
> +
> mm = task->mm;
> + if (mm && (get_dumpable(mm) != SUID_DUMP_USER &&
> +!ptrace_has_cap(mm->user_ns, mode)))
> + return -EPERM;
>
> return security_ptrace_access_check(task, mode);
> }
This has some unfortunate duplication.
Lets go with it for now, but I'll see if I can do something about that
later.
Re: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection
On Sat, Sep 22, 2018 at 11:53:14AM +0200, Thomas Gleixner wrote:
> @@ -86,6 +88,7 @@ extern void exit_ptrace(struct task_stru
> * process_vm_writev or ptrace (and should use the real credentials).
> */
> extern bool ptrace_may_access(struct task_struct *task, unsigned int mode);
> +extern bool ptrace_may_access_sched(struct task_struct *task, unsigned int
> mode);
I like that..
> static inline int ptrace_reparented(struct task_struct *child)
> {
> --- a/kernel/ptrace.c
> +++ b/kernel/ptrace.c
> +bool ptrace_may_access_sched(struct task_struct *task, unsigned int mode)
> +{
> + struct mm_struct *mm;
> + int res;
> +
> + res = __ptrace_may_access_basic(task, mode);
> + if (res <= 0)
> + return !res;
> +
> + rcu_read_lock();
> + res = __ptrace_may_access_cred(__task_cred(task), mode);
> rcu_read_unlock();
> + if (res)
> + return false;
> +
> + mm = task->mm;
> + if (mm && get_dumpable(mm) != SUID_DUMP_USER)
> + return false;
> + return true;
> +}
> +
> +/* Returns 0 on success, -errno on denial. */
> +static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
> +{
> + const struct cred *tcred;
> + struct mm_struct *mm;
> + int res;
> +
> + res = __ptrace_may_access_basic(task, mode);
> + if (res <= 0)
> + return res;
> +
> + rcu_read_lock();
> + tcred = __task_cred(task);
> + res = __ptrace_may_access_cred(tcred, mode);
> + if (res > 0)
> + res = ptrace_has_cap(tcred->user_ns, mode) ? 0 : -EPERM;
> rcu_read_unlock();
> + if (res < 0)
> + return res;
> +
> mm = task->mm;
> + if (mm && (get_dumpable(mm) != SUID_DUMP_USER &&
> +!ptrace_has_cap(mm->user_ns, mode)))
> + return -EPERM;
>
> return security_ptrace_access_check(task, mode);
> }
This has some unfortunate duplication.
Lets go with it for now, but I'll see if I can do something about that
later.
Re: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection
On Sat, 22 Sep 2018, Jiri Kosina wrote:
> On Wed, 19 Sep 2018, Peter Zijlstra wrote:
> > As far as I can tell, this still has:
> >
> > avc_has_perm_noaudit()
> > security_compute_av()
> > read_lock(>ss->policy_rwlock);
> > avc_insert()
> > spin_lock_irqsave();
> > avc_denied()
> > avc_update_node()
> > spin_lock_irqsave();
> >
> > under the scheduler's raw_spinlock_t, which are invalid lock nestings.
>
> Agreed. Therefore, if the current form (v6) of the patches is merged, the
> check before security_ptrace_access_check() should stay.
>
> Once all the LSM callbacks are potentially audited, it could then go in
> second phase.
>
> Is there anything else blocking v6 being merged? (and then Tim's set on
> top I guess, once the details are sorted out there).
I was waiting for this to resolve. Now, looking at the outcome of this, I
think, that calling into security hooks needs very special care from that
context.
So we better split that up right away instead of adding this magic
PTRACE_MODE_NOACCESS_CHK bit. This split up will be needed for adding the
special LSM speculation control hook anyway because adding yet more magic
bits and conditionals makes this code completely undigestable. It's
convoluted enough already.
Something along the compiled but completely untested patch below should do
the trick. If that is what we want this needs to be split up of course.
Thanks,
tglx
8<--
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -7,6 +7,7 @@
#include
#include
#include
+#include
#include
#include
@@ -180,6 +181,19 @@ static void sync_current_stack_to_mm(str
}
}
+static bool ibpb_needed(struct task_struct *tsk, u64 last_ctx_id)
+{
+ /*
+* Check if the current (previous) task has access to the memory
+* of the @tsk (next) task. If access is denied, make sure to
+* issue a IBPB to stop user->user Spectre-v2 attacks.
+*
+* Note: __ptrace_may_access() returns 0 or -ERRNO.
+*/
+ return (tsk && tsk->mm && tsk->mm->context.ctx_id != last_ctx_id &&
+ ptrace_may_access_sched(tsk, PTRACE_MODE_SPEC_IBPB));
+}
+
void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
struct task_struct *tsk)
{
@@ -262,18 +276,13 @@ void switch_mm_irqs_off(struct mm_struct
* one process from doing Spectre-v2 attacks on another.
*
* As an optimization, flush indirect branches only when
-* switching into processes that disable dumping. This
-* protects high value processes like gpg, without having
-* too high performance overhead. IBPB is *expensive*!
-*
-* This will not flush branches when switching into kernel
-* threads. It will also not flush if we switch to idle
-* thread and back to the same process. It will flush if we
-* switch to a different non-dumpable process.
+* switching into a processes that can't be ptrace by the
+* current one (as in such case, attacker has much more
+* convenient way how to tamper with the next process than
+* branch buffer poisoning).
*/
- if (tsk && tsk->mm &&
- tsk->mm->context.ctx_id != last_ctx_id &&
- get_dumpable(tsk->mm) != SUID_DUMP_USER)
+ if (static_cpu_has(X86_FEATURE_USE_IBPB) &&
+ ibpb_needed(tsk, last_ctx_id))
indirect_branch_prediction_barrier();
if (IS_ENABLED(CONFIG_VMAP_STACK)) {
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -62,14 +62,16 @@ extern void exit_ptrace(struct task_stru
#define PTRACE_MODE_READ 0x01
#define PTRACE_MODE_ATTACH 0x02
#define PTRACE_MODE_NOAUDIT0x04
-#define PTRACE_MODE_FSCREDS 0x08
-#define PTRACE_MODE_REALCREDS 0x10
+#define PTRACE_MODE_FSCREDS0x08
+#define PTRACE_MODE_REALCREDS 0x10
+#define PTRACE_MODE_IBPB 0x20
/* shorthands for READ/ATTACH and FSCREDS/REALCREDS combinations */
#define PTRACE_MODE_READ_FSCREDS (PTRACE_MODE_READ | PTRACE_MODE_FSCREDS)
#define PTRACE_MODE_READ_REALCREDS (PTRACE_MODE_READ | PTRACE_MODE_REALCREDS)
#define PTRACE_MODE_ATTACH_FSCREDS (PTRACE_MODE_ATTACH | PTRACE_MODE_FSCREDS)
#define PTRACE_MODE_ATTACH_REALCREDS (PTRACE_MODE_ATTACH |
PTRACE_MODE_REALCREDS)
+#define PTRACE_MODE_SPEC_IBPB (PTRACE_MODE_ATTACH_REALCREDS | PTRACE_MODE_IBPB)
/**
* ptrace_may_access - check whether the caller is permitted to access
@@ -86,6 +88,7 @@ extern void exit_ptrace(struct task_stru
* process_vm_writev or ptrace (and should use the real credentials).
*/
extern bool ptrace_may_access(struct task_struct *task, unsigned int mode);
+extern bool
Re: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection
On Sat, 22 Sep 2018, Jiri Kosina wrote:
> On Wed, 19 Sep 2018, Peter Zijlstra wrote:
> > As far as I can tell, this still has:
> >
> > avc_has_perm_noaudit()
> > security_compute_av()
> > read_lock(>ss->policy_rwlock);
> > avc_insert()
> > spin_lock_irqsave();
> > avc_denied()
> > avc_update_node()
> > spin_lock_irqsave();
> >
> > under the scheduler's raw_spinlock_t, which are invalid lock nestings.
>
> Agreed. Therefore, if the current form (v6) of the patches is merged, the
> check before security_ptrace_access_check() should stay.
>
> Once all the LSM callbacks are potentially audited, it could then go in
> second phase.
>
> Is there anything else blocking v6 being merged? (and then Tim's set on
> top I guess, once the details are sorted out there).
I was waiting for this to resolve. Now, looking at the outcome of this, I
think, that calling into security hooks needs very special care from that
context.
So we better split that up right away instead of adding this magic
PTRACE_MODE_NOACCESS_CHK bit. This split up will be needed for adding the
special LSM speculation control hook anyway because adding yet more magic
bits and conditionals makes this code completely undigestable. It's
convoluted enough already.
Something along the compiled but completely untested patch below should do
the trick. If that is what we want this needs to be split up of course.
Thanks,
tglx
8<--
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -7,6 +7,7 @@
#include
#include
#include
+#include
#include
#include
@@ -180,6 +181,19 @@ static void sync_current_stack_to_mm(str
}
}
+static bool ibpb_needed(struct task_struct *tsk, u64 last_ctx_id)
+{
+ /*
+* Check if the current (previous) task has access to the memory
+* of the @tsk (next) task. If access is denied, make sure to
+* issue a IBPB to stop user->user Spectre-v2 attacks.
+*
+* Note: __ptrace_may_access() returns 0 or -ERRNO.
+*/
+ return (tsk && tsk->mm && tsk->mm->context.ctx_id != last_ctx_id &&
+ ptrace_may_access_sched(tsk, PTRACE_MODE_SPEC_IBPB));
+}
+
void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
struct task_struct *tsk)
{
@@ -262,18 +276,13 @@ void switch_mm_irqs_off(struct mm_struct
* one process from doing Spectre-v2 attacks on another.
*
* As an optimization, flush indirect branches only when
-* switching into processes that disable dumping. This
-* protects high value processes like gpg, without having
-* too high performance overhead. IBPB is *expensive*!
-*
-* This will not flush branches when switching into kernel
-* threads. It will also not flush if we switch to idle
-* thread and back to the same process. It will flush if we
-* switch to a different non-dumpable process.
+* switching into a processes that can't be ptrace by the
+* current one (as in such case, attacker has much more
+* convenient way how to tamper with the next process than
+* branch buffer poisoning).
*/
- if (tsk && tsk->mm &&
- tsk->mm->context.ctx_id != last_ctx_id &&
- get_dumpable(tsk->mm) != SUID_DUMP_USER)
+ if (static_cpu_has(X86_FEATURE_USE_IBPB) &&
+ ibpb_needed(tsk, last_ctx_id))
indirect_branch_prediction_barrier();
if (IS_ENABLED(CONFIG_VMAP_STACK)) {
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -62,14 +62,16 @@ extern void exit_ptrace(struct task_stru
#define PTRACE_MODE_READ 0x01
#define PTRACE_MODE_ATTACH 0x02
#define PTRACE_MODE_NOAUDIT0x04
-#define PTRACE_MODE_FSCREDS 0x08
-#define PTRACE_MODE_REALCREDS 0x10
+#define PTRACE_MODE_FSCREDS0x08
+#define PTRACE_MODE_REALCREDS 0x10
+#define PTRACE_MODE_IBPB 0x20
/* shorthands for READ/ATTACH and FSCREDS/REALCREDS combinations */
#define PTRACE_MODE_READ_FSCREDS (PTRACE_MODE_READ | PTRACE_MODE_FSCREDS)
#define PTRACE_MODE_READ_REALCREDS (PTRACE_MODE_READ | PTRACE_MODE_REALCREDS)
#define PTRACE_MODE_ATTACH_FSCREDS (PTRACE_MODE_ATTACH | PTRACE_MODE_FSCREDS)
#define PTRACE_MODE_ATTACH_REALCREDS (PTRACE_MODE_ATTACH |
PTRACE_MODE_REALCREDS)
+#define PTRACE_MODE_SPEC_IBPB (PTRACE_MODE_ATTACH_REALCREDS | PTRACE_MODE_IBPB)
/**
* ptrace_may_access - check whether the caller is permitted to access
@@ -86,6 +88,7 @@ extern void exit_ptrace(struct task_stru
* process_vm_writev or ptrace (and should use the real credentials).
*/
extern bool ptrace_may_access(struct task_struct *task, unsigned int mode);
+extern bool
Re: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection
On Wed, 19 Sep 2018, Peter Zijlstra wrote:
> > 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(_state, sid, csid,
> > + SECCLASS_PROCESS,
> > PROCESS__PTRACE,
> > + 0, );
> > if (mode & PTRACE_MODE_READ)
> > return avc_has_perm(_state,
> > sid, csid, SECCLASS_FILE, FILE__READ,
> > NULL);
> >
>
> As far as I can tell, this still has:
>
> avc_has_perm_noaudit()
> security_compute_av()
> read_lock(>ss->policy_rwlock);
> avc_insert()
> spin_lock_irqsave();
> avc_denied()
> avc_update_node()
> spin_lock_irqsave();
>
> under the scheduler's raw_spinlock_t, which are invalid lock nestings.
Agreed. Therefore, if the current form (v6) of the patches is merged, the
check before security_ptrace_access_check() should stay.
Once all the LSM callbacks are potentially audited, it could then go in
second phase.
Is there anything else blocking v6 being merged? (and then Tim's set on
top I guess, once the details are sorted out there).
Thanks,
--
Jiri Kosina
SUSE Labs
Re: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection
On Wed, 19 Sep 2018, Peter Zijlstra wrote:
> > 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(_state, sid, csid,
> > + SECCLASS_PROCESS,
> > PROCESS__PTRACE,
> > + 0, );
> > if (mode & PTRACE_MODE_READ)
> > return avc_has_perm(_state,
> > sid, csid, SECCLASS_FILE, FILE__READ,
> > NULL);
> >
>
> As far as I can tell, this still has:
>
> avc_has_perm_noaudit()
> security_compute_av()
> read_lock(>ss->policy_rwlock);
> avc_insert()
> spin_lock_irqsave();
> avc_denied()
> avc_update_node()
> spin_lock_irqsave();
>
> under the scheduler's raw_spinlock_t, which are invalid lock nestings.
Agreed. Therefore, if the current form (v6) of the patches is merged, the
check before security_ptrace_access_check() should stay.
Once all the LSM callbacks are potentially audited, it could then go in
second phase.
Is there anything else blocking v6 being merged? (and then Tim's set on
top I guess, once the details are sorted out there).
Thanks,
--
Jiri Kosina
SUSE Labs
Re: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection
On Mon, Sep 17, 2018 at 04:09:33PM +, Schaufler, Casey wrote:
> 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(_state, sid, csid,
> + SECCLASS_PROCESS, PROCESS__PTRACE,
> + 0, );
> if (mode & PTRACE_MODE_READ)
> return avc_has_perm(_state,
> sid, csid, SECCLASS_FILE, FILE__READ,
> NULL);
>
As far as I can tell, this still has:
avc_has_perm_noaudit()
security_compute_av()
read_lock(>ss->policy_rwlock);
avc_insert()
spin_lock_irqsave();
avc_denied()
avc_update_node()
spin_lock_irqsave();
under the scheduler's raw_spinlock_t, which are invalid lock nestings.
Re: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection
On Mon, Sep 17, 2018 at 04:09:33PM +, Schaufler, Casey wrote:
> 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(_state, sid, csid,
> + SECCLASS_PROCESS, PROCESS__PTRACE,
> + 0, );
> if (mode & PTRACE_MODE_READ)
> return avc_has_perm(_state,
> sid, csid, SECCLASS_FILE, FILE__READ,
> NULL);
>
As far as I can tell, this still has:
avc_has_perm_noaudit()
security_compute_av()
read_lock(>ss->policy_rwlock);
avc_insert()
spin_lock_irqsave();
avc_denied()
avc_update_node()
spin_lock_irqsave();
under the scheduler's raw_spinlock_t, which are invalid lock nestings.
RE: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection
> -Original Message-
> From: Jiri Kosina [mailto:ji...@kernel.org]
> Sent: Wednesday, September 12, 2018 2:05 AM
> To: Thomas Gleixner ; Ingo Molnar ;
> Peter Zijlstra ; Josh Poimboeuf
> ; Andrea Arcangeli ;
> Woodhouse, David ; Andi Kleen ;
> Tim Chen ; Schaufler, Casey
>
> Cc: linux-kernel@vger.kernel.org; x...@kernel.org
> 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
---
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(_state, sid, csid,
+ SECCLASS_PROCESS, PROCESS__PTRACE,
+ 0, );
if (mode & PTRACE_MODE_READ)
return avc_has_perm(_state,
sid, csid, SECCLASS_FILE, FILE__READ, NULL);
casey-jiri-v6.patch
Description: casey-jiri-v6.patch
RE: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection
> -Original Message-
> From: Jiri Kosina [mailto:ji...@kernel.org]
> Sent: Wednesday, September 12, 2018 2:05 AM
> To: Thomas Gleixner ; Ingo Molnar ;
> Peter Zijlstra ; Josh Poimboeuf
> ; Andrea Arcangeli ;
> Woodhouse, David ; Andi Kleen ;
> Tim Chen ; Schaufler, Casey
>
> Cc: linux-kernel@vger.kernel.org; x...@kernel.org
> 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
---
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(_state, sid, csid,
+ SECCLASS_PROCESS, PROCESS__PTRACE,
+ 0, );
if (mode & PTRACE_MODE_READ)
return avc_has_perm(_state,
sid, csid, SECCLASS_FILE, FILE__READ, NULL);
casey-jiri-v6.patch
Description: casey-jiri-v6.patch
[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
[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
