From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail05.groups.io (mail05.groups.io [45.79.224.7]) by spool.mail.gandi.net (Postfix) with ESMTPS id 8D0E5AC0B99 for ; Fri, 10 Jan 2025 07:24:08 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=Q0tmpNeIpdo7be8AG1vIE9THj4hec9fDE/KGAOVgkmc=; c=relaxed/simple; d=groups.io; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject:To:Cc:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Resent-Date:Resent-From:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type; s=20240830; t=1736493848; v=1; x=1736753046; b=ZCcibrt23C4+18jgo2bx2rrIEVHpYEmyN4C9PBub4jziNu7QBd6W1/XNXCQFs1pevVYdkwjo ipMcAo6am2BZOEpyGxKp7dwkb2HiDFSFiWqMj4MZxu+tp2qO722h5RW0ic2cVdvK/u86YTf726t xuAFF/NQ1Lv1yp7SNSeYe9Q4Wx0ujxeXWEn+HLbNgZTKxYJaI2bAQ0vdu/d15g5qDXy4jGX8DWQ 3T3HhOIoa1G23JaYvynxHauqJjBZ/NQQd50DsnRE7WppJWNkWgnHJQJVZFkoK/pPtFpM+JcVR2/ 11xfE3Op9pfJfohM/Y3zK3liCtrjD7iWvh2rnFYj64e5A== X-Received: by 127.0.0.2 with SMTP id VUX8YY7687511xWLG42SsQ7g; Thu, 09 Jan 2025 23:24:06 -0800 X-Received: from nyc.source.kernel.org (nyc.source.kernel.org [147.75.193.91]) by mx.groups.io with SMTP id smtpd.web11.13627.1736493846100833196 for ; Thu, 09 Jan 2025 23:24:06 -0800 X-Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id 80974A41A12 for ; Fri, 10 Jan 2025 07:22:16 +0000 (UTC) X-Received: by smtp.kernel.org (Postfix) with ESMTPSA id AA746C4CEE3 for ; Fri, 10 Jan 2025 07:24:04 +0000 (UTC) X-Received: by mail-lf1-f41.google.com with SMTP id 2adb3069b0e04-54024ecc33dso1853673e87.0 for ; Thu, 09 Jan 2025 23:24:04 -0800 (PST) X-Forwarded-Encrypted: i=1; AJvYcCXGS99ijvf4XNoQhbXZLtvYTWJC5uixhJ2/1nIsndetoCyABhsGBG93X2q87H+QvJLhraZGbw==@edk2.groups.io X-Gm-Message-State: J9HsyrrRdoJhwFTaDY70KFenx7686176AA= X-Google-Smtp-Source: AGHT+IGYqkfOo1l5vp36+uebNKlLgeJDXUixK18FpXwrJSpHyumiIGT64CNoieQ3oaHhrHajrgMKZe9lPCDnkLCfZdM= X-Received: by 2002:a05:6512:3082:b0:542:6d01:f54d with SMTP id 2adb3069b0e04-542844f3fc1mr3354249e87.3.1736493843027; Thu, 09 Jan 2025 23:24:03 -0800 (PST) MIME-Version: 1.0 References: <20250108215957.3437660-1-usamaarif642@gmail.com> <20250108215957.3437660-2-usamaarif642@gmail.com> In-Reply-To: From: "Ard Biesheuvel via groups.io" Date: Fri, 10 Jan 2025 08:21:21 +0100 X-Gmail-Original-Message-ID: X-Gm-Features: AbW1kvaLZXidD5PN4oD-GwjBbzl6R0uQiwKJYbNGyIakDi17-k8ZzLnZ277sE34 Message-ID: Subject: Re: [edk2-devel] [RFC 1/2] efi/memattr: Use desc_size instead of total size to check for corruption To: Usama Arif Cc: linux-efi@vger.kernel.org, devel@edk2.groups.io, kexec@lists.infradead.org, hannes@cmpxchg.org, dyoung@redhat.com, x86@kernel.org, linux-kernel@vger.kernel.org, leitao@debian.org, gourry@gourry.net, kernel-team@meta.com Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Resent-Date: Thu, 09 Jan 2025 23:24:06 -0800 Resent-From: ardb@kernel.org Reply-To: devel@edk2.groups.io,ardb@kernel.org List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: text/plain; charset="UTF-8" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20240830 header.b=ZCcibrt2; dmarc=pass (policy=none) header.from=groups.io; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 45.79.224.7 as permitted sender) smtp.mailfrom=bounce@groups.io On Thu, 9 Jan 2025 at 17:36, Usama Arif wrote: > > > > On 09/01/2025 15:45, Ard Biesheuvel wrote: > > On Wed, 8 Jan 2025 at 23:00, Usama Arif wrote: > >> > >> The commit in [1] introduced a check to see if EFI memory attributes > >> table was corrupted. It assumed that efi.memmap.nr_map remains > >> constant, but it changes during late boot. > >> Hence, the check is valid during cold boot, but not in the subsequent > >> kexec boot. > >> > >> This is best explained with an exampled. At cold boot, for a test > >> machine: > >> efi.memmap.nr_map=91, > >> memory_attributes_table->num_entries=48, > >> desc_size = 48 > >> Hence, the check introduced in [1] where 3x the size of the > >> entire EFI memory map is a reasonable upper bound for the size of this > >> table is valid. > >> > >> In late boot __efi_enter_virtual_mode calls 2 functions that updates > >> efi.memmap.nr_map: > >> - efi_map_regions which reduces the `count` of map entries > >> (for e.g. if should_map_region returns false) and which is reflected > >> in efi.memmap by __efi_memmap_init. > >> At this point efi.memmap.nr_map becomes 46 in the test machine. > >> - efi_free_boot_services which also reduces the number of memory regions > >> available (for e.g. if md->type or md->attribute is not the right value). > >> At this point efi.memmap.nr_map becomes 9 in the test machine. > >> Hence when you kexec into a new kernel and pass efi.memmap, the > >> paramaters that are compared are: > >> efi.memmap.nr_map=9, > >> memory_attributes_table->num_entries=48, > >> desc_size = 48 > >> where the check in [1] is no longer valid with such a low efi.memmap.nr_map > >> as it was reduced due to efi_map_regions and efi_free_boot_services. > >> > >> A more appropriate check is to see if the description size reported by > >> efi and memory attributes table is the same. > >> > >> [1] https://lore.kernel.org/all/20241031175822.2952471-2-ardb+git@google.com/ > >> > >> Fixes: 8fbe4c49c0cc ("efi/memattr: Ignore table if the size is clearly bogus") > >> Reported-by: Breno Leitao > >> Signed-off-by: Usama Arif > >> --- > >> drivers/firmware/efi/memattr.c | 16 ++++++---------- > >> 1 file changed, 6 insertions(+), 10 deletions(-) > >> > > > > The more I think about this, the more I feel that kexec on x86 should > > simply discard this table, and run with the firmware code RWX (which > > is not the end of the world). > > > By discard this table, do you mean kexec not use e820_table_firmware? No, I mean kexec ignores the memory attributes table. > Also a very basic question, what do you mean by run with the firmware RWX? > The memory attributes table is an overlay for the EFI memory map that describes which runtime code regions may be mapped with restricted permissions. Without this table, everything will be mapped writable as well as executable, but only in the EFI page tables, which are only active when an EFI call is in progress. > Sorry for the very basic questions above! > No worries. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#120977): https://edk2.groups.io/g/devel/message/120977 Mute This Topic: https://groups.io/mt/110517813/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-