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 7AFC8740032 for ; Wed, 15 Jan 2025 18:52:54 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=ukhyWtO82L2r46xvwST04Dhn9h1QPxYcnS2V0m3XjrE=; c=relaxed/simple; d=groups.io; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References:MIME-Version:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Resent-Date:Resent-From:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Transfer-Encoding; s=20240830; t=1736967174; v=1; x=1737226372; b=rFwNrmCSXVoQbwTGLkL/M8OTK2FbuWe3dvge/Z/IQWsbwVVs9ydU/xLfQ8AK3kQ4Fp3RqgYG Bb9YvGTjT92Ee29mXot5WhGxmL4CieRkzAC2wZS7BIpzEw82eMBZJi/iGWl7XMiyM9i2PSRtv+T c3w05UNwBEY0200TuMDVod2kNqQcs/g4/aFy1R2PKCFXRX4BSg8+rZBCffN6efKdOvJIknhxQRF JJWVok4t2eQwu5LPTPS4/+vsnYsbRIIy3m+2+/i1JUi2NuNnhKfZJKjAfqpilePDgFaCDthzd9v 9n0sFPvEuWDPAI1uJkYd5eptwd2x4hg71LqI34kfKyp8Q== X-Received: by 127.0.0.2 with SMTP id CvRtYY7687511xmdzi6iFZJM; Wed, 15 Jan 2025 10:52:52 -0800 X-Received: from mail-qv1-f47.google.com (mail-qv1-f47.google.com [209.85.219.47]) by mx.groups.io with SMTP id smtpd.web11.33552.1736373622110588853 for ; Wed, 08 Jan 2025 14:00:22 -0800 X-Received: by mail-qv1-f47.google.com with SMTP id 6a1803df08f44-6dd420f82e2so3676436d6.1 for ; Wed, 08 Jan 2025 14:00:21 -0800 (PST) X-Forwarded-Encrypted: i=1; AJvYcCU19/3ViReI58/AsosYbf08pZn2FWT98LuLoK8JS2v63//HApgn7WKIgyPiuHnJsqdfUshM3g==@edk2.groups.io X-Gm-Message-State: WvWaYWhe9ucRTGogYJ6rIYl6x7686176AA= X-Gm-Gg: ASbGncuxw2bVBeKz1iFN+xTKdEUkciPlwlJXkWjxXtMhPrIOlJVALFO7E1gE/hv1AOd OI/fIuhFl2m4Zl5e6JvCg6CnnTzJI2mYL5o11Wx3gXvbZ+IEL/oeSTZmSt7jyTdMI2NeZ7p8JTC x5v81OpA+dwG0QlJ12Xd+jDZp7DIvfWa3ZrTvXxKKS8bgAlTi/47GYiLiJhW2YBuoPAvKL5w9XS bTJo/NB7i85DLRDW7N90vK/AW9hmyR/zwKX14DL92Q5j59Saton2Zs= X-Google-Smtp-Source: AGHT+IGYAtV0NWg+g25XA8prhBZQtnI1Wq2npFdzbcYk2MRvl9G6tk69B/IbxPWv68nknyTKZ78xgA== X-Received: by 2002:ad4:5d61:0:b0:6d4:dae:6250 with SMTP id 6a1803df08f44-6df9b28c6aemr74791756d6.34.1736373619641; Wed, 08 Jan 2025 14:00:19 -0800 (PST) X-Received: from localhost ([2a03:2880:20ff:40::]) by smtp.gmail.com with ESMTPSA id 6a1803df08f44-6dd3c151877sm165560896d6.127.2025.01.08.14.00.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 08 Jan 2025 14:00:19 -0800 (PST) From: "Usama Arif via groups.io" To: linux-efi@vger.kernel.org, devel@edk2.groups.io, kexec@lists.infradead.org Cc: ardb@kernel.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, Usama Arif Subject: [edk2-devel] [RFC 1/2] efi/memattr: Use desc_size instead of total size to check for corruption Date: Wed, 8 Jan 2025 21:53:36 +0000 Message-ID: <20250108215957.3437660-2-usamaarif642@gmail.com> In-Reply-To: <20250108215957.3437660-1-usamaarif642@gmail.com> References: <20250108215957.3437660-1-usamaarif642@gmail.com> MIME-Version: 1.0 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: Wed, 15 Jan 2025 10:52:51 -0800 Resent-From: usamaarif642@gmail.com Reply-To: devel@edk2.groups.io,usamaarif642@gmail.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Transfer-Encoding: 8bit X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20240830 header.b=rFwNrmCS; 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 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(-) diff --git a/drivers/firmware/efi/memattr.c b/drivers/firmware/efi/memattr.c index c38b1a335590..d3bc161361fb 100644 --- a/drivers/firmware/efi/memattr.c +++ b/drivers/firmware/efi/memattr.c @@ -40,21 +40,17 @@ int __init efi_memattr_init(void) goto unmap; } - /* - * Sanity check: the Memory Attributes Table contains up to 3 entries - * for each entry of type EfiRuntimeServicesCode in the EFI memory map. - * So if the size of the table exceeds 3x the size of the entire EFI - * memory map, there is clearly something wrong, and the table should - * just be ignored altogether. + * Sanity check: the Memory Attributes Table desc_size and + * efi.memmap.desc_size should match. */ - size = tbl->num_entries * tbl->desc_size; - if (size > 3 * efi.memmap.nr_map * efi.memmap.desc_size) { - pr_warn(FW_BUG "Corrupted EFI Memory Attributes Table detected! (version == %u, desc_size == %u, num_entries == %u)\n", - tbl->version, tbl->desc_size, tbl->num_entries); + if (efi.memmap.desc_size != tbl->desc_size) { + pr_warn(FW_BUG "Corrupted EFI Memory Attributes Table detected! (version == %u, table desc_size == %u, efi.memmap.desc_size == %lu, table num_entries == %u)\n", + tbl->version, tbl->desc_size, efi.memmap.desc_size, tbl->num_entries); goto unmap; } + size = tbl->num_entries * tbl->desc_size; tbl_size = sizeof(*tbl) + size; memblock_reserve(efi_mem_attr_table, tbl_size); set_bit(EFI_MEM_ATTR, &efi.flags); -- 2.43.5 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#121000): https://edk2.groups.io/g/devel/message/121000 Mute This Topic: https://groups.io/mt/110633385/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-