From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:46047) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QeLTt-0007H8-AI for qemu-devel@nongnu.org; Wed, 06 Jul 2011 02:21:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QeLTr-0005mL-E7 for qemu-devel@nongnu.org; Wed, 06 Jul 2011 02:21:05 -0400 Received: from cantor2.suse.de ([195.135.220.15]:50818 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QeLTq-0005m4-MV for qemu-devel@nongnu.org; Wed, 06 Jul 2011 02:21:03 -0400 Message-ID: <4E13FECA.10101@suse.de> Date: Wed, 06 Jul 2011 08:20:58 +0200 From: Hannes Reinecke MIME-Version: 1.0 References: <1309863815-28236-1-git-send-email-hare@suse.de> <1309863815-28236-2-git-send-email-hare@suse.de> <1309863815-28236-3-git-send-email-hare@suse.de> <1309863815-28236-4-git-send-email-hare@suse.de> <1309863815-28236-5-git-send-email-hare@suse.de> <1309863815-28236-6-git-send-email-hare@suse.de> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 5/5] megasas: LSI Megaraid SAS emulation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Paolo Bonzini , Alexander Graf , qemu-devel@nongnu.org, kvm@vger.kernel.org, Stefan Haynoczi On 07/05/2011 05:21 PM, Stefan Hajnoczi wrote: > On Tue, Jul 5, 2011 at 12:03 PM, Hannes Reinecke wrote: >> +static void megasas_unmap_sgl(struct megasas_cmd_t *cmd) >> +{ >> + uint16_t flags =3D le16_to_cpu(cmd->frame->header.flags); >> + int i, is_write =3D (flags& MFI_FRAME_DIR_WRITE) ? 1 : 0; >> + >> + for (i =3D 0; i< cmd->frame->header.sge_count; i++) { >> + cpu_physical_memory_unmap(cmd->iov[i].iov_base, cmd->iov[i].i= ov_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=20 store this value internally (in ->iov_cnt). And during unmap I'll be=20 using this value instead of the frame-provided one. That way we'll be checking the sge_count field only once when we=20 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=3D1 and then setting it to sge_count=3D255. We will perform > invalid iov[] accesses. > Thanks for the hint. Will be fixing it up. Cheers, Hannes --=20 Dr. Hannes Reinecke zSeries & Storage hare@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg GF: J. Hawn, J. Guild, F. Imend=C3=B6rffer, HRB 16746 (AG N=C3=BCrnberg)