From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id ADF0F3418C for ; Mon, 27 Nov 2023 20:09:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="eDWXIpgR" Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 2A75760F44 for ; Mon, 27 Nov 2023 20:09:20 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 2A75760F44 Authentication-Results: smtp3.osuosl.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.a=rsa-sha256 header.s=casper.20170209 header.b=eDWXIpgR X-Virus-Scanned: amavisd-new at osuosl.org X-Spam-Flag: NO X-Spam-Score: -4.398 X-Spam-Level: Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id fGEggqfbD2yM for ; Mon, 27 Nov 2023 20:09:19 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [IPv6:2001:8b0:10b:1236::1]) by smtp3.osuosl.org (Postfix) with ESMTPS id 38D5B60F2D for ; Mon, 27 Nov 2023 20:09:19 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 38D5B60F2D DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Transfer-Encoding: Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date: Sender:Reply-To:Content-ID:Content-Description; bh=0g8rmOmWV3gIvWc7PsUFJZCK/mjDmfDlEjIfQZ9fLpw=; b=eDWXIpgRHK0l6Kp1zvM8AozfOF C6Od0RZKcBiM1TpdWu2XQUTQBZigv/lVFcp4wmzQVJpUa30ta0VAtD86zKeO0v7pZOLiVWQRr4KN8 hk4FOzx8yGdiIasjtANlRLndmWRJMvBdSEMGCKmqSYLag0ay+cj9SgXrs1vld5SS+8dkv6bWq3SV5 MDYEEFDnBglQijN9WYMxBEuwTpC4pfbqpYLSGbfh3mrMun861qNxt0UbAxZ6eZUufig+63zyL2qSO J7adiJq4wNyrGWs8soFqK96nVJbz6xxEW1NnrEo4DM5u7kBaGizRWOoC4Gn534kdhjeQxjkmw2wur fASu7jhg==; Received: from j130084.upc-j.chello.nl ([24.132.130.84] helo=noisy.programming.kicks-ass.net) by casper.infradead.org with esmtpsa (Exim 4.94.2 #2 (Red Hat Linux)) id 1r7huM-00Bhd4-E7; Mon, 27 Nov 2023 20:08:42 +0000 Received: by noisy.programming.kicks-ass.net (Postfix, from userid 1000) id 12A193002F1; Mon, 27 Nov 2023 21:08:42 +0100 (CET) Date: Mon, 27 Nov 2023 21:08:41 +0100 From: Peter Zijlstra To: "Madhavan T. Venkataraman" Cc: =?iso-8859-1?Q?Micka=EBl_Sala=FCn?= , Borislav Petkov , Dave Hansen , "H . Peter Anvin" , Ingo Molnar , Kees Cook , Paolo Bonzini , Sean Christopherson , Thomas Gleixner , Vitaly Kuznetsov , Wanpeng Li , Alexander Graf , Chao Peng , "Edgecombe, Rick P" , Forrest Yuan Yu , James Gowans , James Morris , John Andersen , Marian Rotariu , Mihai =?utf-8?B?RG9uyJt1?= , =?utf-8?B?TmljdciZb3IgQ8OuyJt1?= , Thara Gopinath , Trilok Soni , Wei Liu , Will Deacon , Yu Zhang , Zahra Tarkhani , =?utf-8?Q?=C8=98tefan_=C8=98icleru?= , dev@lists.cloudhypervisor.org, kvm@vger.kernel.org, linux-hardening@vger.kernel.org, linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org, linux-security-module@vger.kernel.org, qemu-devel@nongnu.org, virtualization@lists.linux-foundation.org, x86@kernel.org, xen-devel@lists.xenproject.org Subject: Re: [RFC PATCH v2 17/19] heki: x86: Update permissions counters during text patching Message-ID: <20231127200841.GZ3818@noisy.programming.kicks-ass.net> References: <20231113022326.24388-1-mic@digikod.net> <20231113022326.24388-18-mic@digikod.net> <20231113081929.GA16138@noisy.programming.kicks-ass.net> Precedence: bulk X-Mailing-List: virtualization@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: On Mon, Nov 27, 2023 at 10:48:29AM -0600, Madhavan T. Venkataraman wrote: > Apologies for the late reply. I was on vacation. Please see my response below: > > On 11/13/23 02:19, Peter Zijlstra wrote: > > On Sun, Nov 12, 2023 at 09:23:24PM -0500, Mickaël Salaün wrote: > >> From: Madhavan T. Venkataraman > >> > >> X86 uses a function called __text_poke() to modify executable code. This > >> patching function is used by many features such as KProbes and FTrace. > >> > >> Update the permissions counters for the text page so that write > >> permissions can be temporarily established in the EPT to modify the > >> instructions in that page. > >> > >> Cc: Borislav Petkov > >> Cc: Dave Hansen > >> Cc: H. Peter Anvin > >> Cc: Ingo Molnar > >> Cc: Kees Cook > >> Cc: Madhavan T. Venkataraman > >> Cc: Mickaël Salaün > >> Cc: Paolo Bonzini > >> Cc: Sean Christopherson > >> Cc: Thomas Gleixner > >> Cc: Vitaly Kuznetsov > >> Cc: Wanpeng Li > >> Signed-off-by: Madhavan T. Venkataraman > >> --- > >> > >> Changes since v1: > >> * New patch > >> --- > >> arch/x86/kernel/alternative.c | 5 ++++ > >> arch/x86/mm/heki.c | 49 +++++++++++++++++++++++++++++++++++ > >> include/linux/heki.h | 14 ++++++++++ > >> 3 files changed, 68 insertions(+) > >> > >> diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c > >> index 517ee01503be..64fd8757ba5c 100644 > >> --- a/arch/x86/kernel/alternative.c > >> +++ b/arch/x86/kernel/alternative.c > >> @@ -18,6 +18,7 @@ > >> #include > >> #include > >> #include > >> +#include > >> #include > >> #include > >> #include > >> @@ -1801,6 +1802,7 @@ static void *__text_poke(text_poke_f func, void *addr, const void *src, size_t l > >> */ > >> pgprot = __pgprot(pgprot_val(PAGE_KERNEL) & ~_PAGE_GLOBAL); > >> > >> + heki_text_poke_start(pages, cross_page_boundary ? 2 : 1, pgprot); > >> /* > >> * The lock is not really needed, but this allows to avoid open-coding. > >> */ > >> @@ -1865,7 +1867,10 @@ static void *__text_poke(text_poke_f func, void *addr, const void *src, size_t l > >> } > >> > >> local_irq_restore(flags); > >> + > >> pte_unmap_unlock(ptep, ptl); > >> + heki_text_poke_end(pages, cross_page_boundary ? 2 : 1, pgprot); > >> + > >> return addr; > >> } > > > > This makes no sense, we already use a custom CR3 with userspace alias > > for the actual pages to write to, why are you then frobbing permissions > > on that *again* ? > > Today, the permissions for a guest page in the extended page table > (EPT) are RWX (unless permissions are restricted for some specific > reason like for shadow page table pages). In this Heki feature, we > don't allow RWX by default in the EPT. We only allow those permissions > in the EPT that the guest page actually needs. E.g., for a text page, > it is R_X in both the guest page table and the EPT. To what end? If you always mirror what the guest does, you've not actually gained anything. > For text patching, the above code establishes an alternate mapping in > the guest page table that is RW_ so that the text can be patched. That > needs to be reflected in the EPT so that the EPT permissions will > change from R_X to RWX. In other words, RWX is allowed only as > necessary. At the end of patching, the EPT permissions are restored to > R_X. > > Does that address your comment? No, if you want to mirror the native PTEs why don't you hook into the paravirt page-table muck and get all that for free? Also, this is the user range, are you saying you're also playing these daft games with user maps?