* [edk2-devel] [PATCH 1/1] OvmfPkg/VirtNorFlashDxe: sanity-check variables
@ 2023-12-05 13:51 Gerd Hoffmann
2023-12-06 12:58 ` Mike Maslenkin
0 siblings, 1 reply; 3+ messages in thread
From: Gerd Hoffmann @ 2023-12-05 13:51 UTC (permalink / raw)
To: devel; +Cc: Jiewen Yao, Ard Biesheuvel, oliver, Gerd Hoffmann
Extend the ValidateFvHeader function, additionally to the header checks
walk over the list of variables and sanity check them.
In case we find inconsistencies indicating variable store corruption
return EFI_NOT_FOUND so the variable store will be re-initialized.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
OvmfPkg/VirtNorFlashDxe/VirtNorFlashFvb.c | 82 +++++++++++++++++++++--
1 file changed, 77 insertions(+), 5 deletions(-)
diff --git a/OvmfPkg/VirtNorFlashDxe/VirtNorFlashFvb.c b/OvmfPkg/VirtNorFlashDxe/VirtNorFlashFvb.c
index 5ee98e9b595a..72a197e5aa20 100644
--- a/OvmfPkg/VirtNorFlashDxe/VirtNorFlashFvb.c
+++ b/OvmfPkg/VirtNorFlashDxe/VirtNorFlashFvb.c
@@ -185,11 +185,16 @@ ValidateFvHeader (
IN NOR_FLASH_INSTANCE *Instance
)
{
- UINT16 Checksum;
- EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader;
- VARIABLE_STORE_HEADER *VariableStoreHeader;
- UINTN VariableStoreLength;
- UINTN FvLength;
+ UINT16 Checksum;
+ EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader;
+ VARIABLE_STORE_HEADER *VariableStoreHeader;
+ UINTN VarOffset;
+ AUTHENTICATED_VARIABLE_HEADER *VarHeader;
+ UINTN VarSize;
+ CHAR16 *VarName;
+ CHAR8 *VarState;
+ UINTN VariableStoreLength;
+ UINTN FvLength;
FwVolHeader = (EFI_FIRMWARE_VOLUME_HEADER *)Instance->RegionBaseAddress;
@@ -260,6 +265,73 @@ ValidateFvHeader (
return EFI_NOT_FOUND;
}
+ // check variables
+ DEBUG ((DEBUG_INFO, "%a: checking variables\n", __func__));
+ VarOffset = sizeof (*VariableStoreHeader);
+ while (VarOffset + sizeof (*VarHeader) < VariableStoreHeader->Size) {
+ VarHeader = (VOID *)((UINTN)VariableStoreHeader + VarOffset);
+ if (VarHeader->StartId != 0x55aa) {
+ DEBUG ((DEBUG_INFO, "%a: end of var list\n", __func__));
+ break;
+ }
+
+ VarSize = sizeof (*VarHeader) + VarHeader->NameSize + VarHeader->DataSize;
+ if (VarOffset + VarSize > VariableStoreHeader->Size) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: invalid variable size: 0x%x + 0x%x + 0x%x + 0x%x > 0x%x\n",
+ __func__,
+ VarOffset,
+ sizeof (*VarHeader),
+ VarHeader->NameSize,
+ VarHeader->DataSize,
+ VariableStoreHeader->Size
+ ));
+ return EFI_NOT_FOUND;
+ }
+
+ VarName = (VOID *)((UINTN)VariableStoreHeader + VarOffset
+ + sizeof (*VarHeader));
+ switch (VarHeader->State) {
+ case VAR_HEADER_VALID_ONLY:
+ VarState = "header-ok";
+ VarName = L"<unknown>";
+ break;
+ case VAR_ADDED:
+ VarState = "ok";
+ break;
+ case VAR_ADDED &VAR_IN_DELETED_TRANSITION:
+ VarState = "del-in-transition";
+ break;
+ case VAR_ADDED &VAR_DELETED:
+ case VAR_ADDED &VAR_DELETED &VAR_IN_DELETED_TRANSITION:
+ VarState = "deleted";
+ break;
+ default:
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: invalid variable state: 0x%x\n",
+ __func__,
+ VarState
+ ));
+ return EFI_NOT_FOUND;
+ }
+
+ DEBUG ((
+ DEBUG_VERBOSE,
+ "%a: +0x%04x: name=0x%x data=0x%x '%s' (%a)\n",
+ __func__,
+ VarOffset,
+ VarHeader->NameSize,
+ VarHeader->DataSize,
+ VarName,
+ VarState
+ ));
+
+ VarSize = (VarSize + 3) & ~3; // align
+ VarOffset += VarSize;
+ }
+
return EFI_SUCCESS;
}
--
2.43.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112081): https://edk2.groups.io/g/devel/message/112081
Mute This Topic: https://groups.io/mt/102991507/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [edk2-devel] [PATCH 1/1] OvmfPkg/VirtNorFlashDxe: sanity-check variables
2023-12-05 13:51 [edk2-devel] [PATCH 1/1] OvmfPkg/VirtNorFlashDxe: sanity-check variables Gerd Hoffmann
@ 2023-12-06 12:58 ` Mike Maslenkin
2023-12-07 9:43 ` Gerd Hoffmann
0 siblings, 1 reply; 3+ messages in thread
From: Mike Maslenkin @ 2023-12-06 12:58 UTC (permalink / raw)
To: devel, kraxel; +Cc: Jiewen Yao, Ard Biesheuvel, oliver
Hi Gerd,
On Tue, Dec 5, 2023 at 4:51 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> Extend the ValidateFvHeader function, additionally to the header checks
> walk over the list of variables and sanity check them.
>
> In case we find inconsistencies indicating variable store corruption
> return EFI_NOT_FOUND so the variable store will be re-initialized.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> OvmfPkg/VirtNorFlashDxe/VirtNorFlashFvb.c | 82 +++++++++++++++++++++--
> 1 file changed, 77 insertions(+), 5 deletions(-)
>
> diff --git a/OvmfPkg/VirtNorFlashDxe/VirtNorFlashFvb.c b/OvmfPkg/VirtNorFlashDxe/VirtNorFlashFvb.c
> index 5ee98e9b595a..72a197e5aa20 100644
> --- a/OvmfPkg/VirtNorFlashDxe/VirtNorFlashFvb.c
> +++ b/OvmfPkg/VirtNorFlashDxe/VirtNorFlashFvb.c
> @@ -185,11 +185,16 @@ ValidateFvHeader (
> IN NOR_FLASH_INSTANCE *Instance
> )
> {
> - UINT16 Checksum;
> - EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader;
> - VARIABLE_STORE_HEADER *VariableStoreHeader;
> - UINTN VariableStoreLength;
> - UINTN FvLength;
> + UINT16 Checksum;
> + EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader;
> + VARIABLE_STORE_HEADER *VariableStoreHeader;
> + UINTN VarOffset;
> + AUTHENTICATED_VARIABLE_HEADER *VarHeader;
> + UINTN VarSize;
> + CHAR16 *VarName;
> + CHAR8 *VarState;
> + UINTN VariableStoreLength;
> + UINTN FvLength;
>
> FwVolHeader = (EFI_FIRMWARE_VOLUME_HEADER *)Instance->RegionBaseAddress;
>
> @@ -260,6 +265,73 @@ ValidateFvHeader (
> return EFI_NOT_FOUND;
> }
>
> + // check variables
> + DEBUG ((DEBUG_INFO, "%a: checking variables\n", __func__));
> + VarOffset = sizeof (*VariableStoreHeader);
> + while (VarOffset + sizeof (*VarHeader) < VariableStoreHeader->Size) {
> + VarHeader = (VOID *)((UINTN)VariableStoreHeader + VarOffset);
> + if (VarHeader->StartId != 0x55aa) {
> + DEBUG ((DEBUG_INFO, "%a: end of var list\n", __func__));
> + break;
> + }
> +
> + VarSize = sizeof (*VarHeader) + VarHeader->NameSize + VarHeader->DataSize;
> + if (VarOffset + VarSize > VariableStoreHeader->Size) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: invalid variable size: 0x%x + 0x%x + 0x%x + 0x%x > 0x%x\n",
> + __func__,
> + VarOffset,
> + sizeof (*VarHeader),
> + VarHeader->NameSize,
> + VarHeader->DataSize,
> + VariableStoreHeader->Size
> + ));
> + return EFI_NOT_FOUND;
> + }
> +
> + VarName = (VOID *)((UINTN)VariableStoreHeader + VarOffset
> + + sizeof (*VarHeader));
> + switch (VarHeader->State) {
> + case VAR_HEADER_VALID_ONLY:
> + VarState = "header-ok";
> + VarName = L"<unknown>";
> + break;
> + case VAR_ADDED:
> + VarState = "ok";
> + break;
> + case VAR_ADDED &VAR_IN_DELETED_TRANSITION:
> + VarState = "del-in-transition";
> + break;
> + case VAR_ADDED &VAR_DELETED:
> + case VAR_ADDED &VAR_DELETED &VAR_IN_DELETED_TRANSITION:
> + VarState = "deleted";
> + break;
> + default:
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: invalid variable state: 0x%x\n",
> + __func__,
> + VarState
> + ));
Did you want to print VarHeader->State?
Regards,
Mike.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112123): https://edk2.groups.io/g/devel/message/112123
Mute This Topic: https://groups.io/mt/102991507/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [edk2-devel] [PATCH 1/1] OvmfPkg/VirtNorFlashDxe: sanity-check variables
2023-12-06 12:58 ` Mike Maslenkin
@ 2023-12-07 9:43 ` Gerd Hoffmann
0 siblings, 0 replies; 3+ messages in thread
From: Gerd Hoffmann @ 2023-12-07 9:43 UTC (permalink / raw)
To: devel, mike.maslenkin; +Cc: Jiewen Yao, Ard Biesheuvel, oliver
Hi,
> > + default:
> > + DEBUG ((
> > + DEBUG_ERROR,
> > + "%a: invalid variable state: 0x%x\n",
> > + __func__,
> > + VarState
> > + ));
>
> Did you want to print VarHeader->State?
Yes, indeed. Good catch, thank you, will fix send send v2.
take care,
Gerd
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112176): https://edk2.groups.io/g/devel/message/112176
Mute This Topic: https://groups.io/mt/102991507/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-12-07 9:43 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-05 13:51 [edk2-devel] [PATCH 1/1] OvmfPkg/VirtNorFlashDxe: sanity-check variables Gerd Hoffmann
2023-12-06 12:58 ` Mike Maslenkin
2023-12-07 9:43 ` Gerd Hoffmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox