On 07/05/2011 05:21 PM, Stefan Hajnoczi wrote:
On Tue, Jul 5, 2011 at 12:03 PM, Hannes Reinecke<hare@xxxxxxx> 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.
Yes, I've been working on that one already.
What I'll be doing is to read in the sge count during 'map_sgl' and
store this value internally (in ->iov_cnt). And during unmap I'll be
using this value instead of the frame-provided one.
That way we'll be checking the sge_count field only once when we
slurp in the entire frame.
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.
See above, that's what I'll be doing.
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.
Thanks for the hint. Will be fixing it up.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@xxxxxxx +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
