Re: [Qemu-devel] [PATCH 5/5] megasas: LSI Megaraid SAS emulation
On 07/05/2011 05:21 PM, Stefan Hajnoczi wrote:
On Tue, Jul 5, 2011 at 12:03 PM, Hannes Reineckeh...@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.
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
h...@suse.de +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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 5/5] megasas: LSI Megaraid SAS emulation
On 07/06/2011 08:20 AM, Hannes Reinecke wrote: 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. Note the flags too. Perhaps it's easier to simply copy the entire frame header... Paolo -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 5/5] megasas: LSI Megaraid SAS emulation
On Tue, Jul 5, 2011 at 12:03 PM, Hannes Reinecke h...@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
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
