On Fri, Mar 04, 2022 at 03:11:43PM -0500, Tyler Fanelli wrote:
> The length of the attestation report buffer is never checked to be
> valid before allocation is made. If the length of the report is returned
> to be 0, the buffer to retrieve the attestation buffer is allocated with
> length 0 and passed to the kernel to fill with contents of the attestation
> report. Leaving this unchecked is dangerous and could lead to undefined
> behavior.
>
> Signed-off-by: Tyler Fanelli <tfanelli@xxxxxxxxxx>
> ---
> target/i386/sev.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 025ff7a6f8..e82be3e350 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -616,6 +616,8 @@ static SevAttestationReport *sev_get_attestation_report(const char *mnonce,
> return NULL;
> }
>
> + input.len = 0;
> +
> /* Query the report length */
> ret = sev_ioctl(sev->sev_fd, KVM_SEV_GET_ATTESTATION_REPORT,
> &input, &err);
> @@ -626,6 +628,11 @@ static SevAttestationReport *sev_get_attestation_report(const char *mnonce,
> ret, err, fw_error_to_str(err));
> return NULL;
> }
> + } else if (input.len == 0) {
> + error_setg(errp, "SEV: Failed to query attestation report:"
> + " length returned=%u",
> + input.len);
> + return NULL;
I still feel the described scenario is a kernel bug, as QEMU handles
len == 0 safely already AFAICT. I can't see how the upstream kernel
would end up in the problem state. Can you show the buggy kernel
code upstream.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
