From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 0970FAC10CD for ; Wed, 6 Dec 2023 12:58:57 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=edl/0Gsb42ifLaZxYtcCywir3CE9MOnu2MdWelpbaG0=; c=relaxed/simple; d=groups.io; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject:To:Cc:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type:Content-Transfer-Encoding; s=20140610; t=1701867536; v=1; b=owQuwM+qRKdLYMaPuk8RVTyV6N3aQ9ZC5sVmYb+u32K43SSRK48iu/hYZDRupaOmclEFG+bq q9Es36faMunrUG8oaubill+qjueZWZmzL4kn1J6tISwNLZvtuL74PZT8MfL0esi63QYuoq0Gefq Gcz5xCGastwf73N2DDh29ydw= X-Received: by 127.0.0.2 with SMTP id oluwYY7687511xuHmcFBZ5ur; Wed, 06 Dec 2023 04:58:56 -0800 X-Received: from mail-yb1-f171.google.com (mail-yb1-f171.google.com [209.85.219.171]) by mx.groups.io with SMTP id smtpd.web10.30895.1701867535996019075 for ; Wed, 06 Dec 2023 04:58:56 -0800 X-Received: by mail-yb1-f171.google.com with SMTP id 3f1490d57ef6-dae0ab8ac3eso5199395276.0 for ; Wed, 06 Dec 2023 04:58:55 -0800 (PST) X-Gm-Message-State: vsGRfPelJdOQVZDVAH30wPr5x7686176AA= X-Google-Smtp-Source: AGHT+IEXZ8ZXghEMiIoJSwrXSZdMZ7ouJMn5SMVar8iJOqYsJTwJuQxpxE+DPAscREZYNXij0TCa5E9MfqI98PnWugU= X-Received: by 2002:a5b:891:0:b0:da0:365d:9e21 with SMTP id e17-20020a5b0891000000b00da0365d9e21mr619038ybq.22.1701867535035; Wed, 06 Dec 2023 04:58:55 -0800 (PST) MIME-Version: 1.0 References: <20231205135104.265984-1-kraxel@redhat.com> In-Reply-To: <20231205135104.265984-1-kraxel@redhat.com> From: "Mike Maslenkin" Date: Wed, 6 Dec 2023 15:58:19 +0300 Message-ID: Subject: Re: [edk2-devel] [PATCH 1/1] OvmfPkg/VirtNorFlashDxe: sanity-check variables To: devel@edk2.groups.io, kraxel@redhat.com Cc: Jiewen Yao , Ard Biesheuvel , oliver@redhat.com Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,mike.maslenkin@gmail.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=owQuwM+q; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=gmail.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io Hi Gerd, On Tue, Dec 5, 2023 at 4:51=E2=80=AFPM Gerd Hoffmann wr= ote: > > 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 > --- > OvmfPkg/VirtNorFlashDxe/VirtNorFlashFvb.c | 82 +++++++++++++++++++++-- > 1 file changed, 77 insertions(+), 5 deletions(-) > > diff --git a/OvmfPkg/VirtNorFlashDxe/VirtNorFlashFvb.c b/OvmfPkg/VirtNorF= lashDxe/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 =3D (EFI_FIRMWARE_VOLUME_HEADER *)Instance->RegionBaseAddr= ess; > > @@ -260,6 +265,73 @@ ValidateFvHeader ( > return EFI_NOT_FOUND; > } > > + // check variables > + DEBUG ((DEBUG_INFO, "%a: checking variables\n", __func__)); > + VarOffset =3D sizeof (*VariableStoreHeader); > + while (VarOffset + sizeof (*VarHeader) < VariableStoreHeader->Size) { > + VarHeader =3D (VOID *)((UINTN)VariableStoreHeader + VarOffset); > + if (VarHeader->StartId !=3D 0x55aa) { > + DEBUG ((DEBUG_INFO, "%a: end of var list\n", __func__)); > + break; > + } > + > + VarSize =3D sizeof (*VarHeader) + VarHeader->NameSize + VarHeader->D= ataSize; > + 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 =3D (VOID *)((UINTN)VariableStoreHeader + VarOffset > + + sizeof (*VarHeader)); > + switch (VarHeader->State) { > + case VAR_HEADER_VALID_ONLY: > + VarState =3D "header-ok"; > + VarName =3D L""; > + break; > + case VAR_ADDED: > + VarState =3D "ok"; > + break; > + case VAR_ADDED &VAR_IN_DELETED_TRANSITION: > + VarState =3D "del-in-transition"; > + break; > + case VAR_ADDED &VAR_DELETED: > + case VAR_ADDED &VAR_DELETED &VAR_IN_DELETED_TRANSITION: > + VarState =3D "deleted"; > + break; > + default: > + DEBUG (( > + DEBUG_ERROR, > + "%a: invalid variable state: 0x%x\n", > + __func__, > + VarState > + )); Did you want to print VarHeader->State? Regards, Mike. -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- 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] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-