linux-kernel - Re: [PATCH v6 3/6] task_isolation: support PR_TASK_ISOLATION

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150902101347.GF25720@arm.com>
Date:	Wed, 2 Sep 2015 11:13:47 +0100
From:	Will Deacon <will.deacon@....com>
To:	Chris Metcalf <cmetcalf@...hip.com>
Cc:	Gilad Ben Yossef <giladb@...hip.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Ingo Molnar <mingo@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Rik van Riel <riel@...hat.com>, Tejun Heo <tj@...nel.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Christoph Lameter <cl@...ux.com>,
	Viresh Kumar <viresh.kumar@...aro.org>,
	Catalin Marinas <Catalin.Marinas@....com>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	"linux-api@...r.kernel.org" <linux-api@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v6 3/6] task_isolation: support PR_TASK_ISOLATION_STRICT
 mode

On Wed, Aug 26, 2015 at 04:10:34PM +0100, Chris Metcalf wrote:
> On 08/26/2015 06:36 AM, Will Deacon wrote:
> > On Tue, Aug 25, 2015 at 08:55:52PM +0100, Chris Metcalf wrote:
> >> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> >> index d882b833dbdb..e3d83a12f3cf 100644
> >> --- a/arch/arm64/kernel/ptrace.c
> >> +++ b/arch/arm64/kernel/ptrace.c
> >> @@ -37,6 +37,7 @@
> >>   #include <linux/regset.h>
> >>   #include <linux/tracehook.h>
> >>   #include <linux/elf.h>
> >> +#include <linux/isolation.h>
> >>   
> >>   #include <asm/compat.h>
> >>   #include <asm/debug-monitors.h>
> >> @@ -1150,6 +1151,10 @@ static void tracehook_report_syscall(struct pt_regs *regs,
> >>   
> >>   asmlinkage int syscall_trace_enter(struct pt_regs *regs)
> >>   {
> >> +	/* Ensure we report task_isolation violations in all circumstances. */
> >> +	if (test_thread_flag(TIF_NOHZ) && task_isolation_strict())
> > This is going to force us to check TIF_NOHZ on the syscall slowpath even
> > when CONFIG_TASK_ISOLATION=n.
> 
> Yes, good catch.  I was thinking the "&& false" would suppress the TIF
> test but I forgot that test_bit() takes a volatile argument, so it gets
> evaluated even though the result isn't actually used.
> 
> But I don't want to just reorder the two tests, because when isolation
> is enabled, testing TIF_NOHZ first is better.  I think probably the right
> solution is just to put an #ifdef CONFIG_TASK_ISOLATION around that
> test, even though that is a little crufty.  The alternative is to provide
> a task_isolation_configured() macro that just returns true or false, and
> make it a three-part "&&" test with that new macro first, but
> that seems a little crufty as well.  Do you have a preference?

Maybe use IS_ENABLED(CONFIG_TASK_ISOLATION) ?

> >> +		task_isolation_syscall(regs->syscallno);
> >> +
> >>   	/* Do the secure computing check first; failures should be fast. */
> > Here we have the usual priority problems with all the subsystems that
> > hook into the syscall path. If a prctl is later rewritten to a different
> > syscall, do you care about catching it? Either way, the comment about
> > doing secure computing "first" needs fixing.
> 
> I admit I am unclear on the utility of rewriting prctl.  My instinct is that
> we are trying to catch userspace invocations of prctl and allow them,
> and fail most everything else, so doing it pre-rewrite seems OK.
> 
> I'm not sure if it makes sense to catch it before or after the
> secure computing check, though.  On reflection maybe doing it
> afterwards makes more sense - what do you think?

I don't have a strong preference (I really hate all these hooks we have
on the syscall entry/exit path), but we do need to make sure that the
behaviour is consistent across architectures.

Will
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/