* [PATCH v2 0/2] Add error handling and initialize variables
@ 2019-09-12 3:27 Zhang, Shenglei
2019-09-12 3:27 ` [PATCH v2 1/2] MinPlatformPkg/AcpiTables: Initialize variables before used Zhang, Shenglei
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Zhang, Shenglei @ 2019-09-12 3:27 UTC (permalink / raw)
To: devel; +Cc: Michael Kubacki, Chasel Chiu, Nate DeSimone, Liming Gao
1.Add error handling to enhance status checking.
2.Initialize the variables before used and add check before
FreePool().
v2: Update copyright in 01/02.
Cc: Michael Kubacki <michael.a.kubacki@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Shenglei Zhang (2):
MinPlatformPkg/AcpiTables: Initialize variables before used
MinPlatformPkg/AcpiTables: Add error handling to
SortCpuLocalApicInTable
.../Acpi/AcpiTables/AcpiPlatform.c | 29 ++++++++++++-------
1 file changed, 18 insertions(+), 11 deletions(-)
--
2.18.0.windows.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/2] MinPlatformPkg/AcpiTables: Initialize variables before used
2019-09-12 3:27 [PATCH v2 0/2] Add error handling and initialize variables Zhang, Shenglei
@ 2019-09-12 3:27 ` Zhang, Shenglei
2019-09-12 4:05 ` Chiu, Chasel
` (2 more replies)
2019-09-12 3:27 ` [PATCH v2 2/2] MinPlatformPkg/AcpiTables: Add error handling to SortCpuLocalApicInTable Zhang, Shenglei
2019-09-12 21:03 ` [edk2-devel] [PATCH v2 0/2] Add error handling and initialize variables Nate DeSimone
2 siblings, 3 replies; 10+ messages in thread
From: Zhang, Shenglei @ 2019-09-12 3:27 UTC (permalink / raw)
To: devel; +Cc: Michael Kubacki, Chasel Chiu, Nate DeSimone, Liming Gao
MadtStructs, NewMadtTable and MaxMadtStructCount are not initialized
before used or at the proper place. So assign values to them at the
beginning and change the logic when freeing MadtStructs and
NewMadtTable.
Cc: Michael Kubacki <michael.a.kubacki@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
---
.../Acpi/AcpiTables/AcpiPlatform.c | 21 +++++++++++--------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index 5eb72792..2cc55ee8 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -1,7 +1,7 @@
/** @file
ACPI Platform Driver
-Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2017-2019, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -867,13 +867,15 @@ InstallMadtFromScratch (
UINT32 PcIoApicMask;
UINTN PcIoApicIndex;
+ MadtStructs = NULL;
+ NewMadtTable = NULL;
+ MaxMadtStructCount = 0;
+
DetectApicIdMap();
// Call for Local APIC ID Reorder
SortCpuLocalApicInTable ();
- NewMadtTable = NULL;
-
MaxMadtStructCount = (UINT32) (
MAX_CPU_NUM + // processor local APIC structures
MAX_CPU_NUM + // processor local x2APIC structures
@@ -1115,14 +1117,15 @@ Done:
//
// Free memory
//
- for (MadtStructsIndex = 0; MadtStructsIndex < MaxMadtStructCount; MadtStructsIndex++) {
- if (MadtStructs[MadtStructsIndex] != NULL) {
- FreePool (MadtStructs[MadtStructsIndex]);
+ if (MadtStructs != NULL){
+ for (MadtStructsIndex = 0; MadtStructsIndex < MaxMadtStructCount; MadtStructsIndex++) {
+ if (MadtStructs[MadtStructsIndex] != NULL) {
+ FreePool (MadtStructs[MadtStructsIndex]);
+ }
}
+ FreePool (MadtStructs);
}
-
- FreePool (MadtStructs);
-
+
if (NewMadtTable != NULL) {
FreePool (NewMadtTable);
}
--
2.18.0.windows.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/2] MinPlatformPkg/AcpiTables: Add error handling to SortCpuLocalApicInTable
2019-09-12 3:27 [PATCH v2 0/2] Add error handling and initialize variables Zhang, Shenglei
2019-09-12 3:27 ` [PATCH v2 1/2] MinPlatformPkg/AcpiTables: Initialize variables before used Zhang, Shenglei
@ 2019-09-12 3:27 ` Zhang, Shenglei
2019-09-12 4:06 ` Chiu, Chasel
` (2 more replies)
2019-09-12 21:03 ` [edk2-devel] [PATCH v2 0/2] Add error handling and initialize variables Nate DeSimone
2 siblings, 3 replies; 10+ messages in thread
From: Zhang, Shenglei @ 2019-09-12 3:27 UTC (permalink / raw)
To: devel; +Cc: Michael Kubacki, Chasel Chiu, Nate DeSimone, Liming Gao
Change ASSERT_EFI_ERROR to return value when the "if" statement is true.
As a result, when SortCpuLocalApicInTable is called, error handling is
needed. So add "if" statement to judge the returned Status from
SortCpuLocalApicInTable () in function InstallMadtFromScratch().
Cc: Michael Kubacki <michael.a.kubacki@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
---
.../Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index 2cc55ee8..ae25d753 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -354,7 +354,7 @@ SortCpuLocalApicInTable (
if(MAX_CPU_NUM <= Index) {
DEBUG ((EFI_D_ERROR, "Asserting the SortCpuLocalApicInTable Index Bufferflow\n"));
- ASSERT_EFI_ERROR(EFI_INVALID_PARAMETER);
+ return EFI_INVALID_PARAMETER;
}
TempVal = mCpuApicIdOrderTable[Index].ApicId;
@@ -874,7 +874,11 @@ InstallMadtFromScratch (
DetectApicIdMap();
// Call for Local APIC ID Reorder
- SortCpuLocalApicInTable ();
+ Status = SortCpuLocalApicInTable ();
+ if (EFI_ERROR (Status)) {
+ DEBUG ((EFI_D_ERROR, "SortCpuLocalApicInTable failed: %r\n", Status));
+ goto Done;
+ }
MaxMadtStructCount = (UINT32) (
MAX_CPU_NUM + // processor local APIC structures
--
2.18.0.windows.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] MinPlatformPkg/AcpiTables: Initialize variables before used
2019-09-12 3:27 ` [PATCH v2 1/2] MinPlatformPkg/AcpiTables: Initialize variables before used Zhang, Shenglei
@ 2019-09-12 4:05 ` Chiu, Chasel
2019-09-12 6:15 ` Kubacki, Michael A
2019-09-12 18:35 ` [edk2-devel] " Nate DeSimone
2 siblings, 0 replies; 10+ messages in thread
From: Chiu, Chasel @ 2019-09-12 4:05 UTC (permalink / raw)
To: Zhang, Shenglei, devel@edk2.groups.io
Cc: Kubacki, Michael A, Desimone, Nathaniel L, Gao, Liming
Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>
> -----Original Message-----
> From: Zhang, Shenglei
> Sent: Thursday, September 12, 2019 11:27 AM
> To: devel@edk2.groups.io
> Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>; Chiu, Chasel
> <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: [PATCH v2 1/2] MinPlatformPkg/AcpiTables: Initialize variables
> before used
>
> MadtStructs, NewMadtTable and MaxMadtStructCount are not initialized
> before used or at the proper place. So assign values to them at the
> beginning and change the logic when freeing MadtStructs and
> NewMadtTable.
>
> Cc: Michael Kubacki <michael.a.kubacki@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
> ---
> .../Acpi/AcpiTables/AcpiPlatform.c | 21 +++++++++++--------
> 1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index 5eb72792..2cc55ee8 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> @@ -1,7 +1,7 @@
> /** @file
> ACPI Platform Driver
>
> -Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2017-2019, Intel Corporation. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -867,13 +867,15 @@ InstallMadtFromScratch (
> UINT32
> PcIoApicMask;
> UINTN
> PcIoApicIndex;
>
> + MadtStructs = NULL;
> + NewMadtTable = NULL;
> + MaxMadtStructCount = 0;
> +
> DetectApicIdMap();
>
> // Call for Local APIC ID Reorder
> SortCpuLocalApicInTable ();
>
> - NewMadtTable = NULL;
> -
> MaxMadtStructCount = (UINT32) (
> MAX_CPU_NUM + // processor local APIC structures
> MAX_CPU_NUM + // processor local x2APIC structures
> @@ -1115,14 +1117,15 @@ Done:
> //
> // Free memory
> //
> - for (MadtStructsIndex = 0; MadtStructsIndex < MaxMadtStructCount;
> MadtStructsIndex++) {
> - if (MadtStructs[MadtStructsIndex] != NULL) {
> - FreePool (MadtStructs[MadtStructsIndex]);
> + if (MadtStructs != NULL){
> + for (MadtStructsIndex = 0; MadtStructsIndex < MaxMadtStructCount;
> MadtStructsIndex++) {
> + if (MadtStructs[MadtStructsIndex] != NULL) {
> + FreePool (MadtStructs[MadtStructsIndex]);
> + }
> }
> + FreePool (MadtStructs);
> }
> -
> - FreePool (MadtStructs);
> -
> +
> if (NewMadtTable != NULL) {
> FreePool (NewMadtTable);
> }
> --
> 2.18.0.windows.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] MinPlatformPkg/AcpiTables: Add error handling to SortCpuLocalApicInTable
2019-09-12 3:27 ` [PATCH v2 2/2] MinPlatformPkg/AcpiTables: Add error handling to SortCpuLocalApicInTable Zhang, Shenglei
@ 2019-09-12 4:06 ` Chiu, Chasel
2019-09-12 6:17 ` [edk2-devel] " Kubacki, Michael A
2019-09-12 18:36 ` Nate DeSimone
2 siblings, 0 replies; 10+ messages in thread
From: Chiu, Chasel @ 2019-09-12 4:06 UTC (permalink / raw)
To: Zhang, Shenglei, devel@edk2.groups.io
Cc: Kubacki, Michael A, Desimone, Nathaniel L, Gao, Liming
Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>
> -----Original Message-----
> From: Zhang, Shenglei
> Sent: Thursday, September 12, 2019 11:27 AM
> To: devel@edk2.groups.io
> Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>; Chiu, Chasel
> <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: [PATCH v2 2/2] MinPlatformPkg/AcpiTables: Add error handling to
> SortCpuLocalApicInTable
>
> Change ASSERT_EFI_ERROR to return value when the "if" statement is true.
> As a result, when SortCpuLocalApicInTable is called, error handling is
> needed. So add "if" statement to judge the returned Status from
> SortCpuLocalApicInTable () in function InstallMadtFromScratch().
>
> Cc: Michael Kubacki <michael.a.kubacki@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
> ---
> .../Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index 2cc55ee8..ae25d753 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> @@ -354,7 +354,7 @@ SortCpuLocalApicInTable (
>
> if(MAX_CPU_NUM <= Index) {
> DEBUG ((EFI_D_ERROR, "Asserting the SortCpuLocalApicInTable
> Index Bufferflow\n"));
> - ASSERT_EFI_ERROR(EFI_INVALID_PARAMETER);
> + return EFI_INVALID_PARAMETER;
> }
>
> TempVal = mCpuApicIdOrderTable[Index].ApicId;
> @@ -874,7 +874,11 @@ InstallMadtFromScratch (
> DetectApicIdMap();
>
> // Call for Local APIC ID Reorder
> - SortCpuLocalApicInTable ();
> + Status = SortCpuLocalApicInTable ();
> + if (EFI_ERROR (Status)) {
> + DEBUG ((EFI_D_ERROR, "SortCpuLocalApicInTable failed: %r\n",
> Status));
> + goto Done;
> + }
>
> MaxMadtStructCount = (UINT32) (
> MAX_CPU_NUM + // processor local APIC structures
> --
> 2.18.0.windows.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] MinPlatformPkg/AcpiTables: Initialize variables before used
2019-09-12 3:27 ` [PATCH v2 1/2] MinPlatformPkg/AcpiTables: Initialize variables before used Zhang, Shenglei
2019-09-12 4:05 ` Chiu, Chasel
@ 2019-09-12 6:15 ` Kubacki, Michael A
2019-09-12 18:35 ` [edk2-devel] " Nate DeSimone
2 siblings, 0 replies; 10+ messages in thread
From: Kubacki, Michael A @ 2019-09-12 6:15 UTC (permalink / raw)
To: Zhang, Shenglei, devel@edk2.groups.io
Cc: Chiu, Chasel, Desimone, Nathaniel L, Gao, Liming
Reviewed-by: Michael Kubacki <michael.a.kubacki@intel.com>
> -----Original Message-----
> From: Zhang, Shenglei
> Sent: Wednesday, September 11, 2019 8:27 PM
> To: devel@edk2.groups.io
> Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>; Chiu, Chasel
> <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: [PATCH v2 1/2] MinPlatformPkg/AcpiTables: Initialize variables
> before used
>
> MadtStructs, NewMadtTable and MaxMadtStructCount are not initialized
> before used or at the proper place. So assign values to them at the beginning
> and change the logic when freeing MadtStructs and NewMadtTable.
>
> Cc: Michael Kubacki <michael.a.kubacki@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
> ---
> .../Acpi/AcpiTables/AcpiPlatform.c | 21 +++++++++++--------
> 1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index 5eb72792..2cc55ee8 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> @@ -1,7 +1,7 @@
> /** @file
> ACPI Platform Driver
>
> -Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2017-2019, Intel Corporation. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -867,13 +867,15 @@ InstallMadtFromScratch (
> UINT32 PcIoApicMask;
> UINTN PcIoApicIndex;
>
> + MadtStructs = NULL;
> + NewMadtTable = NULL;
> + MaxMadtStructCount = 0;
> +
> DetectApicIdMap();
>
> // Call for Local APIC ID Reorder
> SortCpuLocalApicInTable ();
>
> - NewMadtTable = NULL;
> -
> MaxMadtStructCount = (UINT32) (
> MAX_CPU_NUM + // processor local APIC structures
> MAX_CPU_NUM + // processor local x2APIC structures
> @@ -1115,14 +1117,15 @@ Done:
> //
> // Free memory
> //
> - for (MadtStructsIndex = 0; MadtStructsIndex < MaxMadtStructCount;
> MadtStructsIndex++) {
> - if (MadtStructs[MadtStructsIndex] != NULL) {
> - FreePool (MadtStructs[MadtStructsIndex]);
> + if (MadtStructs != NULL){
> + for (MadtStructsIndex = 0; MadtStructsIndex < MaxMadtStructCount;
> MadtStructsIndex++) {
> + if (MadtStructs[MadtStructsIndex] != NULL) {
> + FreePool (MadtStructs[MadtStructsIndex]);
> + }
> }
> + FreePool (MadtStructs);
> }
> -
> - FreePool (MadtStructs);
> -
> +
> if (NewMadtTable != NULL) {
> FreePool (NewMadtTable);
> }
> --
> 2.18.0.windows.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [PATCH v2 2/2] MinPlatformPkg/AcpiTables: Add error handling to SortCpuLocalApicInTable
2019-09-12 3:27 ` [PATCH v2 2/2] MinPlatformPkg/AcpiTables: Add error handling to SortCpuLocalApicInTable Zhang, Shenglei
2019-09-12 4:06 ` Chiu, Chasel
@ 2019-09-12 6:17 ` Kubacki, Michael A
2019-09-12 18:36 ` Nate DeSimone
2 siblings, 0 replies; 10+ messages in thread
From: Kubacki, Michael A @ 2019-09-12 6:17 UTC (permalink / raw)
To: devel@edk2.groups.io, Zhang, Shenglei
Cc: Chiu, Chasel, Desimone, Nathaniel L, Gao, Liming
Reviewed-by: Michael Kubacki <michael.a.kubacki@intel.com>
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Zhang,
> Shenglei
> Sent: Wednesday, September 11, 2019 8:27 PM
> To: devel@edk2.groups.io
> Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>; Chiu, Chasel
> <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: [edk2-devel] [PATCH v2 2/2] MinPlatformPkg/AcpiTables: Add error
> handling to SortCpuLocalApicInTable
>
> Change ASSERT_EFI_ERROR to return value when the "if" statement is true.
> As a result, when SortCpuLocalApicInTable is called, error handling is needed.
> So add "if" statement to judge the returned Status from
> SortCpuLocalApicInTable () in function InstallMadtFromScratch().
>
> Cc: Michael Kubacki <michael.a.kubacki@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
> ---
> .../Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index 2cc55ee8..ae25d753 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> @@ -354,7 +354,7 @@ SortCpuLocalApicInTable (
>
> if(MAX_CPU_NUM <= Index) {
> DEBUG ((EFI_D_ERROR, "Asserting the SortCpuLocalApicInTable Index
> Bufferflow\n"));
> - ASSERT_EFI_ERROR(EFI_INVALID_PARAMETER);
> + return EFI_INVALID_PARAMETER;
> }
>
> TempVal = mCpuApicIdOrderTable[Index].ApicId;
> @@ -874,7 +874,11 @@ InstallMadtFromScratch (
> DetectApicIdMap();
>
> // Call for Local APIC ID Reorder
> - SortCpuLocalApicInTable ();
> + Status = SortCpuLocalApicInTable ();
> + if (EFI_ERROR (Status)) {
> + DEBUG ((EFI_D_ERROR, "SortCpuLocalApicInTable failed: %r\n", Status));
> + goto Done;
> + }
>
> MaxMadtStructCount = (UINT32) (
> MAX_CPU_NUM + // processor local APIC structures
> --
> 2.18.0.windows.1
>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [PATCH v2 1/2] MinPlatformPkg/AcpiTables: Initialize variables before used
2019-09-12 3:27 ` [PATCH v2 1/2] MinPlatformPkg/AcpiTables: Initialize variables before used Zhang, Shenglei
2019-09-12 4:05 ` Chiu, Chasel
2019-09-12 6:15 ` Kubacki, Michael A
@ 2019-09-12 18:35 ` Nate DeSimone
2 siblings, 0 replies; 10+ messages in thread
From: Nate DeSimone @ 2019-09-12 18:35 UTC (permalink / raw)
To: devel@edk2.groups.io, Zhang, Shenglei
Cc: Kubacki, Michael A, Chiu, Chasel, Gao, Liming
Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Zhang, Shenglei
Sent: Wednesday, September 11, 2019 8:27 PM
To: devel@edk2.groups.io
Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: [edk2-devel] [PATCH v2 1/2] MinPlatformPkg/AcpiTables: Initialize variables before used
MadtStructs, NewMadtTable and MaxMadtStructCount are not initialized before used or at the proper place. So assign values to them at the beginning and change the logic when freeing MadtStructs and NewMadtTable.
Cc: Michael Kubacki <michael.a.kubacki@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
---
.../Acpi/AcpiTables/AcpiPlatform.c | 21 +++++++++++--------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index 5eb72792..2cc55ee8 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -1,7 +1,7 @@
/** @file
ACPI Platform Driver
-Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2017-2019, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -867,13 +867,15 @@ InstallMadtFromScratch (
UINT32 PcIoApicMask;
UINTN PcIoApicIndex;
+ MadtStructs = NULL;
+ NewMadtTable = NULL;
+ MaxMadtStructCount = 0;
+
DetectApicIdMap();
// Call for Local APIC ID Reorder
SortCpuLocalApicInTable ();
- NewMadtTable = NULL;
-
MaxMadtStructCount = (UINT32) (
MAX_CPU_NUM + // processor local APIC structures
MAX_CPU_NUM + // processor local x2APIC structures
@@ -1115,14 +1117,15 @@ Done:
//
// Free memory
//
- for (MadtStructsIndex = 0; MadtStructsIndex < MaxMadtStructCount; MadtStructsIndex++) {
- if (MadtStructs[MadtStructsIndex] != NULL) {
- FreePool (MadtStructs[MadtStructsIndex]);
+ if (MadtStructs != NULL){
+ for (MadtStructsIndex = 0; MadtStructsIndex < MaxMadtStructCount; MadtStructsIndex++) {
+ if (MadtStructs[MadtStructsIndex] != NULL) {
+ FreePool (MadtStructs[MadtStructsIndex]);
+ }
}
+ FreePool (MadtStructs);
}
-
- FreePool (MadtStructs);
-
+
if (NewMadtTable != NULL) {
FreePool (NewMadtTable);
}
--
2.18.0.windows.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [PATCH v2 2/2] MinPlatformPkg/AcpiTables: Add error handling to SortCpuLocalApicInTable
2019-09-12 3:27 ` [PATCH v2 2/2] MinPlatformPkg/AcpiTables: Add error handling to SortCpuLocalApicInTable Zhang, Shenglei
2019-09-12 4:06 ` Chiu, Chasel
2019-09-12 6:17 ` [edk2-devel] " Kubacki, Michael A
@ 2019-09-12 18:36 ` Nate DeSimone
2 siblings, 0 replies; 10+ messages in thread
From: Nate DeSimone @ 2019-09-12 18:36 UTC (permalink / raw)
To: devel@edk2.groups.io, Zhang, Shenglei
Cc: Kubacki, Michael A, Chiu, Chasel, Gao, Liming
Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Zhang, Shenglei
Sent: Wednesday, September 11, 2019 8:27 PM
To: devel@edk2.groups.io
Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: [edk2-devel] [PATCH v2 2/2] MinPlatformPkg/AcpiTables: Add error handling to SortCpuLocalApicInTable
Change ASSERT_EFI_ERROR to return value when the "if" statement is true.
As a result, when SortCpuLocalApicInTable is called, error handling is needed. So add "if" statement to judge the returned Status from SortCpuLocalApicInTable () in function InstallMadtFromScratch().
Cc: Michael Kubacki <michael.a.kubacki@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
---
.../Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index 2cc55ee8..ae25d753 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -354,7 +354,7 @@ SortCpuLocalApicInTable (
if(MAX_CPU_NUM <= Index) {
DEBUG ((EFI_D_ERROR, "Asserting the SortCpuLocalApicInTable Index Bufferflow\n"));
- ASSERT_EFI_ERROR(EFI_INVALID_PARAMETER);
+ return EFI_INVALID_PARAMETER;
}
TempVal = mCpuApicIdOrderTable[Index].ApicId;
@@ -874,7 +874,11 @@ InstallMadtFromScratch (
DetectApicIdMap();
// Call for Local APIC ID Reorder
- SortCpuLocalApicInTable ();
+ Status = SortCpuLocalApicInTable ();
+ if (EFI_ERROR (Status)) {
+ DEBUG ((EFI_D_ERROR, "SortCpuLocalApicInTable failed: %r\n", Status));
+ goto Done;
+ }
MaxMadtStructCount = (UINT32) (
MAX_CPU_NUM + // processor local APIC structures
--
2.18.0.windows.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [PATCH v2 0/2] Add error handling and initialize variables
2019-09-12 3:27 [PATCH v2 0/2] Add error handling and initialize variables Zhang, Shenglei
2019-09-12 3:27 ` [PATCH v2 1/2] MinPlatformPkg/AcpiTables: Initialize variables before used Zhang, Shenglei
2019-09-12 3:27 ` [PATCH v2 2/2] MinPlatformPkg/AcpiTables: Add error handling to SortCpuLocalApicInTable Zhang, Shenglei
@ 2019-09-12 21:03 ` Nate DeSimone
2 siblings, 0 replies; 10+ messages in thread
From: Nate DeSimone @ 2019-09-12 21:03 UTC (permalink / raw)
To: devel@edk2.groups.io, Zhang, Shenglei
Cc: Kubacki, Michael A, Chiu, Chasel, Gao, Liming
The patch series has been pushed as 5dd12a154d.. 9ea86f187d
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Zhang, Shenglei
Sent: Wednesday, September 11, 2019 8:27 PM
To: devel@edk2.groups.io
Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: [edk2-devel] [PATCH v2 0/2] Add error handling and initialize variables
1.Add error handling to enhance status checking.
2.Initialize the variables before used and add check before
FreePool().
v2: Update copyright in 01/02.
Cc: Michael Kubacki <michael.a.kubacki@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Shenglei Zhang (2):
MinPlatformPkg/AcpiTables: Initialize variables before used
MinPlatformPkg/AcpiTables: Add error handling to
SortCpuLocalApicInTable
.../Acpi/AcpiTables/AcpiPlatform.c | 29 ++++++++++++-------
1 file changed, 18 insertions(+), 11 deletions(-)
--
2.18.0.windows.1
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-09-12 21:03 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-09-12 3:27 [PATCH v2 0/2] Add error handling and initialize variables Zhang, Shenglei
2019-09-12 3:27 ` [PATCH v2 1/2] MinPlatformPkg/AcpiTables: Initialize variables before used Zhang, Shenglei
2019-09-12 4:05 ` Chiu, Chasel
2019-09-12 6:15 ` Kubacki, Michael A
2019-09-12 18:35 ` [edk2-devel] " Nate DeSimone
2019-09-12 3:27 ` [PATCH v2 2/2] MinPlatformPkg/AcpiTables: Add error handling to SortCpuLocalApicInTable Zhang, Shenglei
2019-09-12 4:06 ` Chiu, Chasel
2019-09-12 6:17 ` [edk2-devel] " Kubacki, Michael A
2019-09-12 18:36 ` Nate DeSimone
2019-09-12 21:03 ` [edk2-devel] [PATCH v2 0/2] Add error handling and initialize variables Nate DeSimone
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox