MdeModulePkg/AcpiTableDxe: consider version mask when removing tables

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

Posted by Ard Biesheuvel 7 years, 7 months ago

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

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

Posted by Zeng, Star 7 years, 7 months ago

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(-)

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

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

Posted by Ard Biesheuvel 7 years, 7 months ago

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