Hi Zhichao, Reviewed-by: Eric Dong <eric.d...@intel.com>
It's better to add some comments in the code to explain the change which make the code easy to be understood. Thanks, Eric > -----Original Message----- > From: Gao, Zhichao > Sent: Tuesday, June 25, 2019 11:16 PM > To: devel@edk2.groups.io > Cc: Dong, Eric <eric.d...@intel.com>; Ni, Ray <ray...@intel.com>; Laszlo > Ersek <ler...@redhat.com>; Gao, Liming <liming....@intel.com> > Subject: [PATCH V2] UefiCpuPkg/MpInitLib: MicrocodeDetect: Ensure > checked range is valid > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1934 > > V1: > Originally, the checksum part would done before verfiy the microcode data. > Which meas the checksum would be done for a meaningless data. > It would cause a incorrect TotalSize (the size of microcode data), then > incorrect checksum and incorrect pointer increasing would happen. > To fix this, move the checksum part 1 section in 'if (MicrocodeEntryPoint- > >HeaderVersion == 0x1)' section for a valid microcode data. > > V2: > 'if (MicrocodeEntryPoint->HeaderVersion == 0x1)' condition doesn't make > sure the entry data is a valid microcode. So abandon it. Instead, make sure > the checked data is in the microcode data range. Because the DataSize of non > microcde data may make (MicrocodeEntryPoint + TotalSize) larger than > 0xffffffff. For PEI driver, UINTN is 32bit and the result is overflow and it > may > be a very small value. That means the checksum check would be done out of > the microcode range. > > Cc: Eric Dong <eric.d...@intel.com> > Cc: Ray Ni <ray...@intel.com> > Cc: Laszlo Ersek <ler...@redhat.com> > Cc: Liming Gao <liming....@intel.com> > Signed-off-by: Zhichao Gao <zhichao....@intel.com> > --- > UefiCpuPkg/Library/MpInitLib/Microcode.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c > b/UefiCpuPkg/Library/MpInitLib/Microcode.c > index 4763dcfebe..6c0995cb0d 100644 > --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c > +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c > @@ -1,7 +1,7 @@ > /** @file > Implementation of loading microcode on processors. > > - Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR> > + Copyright (c) 2015 - 2019, Intel Corporation. All rights > + reserved.<BR> > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > @@ -170,6 +170,7 @@ MicrocodeDetect ( > /// Check overflow and whether TotalSize is aligned with 4 bytes. > /// > if ( ((UINTN)MicrocodeEntryPoint + TotalSize) > MicrocodeEnd || > + ((UINTN)MicrocodeEntryPoint + TotalSize) < (UINTN) > + CpuMpData->MicrocodePatchAddress || > (TotalSize & 0x3) != 0 > ) { > MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (((UINTN) > MicrocodeEntryPoint) + SIZE_1KB); > -- > 2.21.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#42842): https://edk2.groups.io/g/devel/message/42842 Mute This Topic: https://groups.io/mt/32204622/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
