ArmPkg: Fix uninitialised variable in ArmMmuStandaloneMmLib

The following patches added support for StandaloneMM using FF-A:
9da5ee116a ArmPkg: Allow FF-A calls to set memory region's attributes
0e43e02b9b ArmPkg: Allow FF-A calls to get memory region's attributes

However, in the error handling logic for the Get/Set Memory attributes,
the CLANG compiler reports that a status variable could be used without
initialisation. This issue is a false positive and is not seen with GCC.

The Get/Set Memory attributes operation is atomic and therefore an
FFA_INTERRUPT or FFA_SUCCESS response is not expected in response
to FFA_MSG_SEND_DIRECT_REQ. So the remaining cases that could occur
are:
 - the target sends FFA_MSG_SEND_DIRECT_RESP with a success or
   failure code.
 or
 - FFA_MSG_SEND_DIRECT_REQ transmission failure.

Therefore,
 - reorder the error handling conditions such that it prevents the
   uninitialised variable issue being flagged by CLANG.
 - move the repetitive code to a static helper function and add
   documentation at the appropriate places.
 - fix error handling in functions that invoke GetMemoryPermissions().

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Tested-by: Sughosh Ganu <sughosh.ganu@linaro.org>
Reviewed-by: Sughosh Ganu <sughosh.ganu@linaro.org>
This commit is contained in:
Sami Mujawar 2021-02-25 17:11:10 +00:00 committed by mergify[bot]
parent cd14150c15
commit 31eaefd4df
1 changed files with 199 additions and 164 deletions

View File

