From nobody Sun Feb 8 23:41:11 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) client-ip=66.175.222.108; envelope-from=bounce+27952+106520+1787277+3901457@groups.io; helo=mail02.groups.io; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+106520+1787277+3901457@groups.io ARC-Seal: i=1; a=rsa-sha256; t=1688055638; cv=none; d=zohomail.com; s=zohoarc; b=kg8WkWz+4QBABx6Hb74x2i5cEnRpWTmHOtLIe8Uq1i9uAvfEBltAFchdbqm6fT20KjhLQ7KouQbi5so6mhWz3/2x5SmWV0Ib9yuiLAG1/IyQDRo3uBL5V/G3iDW1cRt8Tl+wjk0iZJfZtheY5W5mkAL6pO6+h6DIUGPAqJYvpSs= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1688055638; h=Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Reply-To:References:Sender:Subject:To; bh=K6qHJGzKlOSs/SJScX7S1tn4BxafFMLvNpxY3cw6fwY=; b=ZM7oM5N+ZjJ7pYYKEx5G28qrLzz70nVOguXyeLc/BU52o421lmiAiIanYgpceq99quvgx5ZM3OXZDPAVJVYZfyzqUj/1hyA5zxjo8fZ+CWaFyr05fuQA2/dxxCSgAJopgtIH3SOJAdi7Z+DNabqITO2ObCTJ2b6Cs5THn6yYJJE= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+106520+1787277+3901457@groups.io Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by mx.zohomail.com with SMTPS id 1688055638159583.1467695737837; Thu, 29 Jun 2023 09:20:38 -0700 (PDT) Return-Path: X-Received: by 127.0.0.2 with SMTP id nPcwYY1788612xdYdPtKjK76; Thu, 29 Jun 2023 09:20:37 -0700 X-Received: from mail-pf1-f177.google.com (mail-pf1-f177.google.com [209.85.210.177]) by mx.groups.io with SMTP id smtpd.web10.1398.1688055636938762346 for ; Thu, 29 Jun 2023 09:20:37 -0700 X-Received: by mail-pf1-f177.google.com with SMTP id d2e1a72fcca58-666eb03457cso600901b3a.1 for ; Thu, 29 Jun 2023 09:20:36 -0700 (PDT) X-Gm-Message-State: eFK5xEJKcyVFCRn9h5F9Y16Lx1787277AA= X-Google-Smtp-Source: APBJJlFlEz623kFAdVpx0kYXFAbfm2ZWf0U6Np0ty71MM+mwySHW30TTbyvbvMVNymVRMm4y9++W3Q== X-Received: by 2002:a05:6a00:2346:b0:673:5d1e:6657 with SMTP id j6-20020a056a00234600b006735d1e6657mr471408pfj.7.1688055636262; Thu, 29 Jun 2023 09:20:36 -0700 (PDT) X-Received: from localhost.localdomain ([50.46.230.135]) by smtp.gmail.com with ESMTPSA id b5-20020aa78705000000b0064f7c56d8b7sm6993578pfo.219.2023.06.29.09.20.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 29 Jun 2023 09:20:35 -0700 (PDT) From: "Taylor Beebe" To: devel@edk2.groups.io Cc: Taylor Beebe , Leif Lindholm , Ard Biesheuvel , Taylor Beebe Subject: [edk2-devel] [PATCH 3/4] ArmPkg: Fix Unsafe ASSERTs in MMU Logic Date: Thu, 29 Jun 2023 09:17:57 -0700 Message-ID: <580bf85d35a2f301cb5b99e88f58ac4af696cae1.1687989723.git.t@taylorbeebe.com> In-Reply-To: References: MIME-Version: 1.0 Precedence: Bulk List-Unsubscribe: List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,t@taylorbeebe.com Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1688055637; bh=egV6Y1TweXmibDlaKAs4KEDtONA09R/NQT7IXgiFtBs=; h=Cc:Date:From:Reply-To:Subject:To; b=UjMdiJn9yZMu+/jsJP50ZHsd2vT4trCM93Sh+ztsWZz2Mz+Dr3aVV9TU409n5wkZ4ha umGZqKAdNDxQJAoqr9nwJA2kBPsFNvI3RbTy/kYk9kZz7i6c0drgNkDP0czu4/YZAwk/s 3VEsibW72VTWgDpM9viFHUoZor7Ck9WFWD8= X-ZohoMail-DKIM: pass (identity @groups.io) X-ZM-MESSAGEID: 1688055640288100011 Content-Type: text/plain; charset="utf-8" From: Taylor Beebe There are ASSERTs present in the MMU logic to ensure various functions return successfully, but these ASSERTs may be ignored on release builds causing unsafe behavior. This patch updates the logic to handle unexpected return values and branch safely. Cc: Leif Lindholm Cc: Ard Biesheuvel Signed-off-by: Taylor Beebe --- ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c | 21 ++++++++++++----- ArmPkg/Drivers/CpuDxe/Arm/Mmu.c | 36 ++++++++++++++++++++++++----- 2 files changed, 45 insertions(+), 12 deletions(-) diff --git a/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c b/ArmPkg/Drivers/CpuDxe/AA= rch64/Mmu.c index 0d3bc2809682..d9d386dbed6b 100644 --- a/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c +++ b/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c @@ -148,10 +148,12 @@ GetNextEntryAttribute ( // Get the memory space map from GCD MemorySpaceMap =3D NULL; Status =3D gDS->GetMemorySpaceMap (&NumberOfDescriptors, &Memory= SpaceMap); - ASSERT_EFI_ERROR (Status); =20 - // We cannot get more than 3-level page table - ASSERT (TableLevel <=3D 3); + if (EFI_ERROR (Status) || (TableLevel > 3)) { + ASSERT_EFI_ERROR (Status); + ASSERT (TableLevel <=3D 3); + return 0; + } =20 // While the top level table might not contain TT_ENTRY_COUNT entries; // the subsequent ones should be filled up @@ -243,7 +245,11 @@ SyncCacheConfig ( // MemorySpaceMap =3D NULL; Status =3D gDS->GetMemorySpaceMap (&NumberOfDescriptors, &Memory= SpaceMap); - ASSERT_EFI_ERROR (Status); + + if (EFI_ERROR (Status)) { + ASSERT_EFI_ERROR (Status); + return Status; + } =20 // The GCD implementation maintains its own copy of the state of memory = space attributes. GCD needs // to know what the initial memory space attributes are. The CPU Arch. = Protocol does not provide a @@ -277,7 +283,7 @@ SyncCacheConfig ( ); =20 // Update GCD with the last region if valid - if (PageAttribute !=3D INVALID_ENTRY) { + if ((PageAttribute !=3D INVALID_ENTRY) && (EndAddressGcdRegion > BaseAdd= ressGcdRegion)) { SetGcdMemorySpaceAttributes ( MemorySpaceMap, NumberOfDescriptors, @@ -430,7 +436,10 @@ GetMemoryRegion ( UINTN EntryCount; UINTN T0SZ; =20 - ASSERT ((BaseAddress !=3D NULL) && (RegionLength !=3D NULL) && (RegionAt= tributes !=3D NULL)); + if ((BaseAddress =3D=3D NULL) || (RegionLength =3D=3D NULL) || (RegionAt= tributes =3D=3D NULL)) { + ASSERT ((BaseAddress !=3D NULL) && (RegionLength !=3D NULL) && (Region= Attributes !=3D NULL)); + return EFI_INVALID_PARAMETER; + } =20 TranslationTable =3D ArmGetTTBR0BaseAddress (); =20 diff --git a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c b/ArmPkg/Drivers/CpuDxe/Arm/Mm= u.c index 268c0bf3f9ae..5a2f36d06086 100644 --- a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c +++ b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c @@ -217,7 +217,10 @@ SyncCacheConfigPage ( } else if (PageAttributes !=3D NextPageAttributes) { // Convert Section Attributes into GCD Attributes Status =3D PageToGcdAttributes (NextPageAttributes, &GcdAttributes= ); - ASSERT_EFI_ERROR (Status); + if (EFI_ERROR (Status)) { + ASSERT_EFI_ERROR (Status); + GcdAttributes =3D 0; + } =20 // update GCD with these changes (this will recurse into our own C= puSetMemoryAttributes below which is OK) SetGcdMemorySpaceAttributes (MemorySpaceMap, NumberOfDescriptors, = *NextRegionBase, *NextRegionLength, GcdAttributes); @@ -230,7 +233,10 @@ SyncCacheConfigPage ( } else if (NextPageAttributes !=3D 0) { // Convert Page Attributes into GCD Attributes Status =3D PageToGcdAttributes (NextPageAttributes, &GcdAttributes); - ASSERT_EFI_ERROR (Status); + if (EFI_ERROR (Status)) { + ASSERT_EFI_ERROR (Status); + GcdAttributes =3D 0; + } =20 // update GCD with these changes (this will recurse into our own Cpu= SetMemoryAttributes below which is OK) SetGcdMemorySpaceAttributes (MemorySpaceMap, NumberOfDescriptors, *N= extRegionBase, *NextRegionLength, GcdAttributes); @@ -278,7 +284,12 @@ SyncCacheConfig ( // MemorySpaceMap =3D NULL; Status =3D gDS->GetMemorySpaceMap (&NumberOfDescriptors, &Memory= SpaceMap); - ASSERT_EFI_ERROR (Status); + + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "SyncCacheConfig - GetMemorySpaceMap() failed! St= atus: %r\n", Status)); + ASSERT_EFI_ERROR (Status); + return Status; + } =20 // The GCD implementation maintains its own copy of the state of memory = space attributes. GCD needs // to know what the initial memory space attributes are. The CPU Arch. = Protocol does not provide a @@ -307,7 +318,12 @@ SyncCacheConfig ( } else if (SectionAttributes !=3D NextSectionAttributes) { // Convert Section Attributes into GCD Attributes Status =3D SectionToGcdAttributes (NextSectionAttributes, &GcdAttr= ibutes); - ASSERT_EFI_ERROR (Status); + + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "SyncCacheConfig - SectionToGcdAttributes()= failed! Status: %r\n", Status)); + ASSERT_EFI_ERROR (Status); + GcdAttributes =3D 0; + } =20 // update GCD with these changes (this will recurse into our own C= puSetMemoryAttributes below which is OK) SetGcdMemorySpaceAttributes (MemorySpaceMap, NumberOfDescriptors, = NextRegionBase, NextRegionLength, GcdAttributes); @@ -343,7 +359,11 @@ SyncCacheConfig ( if (NextSectionAttributes !=3D 0) { // Convert Section Attributes into GCD Attributes Status =3D SectionToGcdAttributes (NextSectionAttributes, &GcdAttr= ibutes); - ASSERT_EFI_ERROR (Status); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "SyncCacheConfig - SectionToGcdAttributes()= failed! Status: %r\n", Status)); + ASSERT_EFI_ERROR (Status); + GcdAttributes =3D 0; + } =20 // update GCD with these changes (this will recurse into our own C= puSetMemoryAttributes below which is OK) SetGcdMemorySpaceAttributes (MemorySpaceMap, NumberOfDescriptors, = NextRegionBase, NextRegionLength, GcdAttributes); @@ -360,7 +380,11 @@ SyncCacheConfig ( if (NextSectionAttributes !=3D 0) { // Convert Section Attributes into GCD Attributes Status =3D SectionToGcdAttributes (NextSectionAttributes, &GcdAttribut= es); - ASSERT_EFI_ERROR (Status); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "SyncCacheConfig - SectionToGcdAttributes() fai= led! Status: %r\n", Status)); + ASSERT_EFI_ERROR (Status); + GcdAttributes =3D 0; + } =20 // update GCD with these changes (this will recurse into our own CpuSe= tMemoryAttributes below which is OK) SetGcdMemorySpaceAttributes (MemorySpaceMap, NumberOfDescriptors, Next= RegionBase, NextRegionLength, GcdAttributes); --=20 2.41.0.windows.1 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#106520): https://edk2.groups.io/g/devel/message/106520 Mute This Topic: https://groups.io/mt/99854265/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-