Re: [Qemu-devel] [PATCH 5/5] megasas: LSI Megaraid SAS emulation

[Stefan Hajnoczi <stefanha@gmail.com>]

On Tue, Jul 5, 2011 at 12:03 PM, Hannes Reinecke <hare@suse.de> wrote:
> +static void megasas_unmap_sgl(struct megasas_cmd_t *cmd)
> +{
> +    uint16_t flags = le16_to_cpu(cmd->frame->header.flags);
> +    int i, is_write = (flags & MFI_FRAME_DIR_WRITE) ? 1 : 0;
> +
> +    for (i = 0; i < cmd->frame->header.sge_count; i++) {
> +        cpu_physical_memory_unmap(cmd->iov[i].iov_base, cmd->iov[i].iov_len,
> +                                  is_write, cmd->iov[i].iov_len);
> +    }

We cannot map control structures from guest memory and treating them
as valid request state later on.

A malicious guest can issue the request, then change the fields the
control structure while QEMU is processing the I/O, and then this
function will execute with is_write/sge_count no longer the same as
when the request started.

Good practice would be to copy in any request state needed instead of
reaching into guest memory at later points of the request lifecycle.
This way a malicious guest can never cause QEMU to crash or do
something due to inconsistent state.

The particular problem I see here is starting the request with
sge_count=1 and then setting it to sge_count=255.  We will perform
invalid iov[] accesses.

Stefan