@ -1,10 +1,15 @@
/** @file /** @file
* File managing the MMU for ARMv8 architecture in S-EL0 File managing the MMU for ARMv8 architecture in S-EL0
*
* Copyright (c) 2017 - 2021, Arm Limited. All rights reserved.<BR> Copyright (c) 2017 - 2021, Arm Limited. All rights reserved.<BR>
* SPDX-License-Identifier: BSD-2-Clause-Patent
* SPDX-License-Identifier: BSD-2-Clause-Patent
* @par Reference(s):
- [1] SPM based on the MM interface.
(https://trustedfirmware-a.readthedocs.io/en/latest/components/
secure-partition-manager-mm.html)
- [2] Arm Firmware Framework for Armv8-A, DEN0077A, version 1.0
(https://developer.arm.com/documentation/den0077/a)
**/ **/
#include <Uefi.h> #include <Uefi.h>
@ -19,37 +24,37 @@
#include <Library/DebugLib.h> #include <Library/DebugLib.h>
#include <Library/PcdLib.h> #include <Library/PcdLib.h>
/** Send memory permission request to target.
@param [in, out] SvcArgs Pointer to SVC arguments to send. On
return it contains the response parameters.
@param [out] RetVal Pointer to return the response value.
@retval EFI_SUCCESS Request successfull.
@retval EFI_INVALID_PARAMETER A parameter is invalid.
@retval EFI_NOT_READY Callee is busy or not in a state to handle
this request.
@retval EFI_UNSUPPORTED This function is not implemented by the
callee.
@retval EFI_ABORTED Message target ran into an unexpected error
and has aborted.
@retval EFI_ACCESS_DENIED Access denied.
@retval EFI_OUT_OF_RESOURCES Out of memory to perform operation.
**/
STATIC STATIC
EFI_STATUS EFI_STATUS
GetMemoryPermissions ( SendMemoryPermissionRequest (
IN EFI_PHYSICAL_ADDRESS BaseAddress, IN OUT ARM_SVC_ARGS *SvcArgs,
OUT UINT32 *MemoryAttributes OUT INT32 *RetVal
) )
{ {
INT32 Ret; if ((SvcArgs == NULL) || (RetVal == NULL)) {
ARM_SVC_ARGS GetMemoryPermissionsSvcArgs; return EFI_INVALID_PARAMETER;
BOOLEAN FfaEnabled;
ZeroMem (&GetMemoryPermissionsSvcArgs, sizeof (ARM_SVC_ARGS));
FfaEnabled = FeaturePcdGet (PcdFfaEnable);
if (FfaEnabled) {
GetMemoryPermissionsSvcArgs.Arg0 = ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH64;
GetMemoryPermissionsSvcArgs.Arg1 = ARM_FFA_DESTINATION_ENDPOINT_ID;
GetMemoryPermissionsSvcArgs.Arg2 = 0;
GetMemoryPermissionsSvcArgs.Arg3 = ARM_SVC_ID_SP_GET_MEM_ATTRIBUTES_AARCH64;
GetMemoryPermissionsSvcArgs.Arg4 = BaseAddress;
} else {
GetMemoryPermissionsSvcArgs.Arg0 = ARM_SVC_ID_SP_GET_MEM_ATTRIBUTES_AARCH64;
GetMemoryPermissionsSvcArgs.Arg1 = BaseAddress;
GetMemoryPermissionsSvcArgs.Arg2 = 0;
GetMemoryPermissionsSvcArgs.Arg3 = 0;
} }
*MemoryAttributes = 0; ArmCallSvc (SvcArgs);
ArmCallSvc (&GetMemoryPermissionsSvcArgs); if (FeaturePcdGet (PcdFfaEnable)) {
if (FfaEnabled) { // Get/Set memory attributes is an atomic call, with
// Getting memory attributes is an atomic call, with
// StandaloneMm at S-EL0 being the caller and the SPM // StandaloneMm at S-EL0 being the caller and the SPM
// core being the callee. Thus there won't be a // core being the callee. Thus there won't be a
// FFA_INTERRUPT or FFA_SUCCESS response to the Direct // FFA_INTERRUPT or FFA_SUCCESS response to the Direct
@ -57,19 +62,24 @@ GetMemoryPermissions (
// for other Direct Request calls which are not atomic // for other Direct Request calls which are not atomic
// We therefore check only for Direct Response by the // We therefore check only for Direct Response by the
// callee. // callee.
if (GetMemoryPermissionsSvcArgs.Arg0 != if (SvcArgs->Arg0 == ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP_AARCH64) {
ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP_AARCH64) { // A Direct Response means FF-A success
// Now check the payload for errors
// The callee sends back the return value
// in Arg3
*RetVal = SvcArgs->Arg3;
} else {
// If Arg0 is not a Direct Response, that means we // If Arg0 is not a Direct Response, that means we
// have an FF-A error. We need to check Arg2 for the // have an FF-A error. We need to check Arg2 for the
// FF-A error code. // FF-A error code.
Ret = GetMemoryPermissionsSvcArgs.Arg2; // See [2], Table 10.8: FFA_ERROR encoding.
switch (Ret) { *RetVal = SvcArgs->Arg2;
switch (*RetVal) {
case ARM_FFA_SPM_RET_INVALID_PARAMETERS: case ARM_FFA_SPM_RET_INVALID_PARAMETERS:
return EFI_INVALID_PARAMETER; return EFI_INVALID_PARAMETER;
case ARM_FFA_SPM_RET_DENIED: case ARM_FFA_SPM_RET_DENIED:
return EFI_NOT_READY; return EFI_ACCESS_DENIED;
case ARM_FFA_SPM_RET_NOT_SUPPORTED: case ARM_FFA_SPM_RET_NOT_SUPPORTED:
return EFI_UNSUPPORTED; return EFI_UNSUPPORTED;
@ -79,112 +89,23 @@ GetMemoryPermissions (
case ARM_FFA_SPM_RET_ABORTED: case ARM_FFA_SPM_RET_ABORTED:
return EFI_ABORTED; return EFI_ABORTED;
default:
// Undefined error code received.
ASSERT (0);
return EFI_INVALID_PARAMETER;
} }
} else if (GetMemoryPermissionsSvcArgs.Arg0 ==
ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP_AARCH64) {
// A Direct Response means FF-A success
// Now check the payload for errors
// The callee sends back the return value
// in Arg3
Ret = GetMemoryPermissionsSvcArgs.Arg3;
} }
} else { } else {
Ret = GetMemoryPermissionsSvcArgs.Arg0; *RetVal = SvcArgs->Arg0;
} }
if (Ret & BIT31) { // Check error response from Callee.
if (*RetVal & BIT31) {
// Bit 31 set means there is an error retured // Bit 31 set means there is an error retured
switch (Ret) { // See [1], Section 13.5.5.1 MM_SP_MEMORY_ATTRIBUTES_GET_AARCH64 and
case ARM_SVC_SPM_RET_INVALID_PARAMS: // Section 13.5.5.2 MM_SP_MEMORY_ATTRIBUTES_SET_AARCH64.
return EFI_INVALID_PARAMETER; switch (*RetVal) {
case ARM_SVC_SPM_RET_NOT_SUPPORTED:
return EFI_UNSUPPORTED;
}
} else {
*MemoryAttributes = Ret;
}
return EFI_SUCCESS;
}
STATIC
EFI_STATUS
RequestMemoryPermissionChange (
IN EFI_PHYSICAL_ADDRESS BaseAddress,
IN UINT64 Length,
IN UINTN Permissions
)
{
INT32 Ret;
BOOLEAN FfaEnabled;
ARM_SVC_ARGS ChangeMemoryPermissionsSvcArgs;
ZeroMem (&ChangeMemoryPermissionsSvcArgs, sizeof (ARM_SVC_ARGS));
FfaEnabled = FeaturePcdGet (PcdFfaEnable);
if (FfaEnabled) {
ChangeMemoryPermissionsSvcArgs.Arg0 = ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH64;
ChangeMemoryPermissionsSvcArgs.Arg1 = ARM_FFA_DESTINATION_ENDPOINT_ID;
ChangeMemoryPermissionsSvcArgs.Arg2 = 0;
ChangeMemoryPermissionsSvcArgs.Arg3 = ARM_SVC_ID_SP_SET_MEM_ATTRIBUTES_AARCH64;
ChangeMemoryPermissionsSvcArgs.Arg4 = BaseAddress;
ChangeMemoryPermissionsSvcArgs.Arg5 = EFI_SIZE_TO_PAGES (Length);
ChangeMemoryPermissionsSvcArgs.Arg6 = Permissions;
} else {
ChangeMemoryPermissionsSvcArgs.Arg0 = ARM_SVC_ID_SP_SET_MEM_ATTRIBUTES_AARCH64;
ChangeMemoryPermissionsSvcArgs.Arg1 = BaseAddress;
ChangeMemoryPermissionsSvcArgs.Arg2 = EFI_SIZE_TO_PAGES (Length);
ChangeMemoryPermissionsSvcArgs.Arg3 = Permissions;
}
ArmCallSvc (&ChangeMemoryPermissionsSvcArgs);
if (FfaEnabled) {
// Setting memory attributes is an atomic call, with
// StandaloneMm at S-EL0 being the caller and the SPM
// core being the callee. Thus there won't be a
// FFA_INTERRUPT or FFA_SUCCESS response to the Direct
// Request sent above. This will have to be considered
// for other Direct Request calls which are not atomic
// We therefore check only for Direct Response by the
// callee.
if (ChangeMemoryPermissionsSvcArgs.Arg0 !=
ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP_AARCH64) {
// If Arg0 is not a Direct Response, that means we
// have an FF-A error. We need to check Arg2 for the
// FF-A error code.
Ret = ChangeMemoryPermissionsSvcArgs.Arg2;
switch (Ret) {
case ARM_FFA_SPM_RET_INVALID_PARAMETERS:
return EFI_INVALID_PARAMETER;
case ARM_FFA_SPM_RET_DENIED:
return EFI_NOT_READY;
case ARM_FFA_SPM_RET_NOT_SUPPORTED:
return EFI_UNSUPPORTED;
case ARM_FFA_SPM_RET_BUSY:
return EFI_NOT_READY;
case ARM_FFA_SPM_RET_ABORTED:
return EFI_ABORTED;
}
} else if (ChangeMemoryPermissionsSvcArgs.Arg0 ==
ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP_AARCH64) {
// A Direct Response means FF-A success
// Now check the payload for errors
// The callee sends back the return value
// in Arg3
Ret = ChangeMemoryPermissionsSvcArgs.Arg3;
}
} else {
Ret = ChangeMemoryPermissionsSvcArgs.Arg0;
}
switch (Ret) {
case ARM_SVC_SPM_RET_NOT_SUPPORTED: case ARM_SVC_SPM_RET_NOT_SUPPORTED:
return EFI_UNSUPPORTED; return EFI_UNSUPPORTED;
@ -195,12 +116,126 @@ RequestMemoryPermissionChange (
return EFI_ACCESS_DENIED; return EFI_ACCESS_DENIED;
case ARM_SVC_SPM_RET_NO_MEMORY: case ARM_SVC_SPM_RET_NO_MEMORY:
return EFI_BAD_BUFFER_SIZE; return EFI_OUT_OF_RESOURCES;
default:
// Undefined error code received.
ASSERT (0);
return EFI_INVALID_PARAMETER;
}
} }
return EFI_SUCCESS; return EFI_SUCCESS;
} }
/** Request the permission attributes of a memory region from S-EL0.
@param [in] BaseAddress Base address for the memory region.
@param [out] MemoryAttributes Pointer to return the memory attributes.
@retval EFI_SUCCESS Request successfull.
@retval EFI_INVALID_PARAMETER A parameter is invalid.
@retval EFI_NOT_READY Callee is busy or not in a state to handle
this request.
@retval EFI_UNSUPPORTED This function is not implemented by the
callee.
@retval EFI_ABORTED Message target ran into an unexpected error
and has aborted.
@retval EFI_ACCESS_DENIED Access denied.
@retval EFI_OUT_OF_RESOURCES Out of memory to perform operation.
**/
STATIC
EFI_STATUS
GetMemoryPermissions (
IN EFI_PHYSICAL_ADDRESS BaseAddress,
OUT UINT32 *MemoryAttributes
)
{
EFI_STATUS Status;
INT32 Ret;
ARM_SVC_ARGS SvcArgs;
if (MemoryAttributes == NULL) {
return EFI_INVALID_PARAMETER;
}
// Prepare the message parameters.
// See [1], Section 13.5.5.1 MM_SP_MEMORY_ATTRIBUTES_GET_AARCH64.
ZeroMem (&SvcArgs, sizeof (ARM_SVC_ARGS));
if (FeaturePcdGet (PcdFfaEnable)) {
// See [2], Section 10.2 FFA_MSG_SEND_DIRECT_REQ.
SvcArgs.Arg0 = ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH64;
SvcArgs.Arg1 = ARM_FFA_DESTINATION_ENDPOINT_ID;
SvcArgs.Arg2 = 0;
SvcArgs.Arg3 = ARM_SVC_ID_SP_GET_MEM_ATTRIBUTES_AARCH64;
SvcArgs.Arg4 = BaseAddress;
} else {
SvcArgs.Arg0 = ARM_SVC_ID_SP_GET_MEM_ATTRIBUTES_AARCH64;
SvcArgs.Arg1 = BaseAddress;
SvcArgs.Arg2 = 0;
SvcArgs.Arg3 = 0;
}
Status = SendMemoryPermissionRequest (&SvcArgs, &Ret);
if (EFI_ERROR (Status)) {
*MemoryAttributes = 0;
return Status;
}
*MemoryAttributes = Ret;
return Status;
}
/** Set the permission attributes of a memory region from S-EL0.
@param [in] BaseAddress Base address for the memory region.
@param [in] Length Length of the memory region.
@param [in] Permissions Memory access controls attributes.
@retval EFI_SUCCESS Request successfull.
@retval EFI_INVALID_PARAMETER A parameter is invalid.
@retval EFI_NOT_READY Callee is busy or not in a state to handle
this request.
@retval EFI_UNSUPPORTED This function is not implemented by the
callee.
@retval EFI_ABORTED Message target ran into an unexpected error
and has aborted.
@retval EFI_ACCESS_DENIED Access denied.
@retval EFI_OUT_OF_RESOURCES Out of memory to perform operation.
**/
STATIC
EFI_STATUS
RequestMemoryPermissionChange (
IN EFI_PHYSICAL_ADDRESS BaseAddress,
IN UINT64 Length,
IN UINT32 Permissions
)
{
INT32 Ret;
ARM_SVC_ARGS SvcArgs;
// Prepare the message parameters.
// See [1], Section 13.5.5.2 MM_SP_MEMORY_ATTRIBUTES_SET_AARCH64.
ZeroMem (&SvcArgs, sizeof (ARM_SVC_ARGS));
if (FeaturePcdGet (PcdFfaEnable)) {
// See [2], Section 10.2 FFA_MSG_SEND_DIRECT_REQ.
SvcArgs.Arg0 = ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH64;
SvcArgs.Arg1 = ARM_FFA_DESTINATION_ENDPOINT_ID;
SvcArgs.Arg2 = 0;
SvcArgs.Arg3 = ARM_SVC_ID_SP_SET_MEM_ATTRIBUTES_AARCH64;
SvcArgs.Arg4 = BaseAddress;
SvcArgs.Arg5 = EFI_SIZE_TO_PAGES (Length);
SvcArgs.Arg6 = Permissions;
} else {
SvcArgs.Arg0 = ARM_SVC_ID_SP_SET_MEM_ATTRIBUTES_AARCH64;
SvcArgs.Arg1 = BaseAddress;
SvcArgs.Arg2 = EFI_SIZE_TO_PAGES (Length);
SvcArgs.Arg3 = Permissions;
}
return SendMemoryPermissionRequest (&SvcArgs, &Ret);
}
EFI_STATUS EFI_STATUS
ArmSetMemoryRegionNoExec ( ArmSetMemoryRegionNoExec (
IN EFI_PHYSICAL_ADDRESS BaseAddress, IN EFI_PHYSICAL_ADDRESS BaseAddress,
@ -212,7 +247,7 @@ ArmSetMemoryRegionNoExec (
UINT32 CodePermission; UINT32 CodePermission;
Status = GetMemoryPermissions (BaseAddress, &MemoryAttributes); Status = GetMemoryPermissions (BaseAddress, &MemoryAttributes);
if (Status != EFI_INVALID_PARAMETER) { if (!EFI_ERROR (Status)) {
CodePermission = SET_MEM_ATTR_CODE_PERM_XN << SET_MEM_ATTR_CODE_PERM_SHIFT; CodePermission = SET_MEM_ATTR_CODE_PERM_XN << SET_MEM_ATTR_CODE_PERM_SHIFT;
return RequestMemoryPermissionChange ( return RequestMemoryPermissionChange (
BaseAddress, BaseAddress,
@ -220,7 +255,7 @@ ArmSetMemoryRegionNoExec (
MemoryAttributes | CodePermission MemoryAttributes | CodePermission
); );
} }
return EFI_INVALID_PARAMETER; return Status;
} }
EFI_STATUS EFI_STATUS
@ -234,7 +269,7 @@ ArmClearMemoryRegionNoExec (
UINT32 CodePermission; UINT32 CodePermission;
Status = GetMemoryPermissions (BaseAddress, &MemoryAttributes); Status = GetMemoryPermissions (BaseAddress, &MemoryAttributes);
if (Status != EFI_INVALID_PARAMETER) { if (!EFI_ERROR (Status)) {
CodePermission = SET_MEM_ATTR_CODE_PERM_XN << SET_MEM_ATTR_CODE_PERM_SHIFT; CodePermission = SET_MEM_ATTR_CODE_PERM_XN << SET_MEM_ATTR_CODE_PERM_SHIFT;
return RequestMemoryPermissionChange ( return RequestMemoryPermissionChange (
BaseAddress, BaseAddress,
@ -242,7 +277,7 @@ ArmClearMemoryRegionNoExec (
MemoryAttributes & ~CodePermission MemoryAttributes & ~CodePermission
); );
} }
return EFI_INVALID_PARAMETER; return Status;
} }
EFI_STATUS EFI_STATUS
@ -256,7 +291,7 @@ ArmSetMemoryRegionReadOnly (
UINT32 DataPermission; UINT32 DataPermission;
Status = GetMemoryPermissions (BaseAddress, &MemoryAttributes); Status = GetMemoryPermissions (BaseAddress, &MemoryAttributes);
if (Status != EFI_INVALID_PARAMETER) { if (!EFI_ERROR (Status)) {
DataPermission = SET_MEM_ATTR_DATA_PERM_RO << SET_MEM_ATTR_DATA_PERM_SHIFT; DataPermission = SET_MEM_ATTR_DATA_PERM_RO << SET_MEM_ATTR_DATA_PERM_SHIFT;
return RequestMemoryPermissionChange ( return RequestMemoryPermissionChange (
BaseAddress, BaseAddress,
@ -264,7 +299,7 @@ ArmSetMemoryRegionReadOnly (
MemoryAttributes | DataPermission MemoryAttributes | DataPermission
); );
} }
return EFI_INVALID_PARAMETER; return Status;
} }
EFI_STATUS EFI_STATUS
@ -278,7 +313,7 @@ ArmClearMemoryRegionReadOnly (
UINT32 PermissionRequest; UINT32 PermissionRequest;
Status = GetMemoryPermissions (BaseAddress, &MemoryAttributes); Status = GetMemoryPermissions (BaseAddress, &MemoryAttributes);
if (Status != EFI_INVALID_PARAMETER) { if (!EFI_ERROR (Status)) {
PermissionRequest = SET_MEM_ATTR_MAKE_PERM_REQUEST (SET_MEM_ATTR_DATA_PERM_RW, PermissionRequest = SET_MEM_ATTR_MAKE_PERM_REQUEST (SET_MEM_ATTR_DATA_PERM_RW,
MemoryAttributes); MemoryAttributes);
return RequestMemoryPermissionChange ( return RequestMemoryPermissionChange (
@ -287,5 +322,5 @@ ArmClearMemoryRegionReadOnly (
PermissionRequest PermissionRequest
); );
} }
return EFI_INVALID_PARAMETER; return Status;
} }