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 40AC2941554 for ; Fri, 5 Jan 2024 13:50:27 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=yoIKJn+jz7UJGFgLAh056hbh3uV5FybOdasybMqj3lw=; c=relaxed/simple; d=groups.io; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20140610; t=1704462625; v=1; b=YnOPt8mSkx1606TDmaJ89/4Qrf6tE6q5BlHD4nc/m1yQ3Bt1g489EXmSVgbH0dL8ZKrcU1Xd xU8Luqrkk/3HRIWDny1JIbWt5I8ig+r/gu+Lr2VHtYREHr553SP2ZD705P2Xd/v9eY/JwvFob1+ pGO7UlS2wS8iVhIPHZJDHeMA= X-Received: by 127.0.0.2 with SMTP id cLGIYY7687511xVsW5c5rwWi; Fri, 05 Jan 2024 05:50:25 -0800 X-Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mx.groups.io with SMTP id smtpd.web10.23734.1704462625213563798 for ; Fri, 05 Jan 2024 05:50:25 -0800 X-Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-385-IkRmpOm_OUy-9VEc0UoL8w-1; Fri, 05 Jan 2024 08:50:21 -0500 X-MC-Unique: IkRmpOm_OUy-9VEc0UoL8w-1 X-Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.rdu2.redhat.com [10.11.54.8]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id C8E113C025C2; Fri, 5 Jan 2024 13:50:20 +0000 (UTC) X-Received: from [10.39.193.6] (unknown [10.39.193.6]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C0DFEC15968; Fri, 5 Jan 2024 13:50:19 +0000 (UTC) Message-ID: Date: Fri, 5 Jan 2024 14:50:18 +0100 MIME-Version: 1.0 Subject: Re: [edk2-devel] [PATCH v3 1/1] OvmfPkg/VirtNorFlashDxe: sanity-check variables To: Gerd Hoffmann Cc: devel@edk2.groups.io, Ard Biesheuvel , oliver@redhat.com, mike.maslenkin@gmail.com, Jiewen Yao References: <20231214153156.46812-1-kraxel@redhat.com> From: "Laszlo Ersek" In-Reply-To: X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.8 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: 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,lersek@redhat.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: OoByC9xWR0qt3rWLlLsYbigux7686176AA= Content-Language: en-US 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=YnOPt8mS; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=redhat.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 On 1/4/24 16:06, Gerd Hoffmann wrote: > Hi, >=20 >>>> - if the StartId is 0x55aa, then we need to look further, beause we >>>> can't decide yet. For example, if State is VAR_HEADER_VALID_ONLY (0x7f= ), >>>> then it might be fine for the variable header (at the very end of the >>>> varstore) *not* to be followed by payload bytes (name, data). >>> >>> Not sure this makes sense. VAR_HEADER_VALID_ONLY is a temporary state, >>> while the variable driver writes name and data just after the header, >>> to be updated to VAR_ADDED when the write completed successfully. So >>> I'd expect to never find a header without space for name + data. >> >> - Do we know for sure that VAR_HEADER_VALID_ONLY is never expected to be >> seen? >=20 > Writing goes like this: >=20 > (1) find free space > (2) write header, with VAR_HEADER_VALID_ONLY. > (3) write name + data > (4) update header, set state =3D VAR_ADDED. >=20 >> What if the variable update design defines VAR_HEADER_VALID_ONLY >> specifically so that the variable driver can recover from a power loss >> "in the middle"? >=20 > Power loss in step (3) can surely lead to variables in > VAR_HEADER_VALID_ONLY state, and I'd expect the variable driver can > actually recover from that. >=20 > [ side note: The (2) write should be small enough that it fits into the > flash block write buffer (128 bytes). Which could be > important to maintain variable store consistency. ] >=20 > Nevertheless we should never find a header at the end of the variable > store, without space allocated for name + date. Minimal space for the > name is 4 bytes (one char16 + '\0'), for the data 1 byte, alignment > rounds the latter to 4 bytes too, so this should be true: >=20 > VarOffset + sizeof(*VarHeader) + 8 <=3D VariableStoreHeader->Size >=20 >> So I figure, if we accept VAR_HEADER_VALID_ONLY in that logic, then we >> should also accept VAR_HEADER_VALID_ONLY if it's at the very end of >> the varstore. >=20 > Disagree, see above. Storing the header at a place which leaves no room > for name + data doesn't make sense to me. OK, that sounds convincing, thanks! Laszlo > We could go the extra mile and look at the next StartId location, verify > StartId !=3D 0x55aa, in the no-space-left-for-header case. >=20 > take care, > Gerd >=20 -=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 (#113283): https://edk2.groups.io/g/devel/message/113283 Mute This Topic: https://groups.io/mt/103171811/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/19134562= 12/xyzzy [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-