Re: [PULL 23/49] hw/core: Add Enclave Image Format (EIF) related helpers

[Peter Maydell <peter.maydell@linaro.org>]

On Thu, 31 Oct 2024 at 17:54, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> From: Dorjoy Chowdhury <dorjoychy111@gmail.com>
>
> An EIF (Enclave Image Format)[1] file is used to boot an AWS nitro
> enclave[2] virtual machine. The EIF file contains the necessary kernel,
> cmdline, ramdisk(s) sections to boot.
>
> Some helper functions have been introduced for extracting the necessary
> sections from an EIF file and then writing them to temporary files as
> well as computing SHA384 hashes from the section data. These will be
> used in the following commit to add support for nitro-enclave machine
> type in QEMU.
>
> The files added in this commit are not compiled yet but will be added
> to the hw/core/meson.build file in the following commit where
> CONFIG_NITRO_ENCLAVE will be introduced.
>
> [1] https://github.com/aws/aws-nitro-enclaves-image-format
> [2] https://docs.aws.amazon.com/enclaves/latest/user/nitro-enclave.html
>
> Signed-off-by: Dorjoy Chowdhury <dorjoychy111@gmail.com>
> Reviewed-by: Alexander Graf <graf@amazon.com>
> Link: https://lore.kernel.org/r/20241008211727.49088-4-dorjoychy111@gmail.com
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Hi; Coverity raises a couple of potential issues with the
read_eif_file() function in this commit, which are both
"Coverity assumes the file we're reading is untrusted and is
unsure that we're correctly sanitizing data from it before use".
Could somebody who understands the use case here check whether
these need addressing?

CID 1564855 "overflowed integer argument" --
basically eif_header.section_offsets[i] is from the data
file and is a uint64_t, but we pass it to fseek() in
read_eif_file. fseek() takes a signed offset, so if the
section_offset in the file header is very large it will
be unexpectedly treated as a negative number.

CID 1564859 "untrusted value as argument" --
in the "default" case for handling a section type
we call g_malloc() with the uint64_t size from the header.
If the file data is bogus we might try to allocate a
huge amount of memory, which will cause g_malloc() to
kill QEMU.

thanks
-- PMM