diff mbox

[edk2] MdeModulePkg/AcpiTableDxe: consider version mask when removing tables

Message ID 1490021496-10195-1-git-send-email-ard.biesheuvel@linaro.org
State Accepted
Commit f859c6796f4064e2142d4bfaae55dbd3aaf70c55
Headers show

Commit Message

Ard Biesheuvel March 20, 2017, 2:51 p.m. UTC 
Invocations of EFI_ACPI_TABLE_PROTOCOL::UninstallAcpiTable() may
result in a crash when the value of PcdAcpiExposedTableVersions does
not include EFI_ACPI_TABLE_VERSION_1_0B.

The reason is that EFI_ACPI_TABLE_PROTOCOL::InstallAcpiTable() will
only populate the Rsdt1/Rsdt3 pointers when EFI_ACPI_TABLE_VERSION_1_0B
is set, whereas EFI_ACPI_TABLE_PROTOCOL::UninstallAcpiTable() will
invoke PublishTables with EFI_ACPI_TABLE_VERSION_1_0B alawys set,
resulting in a NULL pointer dereference of the Rsdt1/Rsdt3 pointers.

So take PcdAcpiExposedTableVersions into account for UninstallAcpiTable
as well.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

-- 
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Zeng, Star March 21, 2017, 1:28 a.m. UTC | #1 
Reviewed-by: Star Zeng <star.zeng@intel.com>


-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] 

Sent: Monday, March 20, 2017 10:52 PM
To: edk2-devel@lists.01.org; Zeng, Star <star.zeng@intel.com>; Tian, Feng <feng.tian@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: [PATCH] MdeModulePkg/AcpiTableDxe: consider version mask when removing tables

Invocations of EFI_ACPI_TABLE_PROTOCOL::UninstallAcpiTable() may result in a crash when the value of PcdAcpiExposedTableVersions does not include EFI_ACPI_TABLE_VERSION_1_0B.

The reason is that EFI_ACPI_TABLE_PROTOCOL::InstallAcpiTable() will only populate the Rsdt1/Rsdt3 pointers when EFI_ACPI_TABLE_VERSION_1_0B is set, whereas EFI_ACPI_TABLE_PROTOCOL::UninstallAcpiTable() will invoke PublishTables with EFI_ACPI_TABLE_VERSION_1_0B alawys set, resulting in a NULL pointer dereference of the Rsdt1/Rsdt3 pointers.

