All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Stephen Smalley <sds@tycho.nsa.gov>
Cc: Jiri Kosina <jikos@kernel.org>, Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Andrea Arcangeli <aarcange@redhat.com>,
	"Woodhouse, David" <dwmw@amazon.co.uk>,
	Andi Kleen <ak@linux.intel.com>,
	Tim Chen <tim.c.chen@linux.intel.com>,
	"Schaufler, Casey" <casey.schaufler@intel.com>,
	linux-kernel@vger.kernel.org, x86@kernel.org
Subject: Re: [PATCH v7 1/3] x86/speculation: apply IBPB more strictly to avoid cross-process data leak
Date: Thu, 27 Sep 2018 22:28:03 +0200 (CEST)	[thread overview]
Message-ID: <alpine.DEB.2.21.1809272218190.8118@nanos.tec.linutronix.de> (raw)
In-Reply-To: <7f9e1a22-37a8-db88-ffc0-91961174ced4@tycho.nsa.gov>

On Thu, 27 Sep 2018, Stephen Smalley wrote:
> On 09/25/2018 08:38 AM, Jiri Kosina wrote:
> >   +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));
> 
> Would there be any safe way to perform the ptrace check earlier at a point
> where the locking constraints are less severe, and just pass down the result
> to this code?  Possibly just defaulting to enabling IBPB for safety if
> something changed in the interim that would invalidate the earlier ptrace
> check?  Probably not possible, but I thought I'd ask as it would avoid the
> need to skip both the ptrace_has_cap check and the LSM hook, and would reduce
> the critical section.

It's not possible unfortunately as this happens under the scheduler run
queue lock and this needs to be taken to figure out which is the next
task. We can't drop it before context switch and revisit the decision
afterwards to verify it, that would be a massive performance issue and open
an even more horrible can of worms.

Any check which needs to be done in that context should be as minimalistic
as possible. So having a special mode which then might invoke special hooks
makes a lot of sense.

> > + * 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.
> 
> Pardon my ignorance, but can you clarify exactly what are the locking
> constraints for any code that might be called now or in the future from
> ptrace_may_access_sched().  What's permissible?  rcu_read_lock()?

rcu_read_lock() is fine. Locks might be fine, but the probability that you
run into a lock inversion is extremly high. Also please keep in mind that
this wants to be a raw_spinlock as otherwise preempt-RT will have issues
and the lock sections need to be really short. switch_to() is a hot path.

Thanks,

	tglx

  reply	other threads:[~2018-09-27 20:28 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-25 12:37 [PATCH v7 0/3] Harden spectrev2 userspace-userspace protection Jiri Kosina
2018-09-25 12:38 ` [PATCH v7 1/3] x86/speculation: apply IBPB more strictly to avoid cross-process data leak Jiri Kosina
2018-09-26 12:30   ` [tip:x86/pti] x86/speculation: Apply " tip-bot for Jiri Kosina
2018-09-27 20:18   ` [PATCH v7 1/3] x86/speculation: apply " Stephen Smalley
2018-09-27 20:28     ` Thomas Gleixner [this message]
2018-09-25 12:38 ` [PATCH v7 2/3] x86/speculation: Enable cross-hyperthread spectre v2 STIBP mitigation Jiri Kosina
2018-09-26 12:31   ` [tip:x86/pti] " tip-bot for Jiri Kosina
2018-09-25 12:39 ` [PATCH v7 3/3] x86/speculation: Propagate information about RSB filling mitigation to sysfs Jiri Kosina
2018-09-26 12:31   ` [tip:x86/pti] " tip-bot for Jiri Kosina

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.21.1809272218190.8118@nanos.tec.linutronix.de \
    --to=tglx@linutronix.de \
    --cc=aarcange@redhat.com \
    --cc=ak@linux.intel.com \
    --cc=casey.schaufler@intel.com \
    --cc=dwmw@amazon.co.uk \
    --cc=jikos@kernel.org \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=sds@tycho.nsa.gov \
    --cc=tim.c.chen@linux.intel.com \
    --cc=x86@kernel.org \
    --subject='Re: [PATCH v7 1/3] x86/speculation: apply IBPB more strictly to avoid cross-process data leak' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.