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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 28FB5C433FE for ; Wed, 13 Oct 2021 12:00:55 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id ADBA6610C9 for ; Wed, 13 Oct 2021 12:00:54 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org ADBA6610C9 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=csgroup.eu Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 47FA46B006C; Wed, 13 Oct 2021 08:00:54 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 42F0D900002; Wed, 13 Oct 2021 08:00:54 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 31E636B0072; Wed, 13 Oct 2021 08:00:54 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0050.hostedemail.com [216.40.44.50]) by kanga.kvack.org (Postfix) with ESMTP id 2226D6B006C for ; Wed, 13 Oct 2021 08:00:54 -0400 (EDT) Received: from smtpin29.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 9CB2C8249980 for ; Wed, 13 Oct 2021 12:00:53 +0000 (UTC) X-FDA: 78691272786.29.4EED103 Received: from pegase2.c-s.fr (pegase2.c-s.fr [93.17.235.10]) by imf20.hostedemail.com (Postfix) with ESMTP id 1F18DD000269 for ; Wed, 13 Oct 2021 12:00:52 +0000 (UTC) Received: from localhost (mailhub3.si.c-s.fr [172.26.127.67]) by localhost (Postfix) with ESMTP id 4HTrhg3GGJz9sSP; Wed, 13 Oct 2021 14:00:51 +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 jWmm-fqIIxwb; Wed, 13 Oct 2021 14:00:51 +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 4HTrhg2CRyz9sSN; Wed, 13 Oct 2021 14:00:51 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by messagerie.si.c-s.fr (Postfix) with ESMTP id 34B3D8B77E; Wed, 13 Oct 2021 14:00:51 +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 oPJNy44hoNDg; Wed, 13 Oct 2021 14:00:51 +0200 (CEST) Received: from PO20335.IDSI0.si.c-s.fr (unknown [172.25.230.103]) by messagerie.si.c-s.fr (Postfix) with ESMTP id E21F08B763; Wed, 13 Oct 2021 14:00:50 +0200 (CEST) Subject: Re: [PATCH v1 10/10] lkdtm: Fix execute_[user]_location() To: Kees Cook Cc: Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman , Andrew Morton , "James E.J. Bottomley" , Helge Deller , Arnd Bergmann , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, linux-ia64@vger.kernel.org, linux-parisc@vger.kernel.org, linux-arch@vger.kernel.org, linux-mm@kvack.org References: <202110130012.4608FFD38E@keescook> From: Christophe Leroy Message-ID: <8f2e80a9-bef1-2594-61ed-85d510e00cf2@csgroup.eu> Date: Wed, 13 Oct 2021 14:00:50 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <202110130012.4608FFD38E@keescook> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: fr-FR Authentication-Results: imf20.hostedemail.com; dkim=none; dmarc=none; spf=pass (imf20.hostedemail.com: domain of christophe.leroy@csgroup.eu designates 93.17.235.10 as permitted sender) smtp.mailfrom=christophe.leroy@csgroup.eu X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 1F18DD000269 X-Stat-Signature: dxawfiueq47x3u6yedc5s7iqpfwcr5wi X-HE-Tag: 1634126452-52115 Content-Transfer-Encoding: quoted-printable X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: Le 13/10/2021 =C3=A0 09:16, Kees Cook a =C3=A9crit=C2=A0: > On Mon, Oct 11, 2021 at 05:25:37PM +0200, Christophe Leroy wrote: >> execute_location() and execute_user_location() intent >> to copy do_nothing() text and execute it at a new location. >> However, at the time being it doesn't copy do_nothing() function >> but do_nothing() function descriptor which still points to the >> original text. So at the end it still executes do_nothing() at >> its original location allthough using a copied function descriptor. >> >> So, fix that by really copying do_nothing() text and build a new >> function descriptor by copying do_nothing() function descriptor and >> updating the target address with the new location. >> >> Also fix the displayed addresses by dereferencing do_nothing() >> function descriptor. >> >> Signed-off-by: Christophe Leroy >> --- >> drivers/misc/lkdtm/perms.c | 45 +++++++++++++++++++++++++++++++-----= -- >> 1 file changed, 37 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c >> index da16564e1ecd..1d03cd44c21d 100644 >> --- a/drivers/misc/lkdtm/perms.c >> +++ b/drivers/misc/lkdtm/perms.c >> @@ -44,19 +44,42 @@ static noinline void do_overwritten(void) >> return; >> } >> =20 >> +static void *setup_function_descriptor(funct_descr_t *fdesc, void *ds= t) >> +{ >> + int err; >> + >> + if (!__is_defined(HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR)) >> + return dst; >> + >> + err =3D copy_from_kernel_nofault(fdesc, do_nothing, sizeof(*fdesc)); >> + if (err < 0) >> + return ERR_PTR(err); >> + >> + fdesc->addr =3D (unsigned long)dst; >> + barrier(); >> + >> + return fdesc; >> +} >> + >> static noinline void execute_location(void *dst, bool write) >> { >> - void (*func)(void) =3D dst; >> + void (*func)(void); >> + funct_descr_t fdesc; >> + void *do_nothing_text =3D dereference_symbol_descriptor(do_nothing); >> =20 >> - pr_info("attempting ok execution at %px\n", do_nothing); >> + pr_info("attempting ok execution at %px\n", do_nothing_text); >> do_nothing(); >> =20 >> if (write =3D=3D CODE_WRITE) { >> - memcpy(dst, do_nothing, EXEC_SIZE); >> + memcpy(dst, do_nothing_text, EXEC_SIZE); >> flush_icache_range((unsigned long)dst, >> (unsigned long)dst + EXEC_SIZE); >> } >> - pr_info("attempting bad execution at %px\n", func); >> + func =3D setup_function_descriptor(&fdesc, dst); >> + if (IS_ERR(func)) >> + return; >=20 > I think this error case should complain with some details. :) Maybe: >=20 > pr_err("FAIL: could not build function descriptor for %px\n", dst); Ok, I was going to add that in v2, but after one more thought I realise=20 that there's no need to use copy_from_kernel_nofault() here, we are=20 copying a static object into our stack, so a memcpy() will be good enough= . >=20 >> + >> + pr_info("attempting bad execution at %px\n", dst); >=20 > And then leave this pr_info() as it was, before the > setup_function_descriptor() call. >=20 >> func(); >> pr_err("FAIL: func returned\n"); >> } >> @@ -66,16 +89,22 @@ static void execute_user_location(void *dst) >> int copied; >> =20 >> /* Intentionally crossing kernel/user memory boundary. */ >> - void (*func)(void) =3D dst; >> + void (*func)(void); >> + funct_descr_t fdesc; >> + void *do_nothing_text =3D dereference_symbol_descriptor(do_nothing); >> =20 >> - pr_info("attempting ok execution at %px\n", do_nothing); >> + pr_info("attempting ok execution at %px\n", do_nothing_text); >> do_nothing(); >> =20 >> - copied =3D access_process_vm(current, (unsigned long)dst, do_nothing= , >> + copied =3D access_process_vm(current, (unsigned long)dst, do_nothing= _text, >> EXEC_SIZE, FOLL_WRITE); >> if (copied < EXEC_SIZE) >> return; >> - pr_info("attempting bad execution at %px\n", func); >> + func =3D setup_function_descriptor(&fdesc, dst); >> + if (IS_ERR(func)) >> + return; >> + >> + pr_info("attempting bad execution at %px\n", dst); >=20 > Same here. >=20 >> func(); >> pr_err("FAIL: func returned\n"); >> } >> --=20 >> 2.31.1 >> >=20