So take PcdAcpiExposedTableVersions into account for UninstallAcpiTable as well.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-develdiff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
index 4bb848df5203..a635e1de5a7c 100644
--- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
+++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
@@ -290,24 +290,27 @@ UninstallAcpiTable (  {
   EFI_ACPI_TABLE_INSTANCE   *AcpiTableInstance;
   EFI_STATUS                Status;
+  EFI_ACPI_TABLE_VERSION    Version;
 
   //
   // Get the instance of the ACPI table protocol
   //
   AcpiTableInstance = EFI_ACPI_TABLE_INSTANCE_FROM_THIS (This);
 
+  Version = PcdGet32 (PcdAcpiExposedTableVersions);
+
   //
   // Uninstall the ACPI table
   //
   Status = RemoveTableFromList (
              AcpiTableInstance,
-             EFI_ACPI_TABLE_VERSION_1_0B | ACPI_TABLE_VERSION_GTE_2_0,
+             Version,
              TableKey
              );
   if (!EFI_ERROR (Status)) {
     Status = PublishTables (
                AcpiTableInstance,
-               EFI_ACPI_TABLE_VERSION_1_0B | ACPI_TABLE_VERSION_GTE_2_0
+               Version
                );
   }
 
--
2.7.4
Ard Biesheuvel March 21, 2017, 7:07 a.m. UTC | #2 
On 21 March 2017 at 01:28, Zeng, Star <star.zeng@intel.com> wrote:
> Reviewed-by: Star Zeng <star.zeng@intel.com>

>


Pushed as f859c6796f40, thanks

> -----Original Message-----

> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> Sent: Monday, March 20, 2017 10:52 PM

> To: edk2-devel@lists.01.org; Zeng, Star <star.zeng@intel.com>; Tian, Feng <feng.tian@intel.com>

> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> Subject: [PATCH] MdeModulePkg/AcpiTableDxe: consider version mask when removing tables

>

> Invocations of EFI_ACPI_TABLE_PROTOCOL::UninstallAcpiTable() may result in a crash when the value of PcdAcpiExposedTableVersions does not include EFI_ACPI_TABLE_VERSION_1_0B.

>

> The reason is that EFI_ACPI_TABLE_PROTOCOL::InstallAcpiTable() will only populate the Rsdt1/Rsdt3 pointers when EFI_ACPI_TABLE_VERSION_1_0B is set, whereas EFI_ACPI_TABLE_PROTOCOL::UninstallAcpiTable() will invoke PublishTables with EFI_ACPI_TABLE_VERSION_1_0B alawys set, resulting in a NULL pointer dereference of the Rsdt1/Rsdt3 pointers.

>

> So take PcdAcpiExposedTableVersions into account for UninstallAcpiTable as well.

>

> Contributed-under: TianoCore Contribution Agreement 1.0

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---

>  MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 7 +++++--

>  1 file changed, 5 insertions(+), 2 deletions(-)

>

> diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c

> index 4bb848df5203..a635e1de5a7c 100644

> --- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c

> +++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c

> @@ -290,24 +290,27 @@ UninstallAcpiTable (  {

>    EFI_ACPI_TABLE_INSTANCE   *AcpiTableInstance;

>    EFI_STATUS                Status;

> +  EFI_ACPI_TABLE_VERSION    Version;

>

>    //

>    // Get the instance of the ACPI table protocol

>    //

>    AcpiTableInstance = EFI_ACPI_TABLE_INSTANCE_FROM_THIS (This);

>

> +  Version = PcdGet32 (PcdAcpiExposedTableVersions);

> +

>    //

>    // Uninstall the ACPI table

>    //

>    Status = RemoveTableFromList (

>               AcpiTableInstance,

> -             EFI_ACPI_TABLE_VERSION_1_0B | ACPI_TABLE_VERSION_GTE_2_0,

> +             Version,

>               TableKey

>               );

>    if (!EFI_ERROR (Status)) {

>      Status = PublishTables (

>                 AcpiTableInstance,

> -               EFI_ACPI_TABLE_VERSION_1_0B | ACPI_TABLE_VERSION_GTE_2_0

> +               Version

>                 );

>    }

>

> --

> 2.7.4

>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
diff mbox

Patch

diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
index 4bb848df5203..a635e1de5a7c 100644
--- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
+++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
@@ -290,24 +290,27 @@  UninstallAcpiTable (
 {
   EFI_ACPI_TABLE_INSTANCE   *AcpiTableInstance;
   EFI_STATUS                Status;
+  EFI_ACPI_TABLE_VERSION    Version;
 
   //
   // Get the instance of the ACPI table protocol
   //
   AcpiTableInstance = EFI_ACPI_TABLE_INSTANCE_FROM_THIS (This);
 
+  Version = PcdGet32 (PcdAcpiExposedTableVersions);
+
   //
   // Uninstall the ACPI table
   //
   Status = RemoveTableFromList (
              AcpiTableInstance,
-             EFI_ACPI_TABLE_VERSION_1_0B | ACPI_TABLE_VERSION_GTE_2_0,
+             Version,
              TableKey
              );
   if (!EFI_ERROR (Status)) {
     Status = PublishTables (
                AcpiTableInstance,
-               EFI_ACPI_TABLE_VERSION_1_0B | ACPI_TABLE_VERSION_GTE_2_0
+               Version
                );
   }