From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.groups.io with SMTP id smtpd.web10.10747.1672917577688304836 for ; Thu, 05 Jan 2023 03:19:38 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=Q2gd0c+I; spf=pass (domain: redhat.com, ip: 170.10.133.124, mailfrom: kraxel@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1672917576; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=tmMmufGgtQqDNgm5RIsf2WuwDkzr1uMkf5Vj3Bsc7pM=; b=Q2gd0c+IyoVW+dPGJg0anCMYzHZSBYm6VzrNiJRMo8twJ+S2jtehVKfZW6YRm34ESvvVNu r5XzgXwz8KChD+pVT/oKqHUJNMWe5xydXeh9lS+wmgyUbX5O+4Yq86OEc9We2VYzmfZPMN CDqgt5eyzqm4Z4bMnXSCTwgYtig/Dng= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-486-YOvf6U5BNTC7umT6ZID_Jw-1; Thu, 05 Jan 2023 06:19:31 -0500 X-MC-Unique: YOvf6U5BNTC7umT6ZID_Jw-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.rdu2.redhat.com [10.11.54.8]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 139BB802D1B; Thu, 5 Jan 2023 11:19:31 +0000 (UTC) Received: from sirius.home.kraxel.org (unknown [10.39.192.238]) by smtp.corp.redhat.com (Postfix) with ESMTPS id CD49EC15BA0; Thu, 5 Jan 2023 11:19:30 +0000 (UTC) Received: by sirius.home.kraxel.org (Postfix, from userid 1000) id 75E7C180062D; Thu, 5 Jan 2023 12:19:29 +0100 (CET) Date: Thu, 5 Jan 2023 12:19:29 +0100 From: "Gerd Hoffmann" To: Ard Biesheuvel Cc: Alexander Graf , dann frazier , devel@edk2.groups.io, Leif Lindholm Subject: Re: [edk2-devel] [PATCH v3 03/16] ArmVirtPkg: make EFI_LOADER_DATA non-executable Message-ID: <20230105111929.wgjtafvbht5gl2x4@sirius.home.kraxel.org> References: <20230105081127.muckhpcw6agfkfrs@sirius.home.kraxel.org> <3F97900D-3C85-4E10-BBC2-360CECFD2D39@csgraf.de> MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 3.1 on 10.11.54.8 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hi, > > What number would you expect? I'd hope that we get to <100 realistically. > > > > I'm happy to hear about alternatives to this approach. I'm very confident 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 eventually 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 patch 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 LoaderData 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. 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 ... take care, Gerd