* [PATCH 0/2] Add error handling and initialize variables
@ 2019-09-02 12:30 Zhang, Shenglei
2019-09-02 12:30 ` [PATCH 1/2] MinPlatformPkg/AcpiTables: Initialize variables before used Zhang, Shenglei
2019-09-02 12:30 ` [PATCH 2/2] MinPlatformPkg/AcpiTables: Add error handling to SortCpuLocalApicInTable Zhang, Shenglei
0 siblings, 2 replies; 6+ messages in thread
From: Zhang, Shenglei @ 2019-09-02 12:30 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().
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 | 27 ++++++++++++-------
1 file changed, 17 insertions(+), 10 deletions(-)
--
2.18.0.windows.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/2] MinPlatformPkg/AcpiTables: Initialize variables before used 2019-09-02 12:30 [PATCH 0/2] Add error handling and initialize variables Zhang, Shenglei @ 2019-09-02 12:30 ` Zhang, Shenglei 2019-09-02 13:48 ` Chiu, Chasel 2019-09-10 22:28 ` Nate DeSimone 2019-09-02 12:30 ` [PATCH 2/2] MinPlatformPkg/AcpiTables: Add error handling to SortCpuLocalApicInTable Zhang, Shenglei 1 sibling, 2 replies; 6+ messages in thread From: Zhang, Shenglei @ 2019-09-02 12:30 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 | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c index 5eb72792..85d1bd9a 100644 --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c @@ -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] 6+ messages in thread
* Re: [PATCH 1/2] MinPlatformPkg/AcpiTables: Initialize variables before used 2019-09-02 12:30 ` [PATCH 1/2] MinPlatformPkg/AcpiTables: Initialize variables before used Zhang, Shenglei @ 2019-09-02 13:48 ` Chiu, Chasel 2019-09-10 22:28 ` Nate DeSimone 1 sibling, 0 replies; 6+ messages in thread From: Chiu, Chasel @ 2019-09-02 13:48 UTC (permalink / raw) To: Zhang, Shenglei, devel@edk2.groups.io Cc: Kubacki, Michael A, Desimone, Nathaniel L, Gao, Liming Please extend copyright. With this update, Reviewed-by: Chasel Chiu <chasel.chiu@intel.com> > -----Original Message----- > From: Zhang, Shenglei > Sent: Monday, September 2, 2019 8:30 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 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 | 19 +++++++++++-------- > 1 file changed, 11 insertions(+), 8 deletions(-) > > diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c > b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c > index 5eb72792..85d1bd9a 100644 > --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c > +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c > @@ -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] 6+ messages in thread
* Re: [PATCH 1/2] MinPlatformPkg/AcpiTables: Initialize variables before used 2019-09-02 12:30 ` [PATCH 1/2] MinPlatformPkg/AcpiTables: Initialize variables before used Zhang, Shenglei 2019-09-02 13:48 ` Chiu, Chasel @ 2019-09-10 22:28 ` Nate DeSimone 1 sibling, 0 replies; 6+ messages in thread From: Nate DeSimone @ 2019-09-10 22:28 UTC (permalink / raw) To: Zhang, Shenglei, devel@edk2.groups.io Cc: Kubacki, Michael A, Chiu, Chasel, Gao, Liming Hi Shenglei, Please send a second patch series with the copyrights updated. Thanks, Nate -----Original Message----- From: Zhang, Shenglei Sent: Monday, September 2, 2019 5:30 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 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 | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c index 5eb72792..85d1bd9a 100644 --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c @@ -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] 6+ messages in thread
* [PATCH 2/2] MinPlatformPkg/AcpiTables: Add error handling to SortCpuLocalApicInTable 2019-09-02 12:30 [PATCH 0/2] Add error handling and initialize variables Zhang, Shenglei 2019-09-02 12:30 ` [PATCH 1/2] MinPlatformPkg/AcpiTables: Initialize variables before used Zhang, Shenglei @ 2019-09-02 12:30 ` Zhang, Shenglei 2019-09-02 13:54 ` Chiu, Chasel 1 sibling, 1 reply; 6+ messages in thread From: Zhang, Shenglei @ 2019-09-02 12:30 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 85d1bd9a..dc68dfaa 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] 6+ messages in thread
* Re: [PATCH 2/2] MinPlatformPkg/AcpiTables: Add error handling to SortCpuLocalApicInTable 2019-09-02 12:30 ` [PATCH 2/2] MinPlatformPkg/AcpiTables: Add error handling to SortCpuLocalApicInTable Zhang, Shenglei @ 2019-09-02 13:54 ` Chiu, Chasel 0 siblings, 0 replies; 6+ messages in thread From: Chiu, Chasel @ 2019-09-02 13:54 UTC (permalink / raw) To: Zhang, Shenglei, devel@edk2.groups.io Cc: Kubacki, Michael A, Desimone, Nathaniel L, Gao, Liming Same with 1/2 patch, please extend copyright. With above update, Reviewed-by: Chasel Chiu <chasel.chiu@intel.com> > -----Original Message----- > From: Zhang, Shenglei > Sent: Monday, September 2, 2019 8:30 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 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 85d1bd9a..dc68dfaa 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] 6+ messages in thread
end of thread, other threads:[~2019-09-10 22:28 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-09-02 12:30 [PATCH 0/2] Add error handling and initialize variables Zhang, Shenglei 2019-09-02 12:30 ` [PATCH 1/2] MinPlatformPkg/AcpiTables: Initialize variables before used Zhang, Shenglei 2019-09-02 13:48 ` Chiu, Chasel 2019-09-10 22:28 ` Nate DeSimone 2019-09-02 12:30 ` [PATCH 2/2] MinPlatformPkg/AcpiTables: Add error handling to SortCpuLocalApicInTable Zhang, Shenglei 2019-09-02 13:54 ` Chiu, Chasel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox