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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id B7B37EB64DA for ; Wed, 12 Jul 2023 03:43:25 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 22A166B0071; Tue, 11 Jul 2023 23:43:25 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 1DA166B0072; Tue, 11 Jul 2023 23:43:25 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0C9686B0075; Tue, 11 Jul 2023 23:43:25 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id F073D6B0071 for ; Tue, 11 Jul 2023 23:43:24 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id B90441C8748 for ; Wed, 12 Jul 2023 03:43:24 +0000 (UTC) X-FDA: 81001564728.05.45DDD7A Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf21.hostedemail.com (Postfix) with ESMTP id F012B1C000A for ; Wed, 12 Jul 2023 03:43:22 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=DgwIEugq; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf21.hostedemail.com: domain of gerg@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=gerg@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1689133403; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=yWqOIbJIjS7o5/XTqWAJd97ejQ09T6/eo1XZPFz0PAw=; b=Hf/vb26CxMom4tKVPLOS9b37OMu3venV4dSzqV0X+hyVqYBSg2znHfpi2vwszRMHH8Uohb PpAEbm/6KxjglhzwtjaRs09NlxzIqyPCavtM0BqIT+ffzQR6o489A2XG+OQTy0Oxqic9Kq sLhCfVRALusb4DRr0ku9GVqFOo5ZBNs= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=DgwIEugq; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf21.hostedemail.com: domain of gerg@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=gerg@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1689133403; a=rsa-sha256; cv=none; b=t7U8AbCt3pHSY0E1ImbgLbOi0rRuTZxDbaxV5vsMC6BDO5qhZmyB0ciw/uGyVSEEup2uvL IPQdhvlD/9HRjxSIFkcCTuWuKnAoQWra83MIip9xSzAkT74sBT26AwOUBrKo638jl3OS1G PRP0brSUTd8FDhfFYcnEna6JhIo3jao= Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id D885E615AE; Wed, 12 Jul 2023 03:43:21 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id AED30C433C7; Wed, 12 Jul 2023 03:43:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1689133401; bh=dIblCOHbs5y4iJ94xVrHaATNeAfkHWAz/gZObzXL80Y=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=DgwIEugqA1YFxdmJgYPE1AZYxSpFGEsaPb3N7Qw7pvlpK5shrEalrexuuk0kIJjot fhnNwTpEe7SFMf9vHHW7vb8HpTTPctzuer/Bszwium13eaQ119IQQVIALNaXmC5LvW sR0sPBUKiazgIPOCo+AIu4MP4pvkpP3Z2tb2yYS/cOGPkli+07xYXnQ9xWpzOAoBYt IGSCXSTSXx9lkk7voUgDcA1X0GL18NlETKJDEdYS8lmNndTJKf6OiNzI5R9suL3hl9 9HhIFpGsYLrIr+F4qqmT0T6yGALKeIOxlCOgWEvRKUIYjUaV1XneeSPOdFvzNmsof3 2Stj3PtXmluLQ== Message-ID: Date: Wed, 12 Jul 2023 13:43:17 +1000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 Subject: Re: [PATCH] fs: binfmt_elf_efpic: fix personality for fdpic ELF Content-Language: en-US To: Kees Cook Cc: linux-arm@lists.infradead.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, viro@zeniv.linux.org.uk, ebiederm@xmission.com, brauner@kernel.org References: <20230711133955.483393-1-gerg@kernel.org> <202307110901.7E9A0D0AE5@keescook> From: Greg Ungerer In-Reply-To: <202307110901.7E9A0D0AE5@keescook> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: F012B1C000A X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: 99rd7qhecde3xf9zdix4x3r9p14ea7s5 X-HE-Tag: 1689133402-678725 X-HE-Meta: U2FsdGVkX1/3+x/z0f7/Y5WYe3O+LFfeStXiK36tuvaq0mgxdj8TSQasagsn6sXenmPQpnZhImXtPSnkQWy4L7te2pmx0+7rv1qFNRmUea6HcnbIU20TL1YNLN4u1Junoojuz22YmxN8ibLp4BJyHDnWcbmvS0dr5XeJ5pkzG2iG/OXHoxYJ9iMt4sr2vF8EoS+U4w5w8gEqsL2LJCSY1G7q1x/YV1ER5RRme+BxnIsQTguqa1jpDvYdqPPNqQrXqTldRvXUHMMEc0+OWEcwLW93oitlrCixUt3buFY6CF8ivTXLm31v5gV8OhNQv8u4s6asJK78CbsxSHLbYuRkSV34R0qQNhujmuQSjQItKQ2ZgLreeVvYT89L6d22a0rHi2XvVRgUNePQq8ND3pjCTvoaqd+XPsfkiYF/ZalXeAs6vL9k6a8cBHcGMYUW9HmRObQp37r321D6XNCIky59oxXhHiqfzIxA6P98GTYwOTnaCYo6Z2cdqLK4AypAxYI2DO+2V/0IEM5rfEcSVhREm3XqOls26eXPGql4FWVdWLEMOxa+gTbh4fXkQhFf+3sFJtddpazscQpT9LcOus9FkRck6nHHN6ynvVGdX9SgiftVlOFK1ROjgmPQQkLnKvoazOz3hcVBmaJ0xAL2yCj0mpKUrdoGgQxC/NX2c7BRXzy25gtPnVIkvt6fn97i8nCQlkQz+WCcAwBv3UA1wagDTIE3ID2bQr0ZaNOQbCSeReeayuj6h6jNiO/9UyqVkP3h8iwygIWopC3pZelLbpegCs67+KBAh+cu1fhPOjiJPOcacECFi3llpuODKAIPh6buaUb8f3g7nVSf4tcYkdkPNU2Ad2p91fA63q2fX4qeiEsY7iOeeRwNB3GpP88mLRT1hk0Mm0yUFPBVlMYZqNoCUq+GuNeOhu2RFr8vgN0wAAhwVPiZ2/seWILmr5TyT29n+f+SF77z5VLawPdlkEm 3dM5wQOs oh63tzyorJDqX+OK1UIwuBZV1ef+3AN9FQ6W9uIVJcKkS/yVvb15TU8SfOKV1AAwEXF9RNF8rxe5mFCzBL6A83m8+N886oCUT45vJfM1JEWJ9FJalgTAjU3Jzl6f8BlY8NdryxG9Dt5HCteTRX2epeVePUoPIy4QUjnQgOnicG3z+De5pHk/a2tnkJUYqfHE+sDPlWQyYuw0Mb82g4xQWUFrPSKjmt1YrDW16NstsidkMXbGHajhFaZycyDfKiqaweknjQe9mQTBrwHz79WzHuv9yJMwTmq/2GvRNnWUnIaZrQrmlMIWhWGeZmcUrR4sMYCjhA/ux9QCmDyctlyOL73FUBO9N8PG7uwdPOuBdZIYzSyveTmWUD5o2rBeePtUf5WjTUldfUZsnJ2A= 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: Hi Kees, On 12/7/23 02:11, Kees Cook wrote: > On Tue, Jul 11, 2023 at 11:39:55PM +1000, Greg Ungerer wrote: >> The elf-fdpic loader hard sets the process personality to either >> PER_LINUX_FDPIC for true elf-fdpic binaries or to PER_LINUX for >> normal ELF binaries (in this case they would be constant displacement >> compiled with -pie for example). The problem with that is that it >> will lose any other bits that may be in the ELF header personality >> (such as the "bug emulation" bits). >> >> On the ARM architecture the ADDR_LIMIT_32BIT flag is used to signify >> a normal 32bit binary - as opposed to a legacy 26bit address binary. >> This matters since start_thread() will set the ARM CPSR register as >> required based on this flag. If the elf-fdpic loader loses this bit >> the process will be mis-configured and crash out pretty quickly. >> >> Modify elf-fdpic loaders personality setting for ELF binaries so that >> it preserves the upper three bytes by using the SET_PERSONALITY macro >> to set it. This macro in the generic case sets PER_LINUX but and >> preserves the upper bytes. Architectures can override this for their >> specific use case, and ARM does exactly this. > > Thanks for tracking this down! > > There are some twisty macros in use across all the architectures here! > > I notice the bare set_personality() call remains, though. Is that right? > > For example, ARM (and sh and xtensa) also sets: > > #define elf_check_fdpic(x) ((x)->e_ident[EI_OSABI] == ELFOSABI_ARM_FDPIC) > > so it's possible the first half of the "if" below could get executed, > and ARM (and possibly other architectures) would again lose the other > flags, if I'm reading correctly. Yes, it is all a little confusing, and the fdpic handling is a little different to the standard ELF handling in binfmt_elf.c (with its use of SET_PERSONALITY2). > (And the fact that PER_LINUX is actually 0x0 is oddly handled, leaving > it implicit in most architectures.) > > What seems perhaps more correct is to remove the "if" entirely and make > sure that SET_PERSONALITY() checks the header flags on all architectures? I had thought along those same lines as well. Changing it to be something more like this: SET_PERSONALITY(exec_params.hdr); if (elf_check_fdpic(&exec_params.hdr)) current->personality |= FDPIC_FUNCPTRS; Which I think better handles any arch specifics via the SET_PERSONALITY() use. But I chickened out since I can't test fdpic binaries at this time. > But I'm less familiar with this area, so please let me know what I'm > missing. :) Me too :-) It is definitely broken for loading standard ELF binaries on a noMMU system using binfmt_elf_fdpic.c, which is what led me down this path. It loses the ADDR_LIMIT_32BIT bit in the personality and that causes application crashing. >> Signed-off-by: Greg Ungerer >> --- >> >> Is anyone out there using elf-fdpic on ARM? > > It would seem you're the first? :) (_Should_ it be usable on ARM?) I was assuming that it must have worked at some time. The binfmt_elf_fdpic loader was enabled for ARM in commit 50b2b2e691cd ("ARM: add ELF_FDPIC support") by Nicolas Pitre. But that was way back in 2017. Regards Greg > -Kees > >> This seems to break it rather badly due to the loss of that ADDR_LIMIT_32BIT >> bit from the process personality. >> >> fs/binfmt_elf_fdpic.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c >> index a05eafcacfb2..f29ae1d96fd7 100644 >> --- a/fs/binfmt_elf_fdpic.c >> +++ b/fs/binfmt_elf_fdpic.c >> @@ -348,7 +348,7 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm) >> if (elf_check_fdpic(&exec_params.hdr)) >> set_personality(PER_LINUX_FDPIC); >> else >> - set_personality(PER_LINUX); >> + SET_PERSONALITY(exec_params.hdr); >> if (elf_read_implies_exec(&exec_params.hdr, executable_stack)) >> current->personality |= READ_IMPLIES_EXEC; >> >> -- >> 2.25.1 >> >