From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.126, mailfrom: chasel.chiu@intel.com) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by groups.io with SMTP; Mon, 26 Aug 2019 00:28:29 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 26 Aug 2019 00:28:27 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,431,1559545200"; d="scan'208";a="196991745" Received: from kmsmsx151.gar.corp.intel.com ([172.21.73.86]) by fmsmga001.fm.intel.com with ESMTP; 26 Aug 2019 00:28:26 -0700 Received: from pgsmsx111.gar.corp.intel.com ([169.254.2.22]) by KMSMSX151.gar.corp.intel.com ([169.254.10.91]) with mapi id 14.03.0439.000; Mon, 26 Aug 2019 15:28:22 +0800 From: "Chiu, Chasel" To: "devel@edk2.groups.io" , "Kubacki, Michael A" CC: "Desimone, Nathaniel L" , "Gao, Liming" Subject: Re: [edk2-devel] [edk2-platforms][PATCH V1 1/1] MinPlatformPkg/MultiBoardInitSupportLib: Fix NULL pointer dereferences Thread-Topic: [edk2-devel] [edk2-platforms][PATCH V1 1/1] MinPlatformPkg/MultiBoardInitSupportLib: Fix NULL pointer dereferences Thread-Index: AQHVWgnt0nXyvACnREOWOYQOxgoeRqcNC4Ag Date: Mon, 26 Aug 2019 07:28:21 +0000 Message-ID: <3C3EFB470A303B4AB093197B6777CCEC5046A8E8@PGSMSX111.gar.corp.intel.com> References: <20190823232420.20184-1-michael.a.kubacki@intel.com> In-Reply-To: <20190823232420.20184-1-michael.a.kubacki@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNDQ3ZjQyMDItYzE3Zi00ZjZjLTkxMzktMTg1YTA5NWM3OGRhIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiKzdVM3ZxYnZxajg0SE5LM3p5S1NZYmxpcGpDNzJcL0NYSEx6Z2tpZmFaU3dQQVRvKzFxSFJmVnVEOWduSkxpYlIifQ== x-ctpclassification: CTP_NT x-originating-ip: [172.30.20.206] MIME-Version: 1.0 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Chasel Chiu > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Kubacki, Michael A > Sent: Saturday, August 24, 2019 7:24 AM > To: devel@edk2.groups.io > Cc: Chiu, Chasel ; Desimone, Nathaniel L > ; Gao, Liming > Subject: [edk2-devel] [edk2-platforms][PATCH V1 1/1] > MinPlatformPkg/MultiBoardInitSupportLib: Fix NULL pointer dereferences >=20 > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D2114 >=20 > Removes potential NULL pointer de-references in the library and validate= s > NULL pointers are not passed to library functions. >=20 > Cc: Chasel Chiu > Cc: Nate DeSimone > Cc: Liming Gao > Signed-off-by: Michael Kubacki > --- > Platform/Intel/MinPlatformPkg/Include/Library/MultiBoardInitSupportLib.= h > | 84 ++++++++++++++++- >=20 > Platform/Intel/MinPlatformPkg/PlatformInit/Library/MultiBoardInitSupport= L > ib/DxeMultiBoardInitSupportLib.c | 24 ++++- > Platform/Intel/MinPlatformPkg/PlatformInit/Library/MultiBoardInitSupport= L > ib/PeiMultiBoardInitSupportLib.c | 97 +++++++++++++++++--- > 3 files changed, 188 insertions(+), 17 deletions(-) >=20 > diff --git > a/Platform/Intel/MinPlatformPkg/Include/Library/MultiBoardInitSupportLib= . > h > b/Platform/Intel/MinPlatformPkg/Include/Library/MultiBoardInitSupportLib= . > h > index 6c14b5677d..a854f61e27 100644 > --- > a/Platform/Intel/MinPlatformPkg/Include/Library/MultiBoardInitSupportLib= . > h > +++ b/Platform/Intel/MinPlatformPkg/Include/Library/MultiBoardInitSuppor > +++ tLib.h > @@ -1,6 +1,6 @@ > /** @file >=20 > -Copyright (c) 2017, Intel Corporation. All rights reserved.
> +Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.
> SPDX-License-Identifier: BSD-2-Clause-Patent >=20 > **/ > @@ -10,18 +10,39 @@ SPDX-License-Identifier: BSD-2-Clause-Patent >=20 > #include >=20 > +/** > + This board service detects the board type. > + > + @retval EFI_SUCCESS The board was detected successfully. > + @retval EFI_NOT_FOUND The board could not be detected. > + > +**/ > typedef > EFI_STATUS > (EFIAPI *BOARD_DETECT) ( > VOID > ); >=20 > +/** > + This board service performs board-specific initialization. > + > + @retval EFI_SUCCESS Board-specific initialization was successful. > + @retval EFI_NOT_READY The board has not been detected yet. > + > +**/ > typedef > EFI_STATUS > (EFIAPI *BOARD_INIT) ( > VOID > ); >=20 > +/** > + This board service detects the boot mode. > + > + @retval EFI_BOOT_MODE The boot mode. > + @retval EFI_NOT_READY The board has not been detected yet. > + > +**/ > typedef > EFI_BOOT_MODE > (EFIAPI *BOARD_BOOT_MODE_DETECT) ( > @@ -52,24 +73,85 @@ typedef struct { > BOARD_INIT BoardInitEndOfFirmware; > } BOARD_NOTIFICATION_INIT_FUNC; >=20 > +/** > + Registers the given function for callback during board detection. > + > + When this function is called the given function pointer is added to > + an internal list. When board detection is performed within the > + BoardDetect() API, the function pointers in the list will be invoked u= ntil a > board detection function reports it has successfully detected the board= . > + > + @param[in] BoardDetect A pointer to a function of type > BOARD_DETECT_FUNC that is called during > + board detection. > + > + @retval EFI_SUCCESS The function was successfully > registered. > + @retval EFI_INVALID_PARAMETER The function pointer given is N= ULL. > + @retval EFI_OUT_OF_RESOURCES Insufficient memory resources > exist to add a new board detection callback. > + > +**/ > EFI_STATUS > EFIAPI > RegisterBoardDetect ( > IN BOARD_DETECT_FUNC *BoardDetect > ); >=20 > +/** > + Registers the given set of board functions for callback to perform > pre-memory board initialization tasks. > + > + When this function is called, the given structure of function > + pointers are stored for future invocation. When board pre-memory > + initialization tasks are required, the corresponding pre-memory > + function in this structure will be called. Typically, RegisterBoardPr= eMemInit() > is called during board detection with the successfully detected board > providing its set of board-specific pre-memory initialization functions. > + > + @param[in] BoardPreMemInit A pointer to a structure of fun= ction > pointers described in the type > + BOARD_PRE_MEM_INIT_FUNC. > + > + @retval EFI_SUCCESS The board pre-memory functions = were > successfully registered. > + @retval EFI_INVALID_PARAMETER The pointer given is NULL. > + @retval EFI_OUT_OF_RESOURCES Insufficient memory resources > exist to register the callback functions. > + > +**/ > EFI_STATUS > EFIAPI > RegisterBoardPreMemInit ( > IN BOARD_PRE_MEM_INIT_FUNC *BoardPreMemInit > ); >=20 > +/** > + Registers the given set of board functions for callback to perform > post-memory board initialization tasks. > + > + When this function is called, the given structure of function > + pointers are stored for future invocation. When board post-memory > + initialization tasks are required, the corresponding post-memory > + function in this structure will be called. Typically, > RegisterBoardPostMemInit() is called during board detection with the > successfuly detected board providing its set of board-specific post-mem= ory > initialization functions. > + > + @param[in] BoardPostMemInit A pointer to a structure of fun= ction > pointers described in the type > + BOARD_POST_MEM_INIT_FUNC. > + > + @retval EFI_SUCCESS The board post-memory functions= were > successfully registered. > + @retval EFI_INVALID_PARAMETER The pointer given is NULL. > + @retval EFI_OUT_OF_RESOURCES Insufficient memory resources > exist to register the callback functions. > + > +**/ > EFI_STATUS > EFIAPI > RegisterBoardPostMemInit ( > IN BOARD_POST_MEM_INIT_FUNC *BoardPostMemInit > ); >=20 > +/** > + Registers the given set of board functions for callback to perform bo= ard > initialization tasks. > + > + When this function is called, the given structure of function > + pointers are stored for future invocation. When board initialization = tasks > are required, the corresponding functions in this structure will be call= ed. > + > + @param[in] BoardNotificationInit A pointer to a structure of fun= ction > pointers described in the type > + BOARD_NOTIFICATION_INIT_FUNC. > + > + @retval EFI_SUCCESS The board notification function= s were > successfully registered. > + @retval EFI_INVALID_PARAMETER The pointer given is NULL. > + @retval EFI_OUT_OF_RESOURCES Insufficient memory resources > exist to register the callback functions. > + > +**/ > EFI_STATUS > EFIAPI > RegisterBoardNotificationInit ( > diff --git > a/Platform/Intel/MinPlatformPkg/PlatformInit/Library/MultiBoardInitSuppo > rtLib/DxeMultiBoardInitSupportLib.c > b/Platform/Intel/MinPlatformPkg/PlatformInit/Library/MultiBoardInitSuppo > rtLib/DxeMultiBoardInitSupportLib.c > index 90aabdbbc7..3eb3a1adc7 100644 > --- > a/Platform/Intel/MinPlatformPkg/PlatformInit/Library/MultiBoardInitSuppo > rtLib/DxeMultiBoardInitSupportLib.c > +++ b/Platform/Intel/MinPlatformPkg/PlatformInit/Library/MultiBoardInitS > +++ upportLib/DxeMultiBoardInitSupportLib.c > @@ -1,6 +1,6 @@ > /** @file >=20 > -Copyright (c) 2017, Intel Corporation. All rights reserved.
> +Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.
> SPDX-License-Identifier: BSD-2-Clause-Patent >=20 > **/ > @@ -11,6 +11,20 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #includ= e > #include >=20 > +/** > + Registers the given set of board functions for callback to perform bo= ard > initialization tasks. > + > + When this function is called, the given structure of function > + pointers are stored for future invocation. When board initialization = tasks > are required, the corresponding functions in this structure will be call= ed. > + > + @param[in] BoardNotificationInit A pointer to a structure of fun= ction > pointers described in the type > + BOARD_NOTIFICATION_INIT_FUNC. > + > + @retval EFI_SUCCESS The board notification function= s were > successfully registered. > + @retval EFI_INVALID_PARAMETER The pointer given is NULL. > + @retval EFI_OUT_OF_RESOURCES Insufficient memory resources > exist to register the callback functions. > + > +**/ > EFI_STATUS > EFIAPI > RegisterBoardNotificationInit ( > @@ -20,6 +34,10 @@ RegisterBoardNotificationInit ( > EFI_HANDLE Handle; > EFI_STATUS Status; >=20 > + if (BoardNotificationInit =3D=3D NULL) { > + return EFI_INVALID_PARAMETER; > + } > + > Handle =3D NULL; > Status =3D gBS->InstallProtocolInterface ( > &Handle, > @@ -27,7 +45,7 @@ RegisterBoardNotificationInit ( > EFI_NATIVE_INTERFACE, > BoardNotificationInit > ); > - ASSERT_EFI_ERROR(Status); > + ASSERT_EFI_ERROR (Status); >=20 > - return EFI_SUCCESS; > + return Status; > } > diff --git > a/Platform/Intel/MinPlatformPkg/PlatformInit/Library/MultiBoardInitSuppo > rtLib/PeiMultiBoardInitSupportLib.c > b/Platform/Intel/MinPlatformPkg/PlatformInit/Library/MultiBoardInitSuppo > rtLib/PeiMultiBoardInitSupportLib.c > index f1cd735e41..6f5b90d9b1 100644 > --- > a/Platform/Intel/MinPlatformPkg/PlatformInit/Library/MultiBoardInitSuppo > rtLib/PeiMultiBoardInitSupportLib.c > +++ b/Platform/Intel/MinPlatformPkg/PlatformInit/Library/MultiBoardInitS > +++ upportLib/PeiMultiBoardInitSupportLib.c > @@ -1,6 +1,6 @@ > /** @file >=20 > -Copyright (c) 2017, Intel Corporation. All rights reserved.
> +Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.
> SPDX-License-Identifier: BSD-2-Clause-Patent >=20 > **/ > @@ -12,6 +12,21 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #includ= e > #include >=20 > +/** > + Registers the given function for callback during board detection. > + > + When this function is called the given function pointer is added to > + an internal list. When board detection is performed within the > + BoardDetect() API, the function pointers in the list will be invoked u= ntil a > board detection function reports it has successfully detected the board= . > + > + @param[in] BoardDetect A pointer to a function of type > BOARD_DETECT_FUNC that is called during > + board detection. > + > + @retval EFI_SUCCESS The function was successfully > registered. > + @retval EFI_INVALID_PARAMETER The function pointer given is N= ULL. > + @retval EFI_OUT_OF_RESOURCES Insufficient memory resources > exist to add a new board detection callback. > + > +**/ > EFI_STATUS > EFIAPI > RegisterBoardDetect ( > @@ -21,18 +36,42 @@ RegisterBoardDetect ( > EFI_STATUS Status; > EFI_PEI_PPI_DESCRIPTOR *PpiListBoardDetect; >=20 > - PpiListBoardDetect =3D AllocatePool (sizeof(EFI_PEI_PPI_DESCRIPTOR)); > - ASSERT (PpiListBoardDetect !=3D NULL); > + if (BoardDetect =3D=3D NULL) { > + return EFI_INVALID_PARAMETER; > + } > + > + PpiListBoardDetect =3D AllocatePool (sizeof (EFI_PEI_PPI_DESCRIPTOR))= ; > + if (PpiListBoardDetect =3D=3D NULL) { > + ASSERT (PpiListBoardDetect !=3D NULL); > + return EFI_OUT_OF_RESOURCES; > + } >=20 > PpiListBoardDetect->Flags =3D (EFI_PEI_PPI_DESCRIPTOR_PPI | > EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST); > PpiListBoardDetect->Guid =3D &gBoardDetectGuid; > PpiListBoardDetect->Ppi =3D BoardDetect; >=20 > Status =3D PeiServicesInstallPpi (PpiListBoardDetect); > - ASSERT_EFI_ERROR(Status); > - return EFI_SUCCESS; > + ASSERT_EFI_ERROR (Status); > + > + return Status; > } >=20 > +/** > + Registers the given set of board functions for callback to perform > pre-memory board initialization tasks. > + > + When this function is called, the given structure of function > + pointers are stored for future invocation. When board pre-memory > + initialization tasks are required, the corresponding pre-memory > + function in this structure will be called. Typically, RegisterBoardPr= eMemInit() > is called during board detection with the successfully detected board > providing its set of board-specific pre-memory initialization functions. > + > + @param[in] BoardPreMemInit A pointer to a structure of fun= ction > pointers described in the type > + BOARD_PRE_MEM_INIT_FUNC. > + > + @retval EFI_SUCCESS The board pre-memory functions = were > successfully registered. > + @retval EFI_INVALID_PARAMETER The pointer given is NULL. > + @retval EFI_OUT_OF_RESOURCES Insufficient memory resources > exist to register the callback functions. > + > +**/ > EFI_STATUS > EFIAPI > RegisterBoardPreMemInit ( > @@ -42,18 +81,42 @@ RegisterBoardPreMemInit ( > EFI_STATUS Status; > EFI_PEI_PPI_DESCRIPTOR *PpiListBoardInitPreMem; >=20 > - PpiListBoardInitPreMem =3D AllocatePool (sizeof(EFI_PEI_PPI_DESCRIPTO= R)); > - ASSERT (PpiListBoardInitPreMem !=3D NULL); > + if (BoardPreMemInit =3D=3D NULL) { > + return EFI_OUT_OF_RESOURCES; > + } > + > + PpiListBoardInitPreMem =3D AllocatePool (sizeof > + (EFI_PEI_PPI_DESCRIPTOR)); if (PpiListBoardInitPreMem =3D=3D NULL) { > + ASSERT (PpiListBoardInitPreMem !=3D NULL); > + return EFI_OUT_OF_RESOURCES; > + } >=20 > PpiListBoardInitPreMem->Flags =3D (EFI_PEI_PPI_DESCRIPTOR_PPI | > EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST); > PpiListBoardInitPreMem->Guid =3D &gBoardPreMemInitGuid; > PpiListBoardInitPreMem->Ppi =3D BoardPreMemInit; >=20 > Status =3D PeiServicesInstallPpi (PpiListBoardInitPreMem); > - ASSERT_EFI_ERROR(Status); > - return EFI_SUCCESS; > + ASSERT_EFI_ERROR (Status); > + > + return Status; > } >=20 > +/** > + Registers the given set of board functions for callback to perform > post-memory board initialization tasks. > + > + When this function is called, the given structure of function > + pointers are stored for future invocation. When board post-memory > + initialization tasks are required, the corresponding post-memory > + function in this structure will be called. Typically, > RegisterBoardPostMemInit() is called during board detection with the > successfuly detected board providing its set of board-specific post-mem= ory > initialization functions. > + > + @param[in] BoardPostMemInit A pointer to a structure of fun= ction > pointers described in the type > + BOARD_POST_MEM_INIT_FUNC. > + > + @retval EFI_SUCCESS The board post-memory functions= were > successfully registered. > + @retval EFI_INVALID_PARAMETER The pointer given is NULL. > + @retval EFI_OUT_OF_RESOURCES Insufficient memory resources > exist to register the callback functions. > + > +**/ > EFI_STATUS > EFIAPI > RegisterBoardPostMemInit ( > @@ -63,14 +126,22 @@ RegisterBoardPostMemInit ( > EFI_STATUS Status; > EFI_PEI_PPI_DESCRIPTOR *PpiListBoardInitPostMem; >=20 > - PpiListBoardInitPostMem =3D AllocatePool (sizeof(EFI_PEI_PPI_DESCRIPT= OR)); > - ASSERT (PpiListBoardInitPostMem !=3D NULL); > + if (BoardPostMemInit =3D=3D NULL) { > + return EFI_INVALID_PARAMETER; > + } > + > + PpiListBoardInitPostMem =3D AllocatePool (sizeof > + (EFI_PEI_PPI_DESCRIPTOR)); if (PpiListBoardInitPostMem =3D=3D NULL) { > + ASSERT (PpiListBoardInitPostMem !=3D NULL); > + return EFI_OUT_OF_RESOURCES; > + } >=20 > PpiListBoardInitPostMem->Flags =3D (EFI_PEI_PPI_DESCRIPTOR_PPI | > EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST); > PpiListBoardInitPostMem->Guid =3D &gBoardPostMemInitGuid; > PpiListBoardInitPostMem->Ppi =3D BoardPostMemInit; >=20 > Status =3D PeiServicesInstallPpi (PpiListBoardInitPostMem); > - ASSERT_EFI_ERROR(Status); > - return EFI_SUCCESS; > + ASSERT_EFI_ERROR (Status); > + > + return Status; > } > -- > 2.16.2.windows.1 >=20 >=20 >=20