Re: [edk2-devel] [PATCH v2 3/6] uefi-sct/SctPkg: TCG MORLOCK SetVariable Test

["Stuart Yoder" <stuart.yoder@arm.com>]

See inline comments...

On 9/21/23 11:37 AM, Abhimanyu Singh via groups.io wrote:
> SCT spec: https://bugzilla.tianocore.org/show_bug.cgi?id=4374
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4419
> 
> -Implement MemoryOverwriteRequestControlLockSetVariable test
>   cases
>   -Add Assertions 9 through 18 from SCT spec
>   -Add Test Case to MemoryOverwriteRequestFunctionTest
> 
> Cc: G Edhaya Chandran <Edhaya.Chandran@arm.com>
> Cc: Barton Gao <gaojie@byosoft.com.cn>
> Cc: Carolyn Gjertsen <Carolyn.Gjertsen@amd.com>
> Signed-off-by: Abhi Singh <Abhi.Singh@arm.com>
> Change-Id: I23ba7256441354f497ecdc96a394df5ba5cae8df
> ---

[cut]

> +
> +  // MORLOCK SetVariable with valid GUID, variable name, Data, and Attributes
> +  // except for invalid DataSize == 0
> +  // verify SetVariable returns EFI_WRITE_PROTECTED and GetVariable returns a Data Value == 0
> +  DataSize = 0;
> +  Attributes = TCG_MOR_VARIABLE_ATTRIBUTES;
> +  MemoryOverwriteRequestControlLockData = MOR_LOCK_DATA_LOCKED_WITHOUT_KEY;
> +
> +  Status = gtRT->SetVariable (
> +                  L"MemoryOverwriteRequestControlLock",        // VariableName
> +                  &gEfiMemoryOverwriteRequestControlLockGuid,  // VendorGuid
> +                  Attributes,                                  // Attributes
> +                  DataSize,                                    // DataSize
> +                  &MemoryOverwriteRequestControlLockData       // Data
> +                  );
> +  if (Status == EFI_WRITE_PROTECTED) {
> +    Result = EFI_TEST_ASSERTION_PASSED;
> +  } else {
> +    Result = EFI_TEST_ASSERTION_FAILED;
> +  }
> +
> +  StandardLib->RecordAssertion (
> +                  StandardLib,
> +                  Result,
> +                  gTCGMemoryOverwriteRequestTestFunctionAssertionGuid011,
> +                  L"MemoryOverwriteRequestControlLock - SetVariable() with DataSize == 0 returns EFI_WRITE_PROTECTED",
> +                  L"%a:%d:Status - %r",
> +                  __FILE__,
> +                  (UINTN)__LINE__,
> +                  Status
> +                  );
> +
> +  // change datasize to valid value before GetVariable
> +  DataSize = sizeof(MemoryOverwriteRequestControlLockData);
> +  Attributes = TCG_MOR_VARIABLE_ATTRIBUTES;
> +
> +  // now check that MORLOCK value is still 0x00 or Unlocked
> +  Status = gtRT->GetVariable (
> +                  L"MemoryOverwriteRequestControlLock",        // VariableName
> +                  &gEfiMemoryOverwriteRequestControlLockGuid,  // VendorGuid
> +                  &Attributes,                                 // Attributes
> +                  &DataSize,                                   // DataSize
> +                  &MemoryOverwriteRequestControlLockData       // Data
> +                  );
> +  if (EFI_ERROR (Status) || (MemoryOverwriteRequestControlLockData != MOR_LOCK_DATA_UNLOCKED)) {
> +    Result = EFI_TEST_ASSERTION_FAILED;
> +  } else {
> +    Result = EFI_TEST_ASSERTION_PASSED;
> +  }
> +
> +  StandardLib->RecordAssertion (
> +                  StandardLib,
> +                  Result,
> +                  gTCGMemoryOverwriteRequestTestFunctionAssertionGuid012,
> +                  L"MemoryOverwriteRequestControlLock - Lock value remains Unlocked",
> +                  L"%a:%d:Status - %r",
> +                  __FILE__,
> +                  (UINTN)__LINE__,
> +                  Status
> +                  );
> +
> +  // MORLOCK SetVariable with valid GUID, variable name, DataSize, and Attributes
> +  // except for Data == NULL
> +  // verify SetVariable returns EFI_INVALID_PARAMETER and GetVariable returns a Data Value == 0
> +  DataSize = sizeof(MemoryOverwriteRequestControlLockData);
> +  Attributes = TCG_MOR_VARIABLE_ATTRIBUTES;
> +
> +  Status = gtRT->SetVariable (
> +                  L"MemoryOverwriteRequestControlLock",        // VariableName
> +                  &gEfiMemoryOverwriteRequestControlLockGuid,  // VendorGuid
> +                  Attributes,                                  // Attributes
> +                  DataSize,                                    // DataSize
> +                  NULL                                         // Data
> +                  );
> +  if (Status == EFI_INVALID_PARAMETER) {
> +    Result = EFI_TEST_ASSERTION_PASSED;
> +  } else {
> +    Result = EFI_TEST_ASSERTION_FAILED;
> +  }

This code checks for EFI_INVALID_PARAMETER, which seems correct.  But,
it does not match the SCT specification for these tests on the
bugzilla ticket, which says that EFI_WRITE_PROTECTED is expected.
I think you need to fix the spec.

Thanks,
Stuart



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108964): https://edk2.groups.io/g/devel/message/108964
Mute This Topic: https://groups.io/mt/101504339/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-