Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 10 Sep 2018 22:42:21 +0200 (CEST)
From:   Jiri Kosina <jikos@kernel.org>
To:     "Schaufler, Casey" <casey.schaufler@intel.com>
cc:     Thomas Gleixner <tglx@linutronix.de>,
        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>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "x86@kernel.org" <x86@kernel.org>
Subject: RE: [PATCH v5 1/2] x86/speculation: apply IBPB more strictly to
 avoid cross-process data leak
In-Reply-To: <99FC4B6EFCEFD44486C35F4C281DC6732144AF24@ORSMSX107.amr.corp.intel.com>
Message-ID: <nycvar.YFH.7.76.1809102235010.15880@cbobk.fhfr.pm>
References: <nycvar.YFH.7.76.1809101119280.15880@cbobk.fhfr.pm> <nycvar.YFH.7.76.1809101123020.15880@cbobk.fhfr.pm> <99FC4B6EFCEFD44486C35F4C281DC6732144ADF9@ORSMSX107.amr.corp.intel.com> <nycvar.YFH.7.76.1809102111410.15880@cbobk.fhfr.pm>
 <99FC4B6EFCEFD44486C35F4C281DC6732144AEC9@ORSMSX107.amr.corp.intel.com> <nycvar.YFH.7.76.1809102132480.15880@cbobk.fhfr.pm> <99FC4B6EFCEFD44486C35F4C281DC6732144AF24@ORSMSX107.amr.corp.intel.com>
User-Agent: Alpine 2.21 (LSU 202 2017-01-01)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, 10 Sep 2018, Schaufler, Casey wrote:

> It you're going to call __ptrace_access_check(), 

I guess you mean __ptrace_may_access() here.

> which already includes an LSM hook, it makes a whole lot of sense to 
> make that the path for doing any module specific checks. It seems wrong 
> to disable the LSM hook there, then turn around and introduce a new one 
> that does the check you just disabled. The patches I had proposed 
> created a new LSM hook because there was not path to an existing hook. 
> With your addition of __ptrace_access_check() that is no longer an issue 
> once any locking problems are resolved. Rather than use a new hook, the 
> existing ptrace hooks ought to work just fine, and any new checks can be 
> added in a new module that has its own ptrace_access_check hook.

Sorry for being dense, but what exactly are you proposing here then?

This patch (v4 and v5) explicitly avoids calling out to ptrace_has_cap() 
(which calls out to LSM through has_ns_capability_*() -> 
security_capable()) in PTRACE_MODE_IBPB case, exactly to avoid locking 
issues with security modules; there are known callchains that lead to 
deadlock.

With the same reasoning, security_ptrace_access_check() call is avoided, 
only there is no know particular callchain that'd lead to a lock being 
taken, but noone has done such audit (yet), as it's all hidden behind LSM 
callbacks.

So please tell me what exactly you'd like to see changed in the IBPB patch 
and why exactly, I am not seeing it yet.

Thanks,

-- 
Jiri Kosina
SUSE Labs