From: Michael Kubacki <michael.kuba...@microsoft.com> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2869
Makes some minor improvements to function parameter validation in FmpDxe, in particular to externally exposed functions such as those that back EFI_FIRMWARE_MANAGEMENT_PROTOCOL. Cc: Liming Gao <liming....@intel.com> Cc: Michael D Kinney <michael.d.kin...@intel.com> Signed-off-by: Michael Kubacki <michael.kuba...@microsoft.com> --- FmpDevicePkg/FmpDxe/FmpDxe.c | 56 +++++++++++++++++--- FmpDevicePkg/FmpDxe/FmpDxe.h | 10 ++-- 2 files changed, 54 insertions(+), 12 deletions(-) diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c b/FmpDevicePkg/FmpDxe/FmpDxe.c index a3e342591936..958d9b394b71 100644 --- a/FmpDevicePkg/FmpDxe/FmpDxe.c +++ b/FmpDevicePkg/FmpDxe/FmpDxe.c @@ -278,6 +278,11 @@ PopulateDescriptor ( EFI_STATUS Status; UINT32 DependenciesSize; + if (Private == NULL) { + DEBUG ((DEBUG_ERROR, "FmpDxe(%s): PopulateDescriptor() - Private is NULL.\n", mImageIdName)); + return; + } + if (Private->DescriptorPopulated) { return; } @@ -429,7 +434,7 @@ PopulateDescriptor ( @retval EFI_SUCCESS The device was successfully updated with the new image. @retval EFI_BUFFER_TOO_SMALL The ImageInfo buffer was too small. The current buffer size needed to hold the image(s) information is returned in ImageInfoSize. - @retval EFI_INVALID_PARAMETER ImageInfoSize is NULL. + @retval EFI_INVALID_PARAMETER A required pointer is NULL. @retval EFI_DEVICE_ERROR Valid information could not be returned. Possible corrupted image. **/ @@ -451,6 +456,12 @@ GetTheImageInfo ( Status = EFI_SUCCESS; + if (This == NULL) { + DEBUG ((DEBUG_ERROR, "FmpDxe(%s): GetImageInfo() - This is NULL.\n", mImageIdName)); + Status = EFI_INVALID_PARAMETER; + goto cleanup; + } + // // Retrieve the private context structure // @@ -536,7 +547,7 @@ GetTheImageInfo ( @retval EFI_BUFFER_TOO_SMALL The buffer specified by ImageSize is too small to hold the image. The current buffer size needed to hold the image is returned in ImageSize. - @retval EFI_INVALID_PARAMETER The Image was NULL. + @retval EFI_INVALID_PARAMETER A required pointer is NULL or ImageIndex is invalid. @retval EFI_NOT_FOUND The current image is not copied to the buffer. @retval EFI_UNSUPPORTED The operation is not supported. @retval EFI_SECURITY_VIOLATION The operation could not be performed due to an authentication failure. @@ -561,6 +572,12 @@ GetTheImage ( Status = EFI_SUCCESS; + if (This == NULL) { + DEBUG ((DEBUG_ERROR, "FmpDxe(%s): GetImage() - This is NULL.\n", mImageIdName)); + Status = EFI_INVALID_PARAMETER; + goto cleanup; + } + // // Retrieve the private context structure // @@ -615,7 +632,8 @@ GetTheImage ( @param[in] Image Pointer to the image. @param[in] ImageSize Size of the image. @param[in] AdditionalHeaderSize Size of any headers that cannot be calculated by this function. - @param[out] PayloadSize + @param[out] PayloadSize An optional pointer to a UINTN that holds the size of the payload + (image size minus headers) @retval !NULL Valid pointer to the header. @retval NULL Structure is bad and pointer cannot be found. @@ -626,7 +644,7 @@ GetFmpHeader ( IN CONST EFI_FIRMWARE_IMAGE_AUTHENTICATION *Image, IN CONST UINTN ImageSize, IN CONST UINTN AdditionalHeaderSize, - OUT UINTN *PayloadSize + OUT UINTN *PayloadSize OPTIONAL ) { // @@ -640,7 +658,10 @@ GetFmpHeader ( return NULL; } - *PayloadSize = ImageSize - (sizeof (Image->MonotonicCount) + Image->AuthInfo.Hdr.dwLength + AdditionalHeaderSize); + if (PayloadSize != NULL) { + *PayloadSize = ImageSize - (sizeof (Image->MonotonicCount) + Image->AuthInfo.Hdr.dwLength + AdditionalHeaderSize); + } + return (VOID *)((UINT8 *)Image + sizeof (Image->MonotonicCount) + Image->AuthInfo.Hdr.dwLength + AdditionalHeaderSize); } @@ -663,6 +684,10 @@ GetAllHeaderSize ( { UINT32 CalculatedSize; + if (Image == NULL) { + return 0; + } + CalculatedSize = sizeof (Image->MonotonicCount) + AdditionalHeaderSize + Image->AuthInfo.Hdr.dwLength; @@ -698,7 +723,7 @@ GetAllHeaderSize ( @retval EFI_SUCCESS The image was successfully checked. @retval EFI_ABORTED The operation is aborted. - @retval EFI_INVALID_PARAMETER The Image was NULL. + @retval EFI_INVALID_PARAMETER A required pointer is NULL. @retval EFI_UNSUPPORTED The operation is not supported. @retval EFI_SECURITY_VIOLATION The operation could not be performed due to an authentication failure. @@ -743,6 +768,12 @@ CheckTheImage ( return EFI_UNSUPPORTED; } + if (This == NULL) { + DEBUG ((DEBUG_ERROR, "FmpDxe(%s): CheckImage() - This is NULL.\n", mImageIdName)); + Status = EFI_INVALID_PARAMETER; + goto cleanup; + } + // // Retrieve the private context structure // @@ -978,7 +1009,7 @@ CheckTheImage ( @retval EFI_SUCCESS The device was successfully updated with the new image. @retval EFI_ABORTED The operation is aborted. - @retval EFI_INVALID_PARAMETER The Image was NULL. + @retval EFI_INVALID_PARAMETER A required pointer is NULL. @retval EFI_UNSUPPORTED The operation is not supported. @retval EFI_SECURITY_VIOLATION The operation could not be performed due to an authentication failure. @@ -1026,6 +1057,12 @@ SetTheImage ( return EFI_UNSUPPORTED; } + if (This == NULL) { + DEBUG ((DEBUG_ERROR, "FmpDxe(%s): SetTheImage() - This is NULL.\n", mImageIdName)); + Status = EFI_INVALID_PARAMETER; + goto cleanup; + } + // // Retrieve the private context structure // @@ -1382,6 +1419,11 @@ FmpDxeLockEventNotify ( EFI_STATUS Status; FIRMWARE_MANAGEMENT_PRIVATE_DATA *Private; + if (Context == NULL) { + ASSERT (Context != NULL); + return; + } + Private = (FIRMWARE_MANAGEMENT_PRIVATE_DATA *)Context; if (!Private->FmpDeviceLocked) { diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.h b/FmpDevicePkg/FmpDxe/FmpDxe.h index 30754dea495e..4dfec316a558 100644 --- a/FmpDevicePkg/FmpDxe/FmpDxe.h +++ b/FmpDevicePkg/FmpDxe/FmpDxe.h @@ -3,7 +3,7 @@ image stored in a firmware device with platform and firmware device specific information provided through PCDs and libraries. - Copyright (c) 2016, Microsoft Corporation. All rights reserved.<BR> + Copyright (c) Microsoft Corporation.<BR> Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.<BR> SPDX-License-Identifier: BSD-2-Clause-Patent @@ -132,7 +132,7 @@ DetectTestKey ( @retval EFI_SUCCESS The device was successfully updated with the new image. @retval EFI_BUFFER_TOO_SMALL The ImageInfo buffer was too small. The current buffer size needed to hold the image(s) information is returned in ImageInfoSize. - @retval EFI_INVALID_PARAMETER ImageInfoSize is NULL. + @retval EFI_INVALID_PARAMETER A required pointer is NULL. @retval EFI_DEVICE_ERROR Valid information could not be returned. Possible corrupted image. **/ @@ -166,7 +166,7 @@ GetTheImageInfo ( @retval EFI_BUFFER_TOO_SMALL The buffer specified by ImageSize is too small to hold the image. The current buffer size needed to hold the image is returned in ImageSize. - @retval EFI_INVALID_PARAMETER The Image was NULL. + @retval EFI_INVALID_PARAMETER A required pointer is NULL or ImageIndex is invalid. @retval EFI_NOT_FOUND The current image is not copied to the buffer. @retval EFI_UNSUPPORTED The operation is not supported. @retval EFI_SECURITY_VIOLATION The operation could not be performed due to an authentication failure. @@ -198,7 +198,7 @@ GetTheImage ( @retval EFI_SUCCESS The image was successfully checked. @retval EFI_ABORTED The operation is aborted. - @retval EFI_INVALID_PARAMETER The Image was NULL. + @retval EFI_INVALID_PARAMETER A required pointer is NULL. @retval EFI_UNSUPPORTED The operation is not supported. @retval EFI_SECURITY_VIOLATION The operation could not be performed due to an authentication failure. @@ -254,7 +254,7 @@ CheckTheImage ( @retval EFI_SUCCESS The device was successfully updated with the new image. @retval EFI_ABORTED The operation is aborted. - @retval EFI_INVALID_PARAMETER The Image was NULL. + @retval EFI_INVALID_PARAMETER A required pointer is NULL. @retval EFI_UNSUPPORTED The operation is not supported. @retval EFI_SECURITY_VIOLATION The operation could not be performed due to an authentication failure. -- 2.27.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#63542): https://edk2.groups.io/g/devel/message/63542 Mute This Topic: https://groups.io/mt/75900913/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-