From 772c5bb8dcb9841f1839dec02f33324e31b36d25 Mon Sep 17 00:00:00 2001 From: Michael Kubacki Date: Wed, 5 Jan 2022 04:38:24 +0800 Subject: FmpDevicePkg/FmpDxe: Update FmpDeviceCheckImageWithStatus() handling Update the logic handling last attempt status codes from FmpDeviceCheckImageWithStatus() implementations to account for cases when the function return status code is EFI_SUCCESS (since the image was checked successfully) but the ImageUpdatable value is not valid. In addition the following sentence is removed from the LastAttemptStatus parameter definition for FmpDeviceCheckImageWithStatus() since it can lead to confusion. The expected status code value range is sufficient to implement the library API. "This value will only be checked when this function returns an error." Cc: Liming Gao Cc: Michael D Kinney Cc: Guomin Jiang Cc: Wei6 Xu Signed-off-by: Michael Kubacki Reviewed-by: Guomin Jiang --- FmpDevicePkg/FmpDxe/FmpDxe.c | 23 ++++++++++++++++------ FmpDevicePkg/Include/Library/FmpDeviceLib.h | 3 +-- .../Library/FmpDeviceLibNull/FmpDeviceLib.c | 3 +-- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c b/FmpDevicePkg/FmpDxe/FmpDxe.c index 197df28c8d..1e7ec4a09e 100644 --- a/FmpDevicePkg/FmpDxe/FmpDxe.c +++ b/FmpDevicePkg/FmpDxe/FmpDxe.c @@ -1040,8 +1040,19 @@ CheckTheImageInternal ( // Status = FmpDeviceCheckImageWithStatus ((((UINT8 *)Image) + AllHeaderSize), RawSize, ImageUpdatable, LastAttemptStatus); if (EFI_ERROR (Status)) { + // The image cannot be valid if an error occurred checking the image + if (*ImageUpdatable == IMAGE_UPDATABLE_VALID) { + *ImageUpdatable = IMAGE_UPDATABLE_INVALID; + } + DEBUG ((DEBUG_ERROR, "FmpDxe(%s): CheckTheImage() - FmpDeviceLib CheckImage failed. Status = %r\n", mImageIdName, Status)); + } + // + // Only validate the library last attempt status code if the image is not updatable. + // This specifically avoids converting LAST_ATTEMPT_STATUS_SUCCESS if it set for an updatable image. + // + if (*ImageUpdatable != IMAGE_UPDATABLE_VALID) { // // LastAttemptStatus returned from the device library should fall within the designated error range // [LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE, LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE] @@ -1049,12 +1060,12 @@ CheckTheImageInternal ( if ((*LastAttemptStatus < LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE) || (*LastAttemptStatus > LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE)) { - DEBUG ( - (DEBUG_ERROR, - "FmpDxe(%s): CheckTheImage() - LastAttemptStatus %d from FmpDeviceCheckImageWithStatus() is invalid.\n", - mImageIdName, - *LastAttemptStatus) - ); + DEBUG (( + DEBUG_ERROR, + "FmpDxe(%s): CheckTheImage() - LastAttemptStatus %d from FmpDeviceCheckImageWithStatus() is invalid.\n", + mImageIdName, + *LastAttemptStatus + )); *LastAttemptStatus = LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL; } } diff --git a/FmpDevicePkg/Include/Library/FmpDeviceLib.h b/FmpDevicePkg/Include/Library/FmpDeviceLib.h index a14406abe8..f82ef64503 100644 --- a/FmpDevicePkg/Include/Library/FmpDeviceLib.h +++ b/FmpDevicePkg/Include/Library/FmpDeviceLib.h @@ -421,8 +421,7 @@ FmpDeviceCheckImage ( IMAGE_UPDATABLE_VALID_WITH_VENDOR_CODE @param[out] LastAttemptStatus A pointer to a UINT32 that holds the last attempt status to report back to the ESRT table in case - of error. This value will only be checked when this - function returns an error. + of error. The return status code must fall in the range of LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE to diff --git a/FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c b/FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c index 2e5c17b2b0..82219e87a4 100644 --- a/FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c +++ b/FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c @@ -434,8 +434,7 @@ FmpDeviceCheckImage ( IMAGE_UPDATABLE_VALID_WITH_VENDOR_CODE @param[out] LastAttemptStatus A pointer to a UINT32 that holds the last attempt status to report back to the ESRT table in case - of error. This value will only be checked when this - function returns an error. + of error. The return status code must fall in the range of LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE to -- cgit v1.2.3