From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.1 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5ADA3C169C4 for ; Mon, 11 Feb 2019 22:48:02 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 1F32A2184E for ; Mon, 11 Feb 2019 22:48:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1549925282; bh=rV440hGDSZ3wXtwBOPW52PZyuO31p4qEHmRDqv7v/C0=; h=References:In-Reply-To:From:Date:Subject:To:Cc:List-ID:From; b=n5rYuYKXMpf/KPtpIkE9yYi39zA8pnFP8Ud8gwCnqNnp1bkazG+Y/G6v7XTx3gEI/ 77C/g0vuo5RoXciQ4H76kzKfPG1W8IzrhwkJJqjgOEcr7KIaM/WPkYqzmzRj78jKMG 8kkeTwG+GhRoFnUFnaOp9hiByheMZwH6x/SFqefE= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727605AbfBKWr4 (ORCPT ); Mon, 11 Feb 2019 17:47:56 -0500 Received: from mail.kernel.org ([198.145.29.99]:36026 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727556AbfBKWr4 (ORCPT ); Mon, 11 Feb 2019 17:47:56 -0500 Received: from mail-wm1-f52.google.com (mail-wm1-f52.google.com [209.85.128.52]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 796B1222A0 for ; Mon, 11 Feb 2019 22:47:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1549925274; bh=rV440hGDSZ3wXtwBOPW52PZyuO31p4qEHmRDqv7v/C0=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=2qTB9vzRMdPY0stzJ7/VKb/Y1Z1BstTZsqOtQo7ivCGfDJ8FmzkIxv43/rrWP4x3F yste5X5BVJeQIUceUmQ9K+mMQJ+St7Ye69qYVRSCNvV1Mc1QlgRp/BxHOMZ/iDkSXc xCzKOmorFsxK6wArhlpHajpEWC36AB/b12KduuZk= Received: by mail-wm1-f52.google.com with SMTP id j125so974553wmj.1 for ; Mon, 11 Feb 2019 14:47:54 -0800 (PST) X-Gm-Message-State: AHQUAua1jgm7Z8EyhY/l4JULFoHv6ZZrOH/VJilKYYM9+YxglaLY2s1/ gK0dyYnmTutKpC8AnSuUqH0wm940SadqA4tdR+EzQw== X-Google-Smtp-Source: AHgI3IYhqPQS6XNR2rbuUzIn1bPg4Vb52Sam8S3ebJby84Lwzx4pkjy45X+4hrjhxv9sDdjl1Ar3ML3HVCnAR1dksqs= X-Received: by 2002:a7b:cc13:: with SMTP id f19mr353124wmh.83.1549925272767; Mon, 11 Feb 2019 14:47:52 -0800 (PST) MIME-Version: 1.0 References: <20190129003422.9328-1-rick.p.edgecombe@intel.com> <20190129003422.9328-6-rick.p.edgecombe@intel.com> <162C6C29-CD81-46FE-9A54-6ED05A93A9CB@gmail.com> <00649AE8-69C0-4CD2-A916-B8C8F0F5DAC3@amacapital.net> <6FE10C97-25FF-4E99-A96A-465CBACA935B@gmail.com> <3EA322C6-5645-4900-AEC6-97FC05716F75@gmail.com> In-Reply-To: <3EA322C6-5645-4900-AEC6-97FC05716F75@gmail.com> From: Andy Lutomirski Date: Mon, 11 Feb 2019 14:47:39 -0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v2 05/20] x86/alternative: initializing temporary mm for patching To: Nadav Amit Cc: Andy Lutomirski , Rick Edgecombe , Ingo Molnar , LKML , X86 ML , "H. Peter Anvin" , Thomas Gleixner , Borislav Petkov , Dave Hansen , Peter Zijlstra , Damian Tometzki , linux-integrity , LSM List , Andrew Morton , Kernel Hardening , Linux-MM , Will Deacon , Ard Biesheuvel , Kristen Carlson Accardi , "Dock, Deneen T" , Kees Cook , Dave Hansen Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: On Mon, Feb 11, 2019 at 11:18 AM Nadav Amit wrote: > > > On Feb 11, 2019, at 11:07 AM, Andy Lutomirski wrote: > > > > I'm certainly amenable to other solutions, but this one does seem the > > least messy. I looked at my old patch, and it doesn't do what you > > want. I'd suggest you just add a percpu variable like cpu_dr7 and rig > > up some accessors so that it stays up to date. Then you can skip the > > dr7 writes if there are no watchpoints set. > > > > Also, EFI is probably a less interesting example than rare_write. > > With rare_write, especially the dynamically allocated variants that > > people keep coming up with, we'll need a swath of address space fully > > as large as the vmalloc area. and getting *that* right while still > > using the kernel address range might be more of a mess than we really > > want to deal with. > > As long as you feel comfortable with this solution, I=E2=80=99m fine with= it. > > Here is what I have (untested). I prefer to save/restore all the DRs, > because IIRC DR6 indications are updated even if breakpoints are disabled > (in DR7). And anyhow, that is the standard interface. Seems reasonable, but: > > > -- >8 -- > > From: Nadav Amit > Date: Mon, 11 Feb 2019 03:07:08 -0800 > Subject: [PATCH] mm: save DRs when loading temporary mm > > Signed-off-by: Nadav Amit > --- > arch/x86/include/asm/mmu_context.h | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/arch/x86/include/asm/mmu_context.h b/arch/x86/include/asm/mm= u_context.h > index d684b954f3c0..4f92ec3df149 100644 > --- a/arch/x86/include/asm/mmu_context.h > +++ b/arch/x86/include/asm/mmu_context.h > @@ -13,6 +13,7 @@ > #include > #include > #include > +#include > > extern atomic64_t last_mm_ctx_id; > > @@ -358,6 +359,7 @@ static inline unsigned long __get_current_cr3_fast(vo= id) > > typedef struct { > struct mm_struct *prev; > + unsigned short bp_enabled : 1; > } temp_mm_state_t; > > /* > @@ -380,6 +382,15 @@ static inline temp_mm_state_t use_temporary_mm(struc= t mm_struct *mm) > lockdep_assert_irqs_disabled(); > state.prev =3D this_cpu_read(cpu_tlbstate.loaded_mm); > switch_mm_irqs_off(NULL, mm, current); > + > + /* > + * If breakpoints are enabled, disable them while the temporary m= m is > + * used - they do not belong and might cause wrong signals or cra= shes. > + */ Maybe clarify this? Add some mention that the specific problem is that user code could set a watchpoint on an address that is also used in the temporary mm. Arguably we should not disable *kernel* breakpoints a la perf, but that seems like quite a minor issue, at least as long as use_temporary_mm() doesn't get wider use. But a comment that this also disables perf breakpoints and that this could be undesirable might be in order as well.