From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (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 66444364D2 for ; Mon, 27 Nov 2023 16:48:35 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.microsoft.com header.i=@linux.microsoft.com header.b="aDtpsQCv" Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 8EEDB81437 for ; Mon, 27 Nov 2023 16:48:34 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 8EEDB81437 Authentication-Results: smtp1.osuosl.org; dkim=pass (1024-bit key) header.d=linux.microsoft.com header.i=@linux.microsoft.com header.a=rsa-sha256 header.s=default header.b=aDtpsQCv X-Virus-Scanned: amavisd-new at osuosl.org X-Spam-Flag: NO X-Spam-Score: -17.502 X-Spam-Level: Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id zUA1Be7ObH3a for ; Mon, 27 Nov 2023 16:48:33 +0000 (UTC) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by smtp1.osuosl.org (Postfix) with ESMTP id 546EB81435 for ; Mon, 27 Nov 2023 16:48:33 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 546EB81435 Received: from [192.168.4.26] (unknown [47.186.13.91]) by linux.microsoft.com (Postfix) with ESMTPSA id 862F420B74C0; Mon, 27 Nov 2023 08:48:30 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 862F420B74C0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1701103712; bh=jeMkp5Q+EKa19c6aqn2EEeUT1WIFRK1lWII/T1Aq+rQ=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=aDtpsQCv5Z7lrnyBfZDtXIQ5YkOeXU+eDniqpgG5DSD+i9xBYSIgXtJO6yM/hN7oZ EiQi/S+Yjy2TnSaq8aWFJwrDCDQJ4E01iXdFLG0uIDqHbnx9CwBuz6XIqHOPh2NLet Jzp9sNlniTRePSjgypI8ZiHOZ0a54xSepS+7ZLF8= Message-ID: Date: Mon, 27 Nov 2023 10:48:29 -0600 Precedence: bulk X-Mailing-List: virtualization@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [RFC PATCH v2 17/19] heki: x86: Update permissions counters during text patching Content-Language: en-US To: Peter Zijlstra , =?UTF-8?B?TWlja2HDq2wgU2FsYcO8?= =?UTF-8?Q?n?= Cc: 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 , =?UTF-8?Q?Mihai_Don=C8=9Bu?= , =?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 References: <20231113022326.24388-1-mic@digikod.net> <20231113022326.24388-18-mic@digikod.net> <20231113081929.GA16138@noisy.programming.kicks-ass.net> From: "Madhavan T. Venkataraman" In-Reply-To: <20231113081929.GA16138@noisy.programming.kicks-ass.net> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. 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? Madhavan