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=-15.4 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham 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 205D5C4338F for ; Thu, 5 Aug 2021 09:09:12 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id F27DE61029 for ; Thu, 5 Aug 2021 09:09:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236274AbhHEJJZ (ORCPT ); Thu, 5 Aug 2021 05:09:25 -0400 Received: from pegase2.c-s.fr ([93.17.235.10]:41185 "EHLO pegase2.c-s.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239135AbhHEJJY (ORCPT ); Thu, 5 Aug 2021 05:09:24 -0400 X-Greylist: delayed 351 seconds by postgrey-1.27 at vger.kernel.org; Thu, 05 Aug 2021 05:09:24 EDT Received: from localhost (mailhub3.si.c-s.fr [172.26.127.67]) by localhost (Postfix) with ESMTP id 4GgN8P4tb5z9sWJ; Thu, 5 Aug 2021 11:09:09 +0200 (CEST) X-Virus-Scanned: amavisd-new at c-s.fr Received: from pegase2.c-s.fr ([172.26.127.65]) by localhost (pegase2.c-s.fr [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id bbX-U_KwuJ3S; Thu, 5 Aug 2021 11:09:09 +0200 (CEST) Received: from messagerie.si.c-s.fr (messagerie.si.c-s.fr [192.168.25.192]) by pegase2.c-s.fr (Postfix) with ESMTP id 4GgN8P3rktz9sWH; Thu, 5 Aug 2021 11:09:09 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by messagerie.si.c-s.fr (Postfix) with ESMTP id 213358B7BE; Thu, 5 Aug 2021 11:09:09 +0200 (CEST) X-Virus-Scanned: amavisd-new at c-s.fr Received: from messagerie.si.c-s.fr ([127.0.0.1]) by localhost (messagerie.si.c-s.fr [127.0.0.1]) (amavisd-new, port 10023) with ESMTP id FM7R-FbjtVsw; Thu, 5 Aug 2021 11:09:09 +0200 (CEST) Received: from [192.168.4.90] (unknown [192.168.4.90]) by messagerie.si.c-s.fr (Postfix) with ESMTP id 7BFF08B7BD; Thu, 5 Aug 2021 11:09:08 +0200 (CEST) Subject: Re: [PATCH v5 4/8] lkdtm/x86_64: Add test to hijack a patch mapping To: "Christopher M. Riedl" , linuxppc-dev@lists.ozlabs.org Cc: keescook@chromium.org, peterz@infradead.org, x86@kernel.org, npiggin@gmail.com, linux-hardening@vger.kernel.org, tglx@linutronix.de, dja@axtens.net References: <20210713053113.4632-1-cmr@linux.ibm.com> <20210713053113.4632-5-cmr@linux.ibm.com> From: Christophe Leroy Message-ID: <95de5ec5-8d48-c969-3c9f-966561f9f58e@csgroup.eu> Date: Thu, 5 Aug 2021 11:09:06 +0200 User-Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: <20210713053113.4632-5-cmr@linux.ibm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: fr Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-hardening@vger.kernel.org Le 13/07/2021 à 07:31, Christopher M. Riedl a écrit : > A previous commit implemented an LKDTM test on powerpc to exploit the > temporary mapping established when patching code with STRICT_KERNEL_RWX > enabled. Extend the test to work on x86_64 as well. > > Signed-off-by: Christopher M. Riedl > --- > drivers/misc/lkdtm/perms.c | 26 ++++++++++++++++++++++---- > 1 file changed, 22 insertions(+), 4 deletions(-) > > diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c > index 39e7456852229..41e87e5f9cc86 100644 > --- a/drivers/misc/lkdtm/perms.c > +++ b/drivers/misc/lkdtm/perms.c > @@ -224,7 +224,7 @@ void lkdtm_ACCESS_NULL(void) > } > > #if (IS_BUILTIN(CONFIG_LKDTM) && defined(CONFIG_STRICT_KERNEL_RWX) && \ > - defined(CONFIG_PPC)) > + (defined(CONFIG_PPC) || defined(CONFIG_X86_64))) > /* > * This is just a dummy location to patch-over. > */ > @@ -233,12 +233,25 @@ static void patching_target(void) > return; > } > > -#include > const u32 *patch_site = (const u32 *)&patching_target; > > +#ifdef CONFIG_PPC > +#include > +#endif > + > +#ifdef CONFIG_X86_64 > +#include > +#endif > + > static inline int lkdtm_do_patch(u32 data) > { > +#ifdef CONFIG_PPC > return patch_instruction((u32 *)patch_site, ppc_inst(data)); > +#endif > +#ifdef CONFIG_X86_64 > + text_poke((void *)patch_site, &data, sizeof(u32)); > + return 0; > +#endif > } > > static inline u32 lkdtm_read_patch_site(void) > @@ -249,11 +262,16 @@ static inline u32 lkdtm_read_patch_site(void) > /* Returns True if the write succeeds */ > static inline bool lkdtm_try_write(u32 data, u32 *addr) > { > +#ifdef CONFIG_PPC > __put_kernel_nofault(addr, &data, u32, err); > return true; > > err: > return false; > +#endif > +#ifdef CONFIG_X86_64 > + return !__put_user(data, addr); > +#endif > } > > static int lkdtm_patching_cpu(void *data) > @@ -346,8 +364,8 @@ void lkdtm_HIJACK_PATCH(void) > > void lkdtm_HIJACK_PATCH(void) > { > - if (!IS_ENABLED(CONFIG_PPC)) > - pr_err("XFAIL: this test only runs on powerpc\n"); > + if (!IS_ENABLED(CONFIG_PPC) && !IS_ENABLED(CONFIG_X86_64)) > + pr_err("XFAIL: this test only runs on powerpc and x86_64\n"); > if (!IS_ENABLED(CONFIG_STRICT_KERNEL_RWX)) > pr_err("XFAIL: this test requires CONFIG_STRICT_KERNEL_RWX\n"); > if (!IS_BUILTIN(CONFIG_LKDTM)) > Instead of spreading arch specific stuff into LKDTM, wouldn't it make sence to define common a common API ? Because the day another arch like arm64 implements it own approach, do we add specific functions again and again into LKDTM ? Also, I find it odd to define tests only when they can succeed. For other tests like ACCESS_USERSPACE, they are there all the time, regardless of whether we have selected CONFIG_PPC_KUAP or not. I think it should be the same here, have it all there time, if CONFIG_STRICT_KERNEL_RWX is selected the test succeeds otherwise it fails, but it is always there. Christophe