Re: [edk2-devel] [PATCH v2 1/1] OvmfPkg/VirtNorFlashDxe: sanity-check variables

["Laszlo Ersek" <lersek@redhat.com>]

On 12/14/23 16:31, Gerd Hoffmann wrote:
>   Hi,
>  
>> 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.)
> 
> 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



-=-=-=-=-=-=-=-=-=-=-=-
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/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-