From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by mx.groups.io with SMTP id smtpd.web11.10871.1672919071300408180 for ; Thu, 05 Jan 2023 03:44:31 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=TFMZhzGA; spf=pass (domain: kernel.org, ip: 145.40.68.75, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 2FF91B81A84 for ; Thu, 5 Jan 2023 11:44:29 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id AF278C433F2 for ; Thu, 5 Jan 2023 11:44:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1672919068; bh=kxzTDsjnCe8IDq9He8R4giX6N2klvP1ydXzKGcW/NuE=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=TFMZhzGAdWR0IlZ/RufMCTv+KRKj+A3ZLKjGqXY2LzbeEp27ldYpawecZzDbrsQn9 ykwITibz/ZBDZqGaQtEJW26JNK9cjPcZMX4HG8AldaWISarjwGAoEJ1LTlCf8Rjtz0 N/lIx4TU5ommFq5JMCv5uLaMNz1X43qJ5PrL98V5xaq38aehdNfp8HKwenEmdL/a/h khEvdXKanXERHJUQOUkRLS4UKGBWXCZuaJYF6xduuY0CL2kY+L+9Dk55rdd+u3AZFY vFgGNIGRrqjH+97QIJftFTx/zMJYiu5dCBo6wB6e4KfnPPhcSz1IHMXcEL8J+smV8S 4IC6bQ1bftrHQ== Received: by mail-lj1-f178.google.com with SMTP id p2so22803563ljn.7 for ; Thu, 05 Jan 2023 03:44:28 -0800 (PST) X-Gm-Message-State: AFqh2kpLuagIMi+d9L6TRKVvTv4Egp5lDjCcuUAQRsTHfHJBOCrL1Cfc aF21VexnTk5aE/dKW9+feHEzKfoVaFpK13QPit0= X-Google-Smtp-Source: AMrXdXubJLxcHt9zELbL2lsmH+Gb3B/GLW1cRw8XevbuByPyf+RPx4PlAntq7xLpfgiGkDIlGb1SsbjDjYSm+0a+atE= X-Received: by 2002:a2e:bd0c:0:b0:27f:bc58:3924 with SMTP id n12-20020a2ebd0c000000b0027fbc583924mr2915949ljq.352.1672919066726; Thu, 05 Jan 2023 03:44:26 -0800 (PST) MIME-Version: 1.0 References: <20230105081127.muckhpcw6agfkfrs@sirius.home.kraxel.org> <3F97900D-3C85-4E10-BBC2-360CECFD2D39@csgraf.de> <20230105111929.wgjtafvbht5gl2x4@sirius.home.kraxel.org> In-Reply-To: <20230105111929.wgjtafvbht5gl2x4@sirius.home.kraxel.org> From: "Ard Biesheuvel" Date: Thu, 5 Jan 2023 12:44:15 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [PATCH v3 03/16] ArmVirtPkg: make EFI_LOADER_DATA non-executable To: devel@edk2.groups.io, kraxel@redhat.com Cc: Alexander Graf , dann frazier , Leif Lindholm Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, 5 Jan 2023 at 12:19, Gerd Hoffmann wrote: > > Hi, > > > > What number would you expect? I'd hope that we get to <100 realistica= lly. > > > > > > I'm happy to hear about alternatives to this approach. I'm very confi= dent that forcing NX on always is going to have the opposite effect of what= we want: Everyone who ships AAVMF binaries will disable NX because they ev= entually get bug reports from users that their shiny update regressed some = legit use case. > > > > > > The only alternative I can think of would be logic similar to the pat= ch I sent without any grub hash check: Exclude AllocatePages for LoaderData= from the NX logic. Keep NX for any other non-code memory type as well as L= oaderData pool allocations. > > > Another thing we might consider is trapping exec permission violations > > and switching the pages in question from rw- to r-x. > > That sounds neat, especially as we can print a big'n'fat warning in that > case, so the problem gets attention without actually breaking users. > That, and a sleep(5) > Looking at the efi calls it looks like edk2 doesn't track the owner of > an allocation (say by image handle), so I suspect it is not possible to > automatically figure who is to blame? > > > Does GRUB generally load/map executable modules at page granularity? > > I don't think so, at least the code handles modules not being page > aligned. But I think it's not grub modules, that fix was actually > picked up meanwhile. But there are downstream patches for image > loader code which look suspicious to me ... > OK, so the GRUB PE/COFF loader strikes again :-( So shim may be broken in the exact same way, and the things shim loads may not adhere to page alignment for the sections. Loading the kernel itself this way should be fine, though - we always had section alignment in the EFI stub kernel. The downside of this approach is that it can only be done on a page-by-page basis, given that the EFI_LOADER_DATA region in question will cover both the .text/.rodata and the .data/.bss of the loaded image. Could someone check/confirm whether shim builds need to be take into account here? Thanks.