From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754358AbdHUOIc (ORCPT ); Mon, 21 Aug 2017 10:08:32 -0400 Received: from bombadil.infradead.org ([65.50.211.133]:32990 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754339AbdHUOI1 (ORCPT ); Mon, 21 Aug 2017 10:08:27 -0400 Date: Mon, 21 Aug 2017 16:08:13 +0200 From: Peter Zijlstra To: Andy Lutomirski Cc: Andy Lutomirski , Will Deacon , Mark Rutland , Matt Fleming , Ard Biesheuvel , Sai Praneeth Prakhya , "linux-efi@vger.kernel.org" , "linux-kernel@vger.kernel.org" , joeyli , Borislav Petkov , "Michael S. Tsirkin" , "Neri, Ricardo" , "Ravi V. Shankar" Subject: Re: [PATCH 3/3] x86/efi: Use efi_switch_mm() rather than manually twiddling with cr3 Message-ID: <20170821140813.idloyrk4lowann3j@hirez.programming.kicks-ass.net> References: <20170816095338.GB17270@leverpostej> <20170816100709.GG12845@arm.com> <20170816110321.GC17270@leverpostej> <20170816125715.GB3384@codeblueprint.co.uk> <20170815223541.GA25778@remoulade> <20170817103514.GC27872@arm.com> <20170821103359.jt2xf2cx5wxjldau@hirez.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170609 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Aug 21, 2017 at 06:56:01AM -0700, Andy Lutomirski wrote: > > > > On Aug 21, 2017, at 3:33 AM, Peter Zijlstra wrote: > >> > >> Using a kernel thread solves the problem for real. Anything that > >> blindly accesses user memory in kernel thread context is terminally > >> broken no matter what. > > > > So perf-callchain doesn't do it 'blindly', it wants either: > > > > - user_mode(regs) true, or > > - task_pt_regs() set. > > > > However I'm thinking that if the kernel thread has ->mm == &efi_mm, the > > EFI code running could very well have user_mode(regs) being true. > > > > intel_pmu_pebs_fixup() OTOH 'blindly' assumes that the LBR addresses are > > accessible. It bails on error though. So while its careful, it does > > attempt to access the 'user' mapping directly. Which should also trigger > > with the EFI code. > > > > And I'm not seeing anything particularly broken with either. The PEBS > > fixup relies on the CPU having just executed the code, and if it could > > fetch and execute the code, why shouldn't it be able to fetch and read? > > There are two ways this could be a problem. One is that u privileged > user apps shouldn't be able to read from EFI memory. Ah, but only root can create per-cpu events or attach events to kernel threads (with sensible paranoia levels). > The other is that, if EFI were to have IO memory mapped at a "user" > address, perf could end up reading it. Ah, but in neither mode does perf assume much, the LBR follows branches the CPU took and thus we _know_ there was code there, not MMIO. And the stack unwind simply follows the stack up, although I suppose it could be 'tricked' into probing MMIO. We can certainly add an "->mm != ->active_mm" escape clause to the unwind code. Although I don't see how we're currently avoiding the same problem with existing userspace unwinds, userspace can equally have MMIO mapped. But neither will use pre-existing user addresses in the efi_mm I think.