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 0B8A5D8119E for ; Thu, 14 Dec 2023 16:18:29 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=tHCfTcNELpuRE0G2g7F08h95xgS6/25HDgoYlDQ774Q=; 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=1702570708; v=1; b=ei3dPu5ewmrlzb5W6WC83z/w3Uk6PYwDGZtqxFPlnvM0Of0v/86LAUCPxoHTqPr7kqcG8Q1B EjGuwDhlzCm7nCYH/pYrt6Ii/YSE85lR1MjdU7whJ9zmoFSkW0R32hzxKjJzd6bIEoYMwxcSd/O pCgDpQJVEnwIoF/v/6D006EI= X-Received: by 127.0.0.2 with SMTP id fPViYY7687511xuXT9vspdde; Thu, 14 Dec 2023 08:18:28 -0800 X-Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.groups.io with SMTP id smtpd.web10.26769.1702570707896603673 for ; Thu, 14 Dec 2023 08:18:28 -0800 X-Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-426-_37c-Qg4OcyhxIcO-URWyg-1; Thu, 14 Dec 2023 11:18:21 -0500 X-MC-Unique: _37c-Qg4OcyhxIcO-URWyg-1 X-Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (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 8CC27100F801; Thu, 14 Dec 2023 16:18:20 +0000 (UTC) X-Received: from [10.39.192.41] (unknown [10.39.192.41]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 6ECD4492BC6; Thu, 14 Dec 2023 16:18:19 +0000 (UTC) Message-ID: <5e89b8db-f2d1-68c0-8307-51151df2fec7@redhat.com> Date: Thu, 14 Dec 2023 17:18:18 +0100 MIME-Version: 1.0 Subject: Re: [edk2-devel] [PATCH v2 1/1] OvmfPkg/VirtNorFlashDxe: sanity-check variables To: Gerd Hoffmann Cc: devel@edk2.groups.io, mike.maslenkin@gmail.com, Jiewen Yao , Ard Biesheuvel , oliver@redhat.com References: <20231207094404.270381-1-kraxel@redhat.com> <72vsvymwepkdu7m7b3uy5wxifxggegezwi44zkascp3weya7ci@afbg44yzs3ov> From: "Laszlo Ersek" In-Reply-To: <72vsvymwepkdu7m7b3uy5wxifxggegezwi44zkascp3weya7ci@afbg44yzs3ov> X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.9 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: h0cE3nLj44DDEdznu4WJPB7fx7686176AA= 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=ei3dPu5e; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=redhat.com (policy=none) On 12/14/23 16:31, Gerd Hoffmann wrote: > Hi, > =20 >> The general idea is, once we don't trust the varstore, there cannot be >> a *single* unchecked addition in the code. (Unless we can *prove* that >> overflow is impossible.) >=20 > There are some cases where we add a small, constant number to a value we > know is smaller than VariableStoreHeader->Size. I don't see how those > can overflow, given that varstore flash typically is an order of > magnitude smaller than MAX_UINT32 OK. Please add comments about these though, possibly expressed as ASSERT()s= . > (unless VariableStoreHeader->Size is > corrupted, but then we have bigger problems anyway ...). Right... I had given some thought to that as well. If there's an easy -- albeit perhaps arbitrary -- range check for that up-front, maybe we should do it. Maybe. But I'm certainly not asking for armoring existent code in the affected function. That's too much work -- ridding all existent code of overflows is just a special case of eliminating technical debt, and there is enough technical debt in edk2 to spend a lifetime fixing. The only reasonable approach I can imagine is to stop introducing technical debt, or *at least* to fix technical debt at a higher rate than adding it. (This is no small challenge.) Thanks, Laszlo -=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 (#112545): https://edk2.groups.io/g/devel/message/112545 Mute This Topic: https://groups.io/mt/103031342/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-