From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Hajnoczi Subject: Re: [Qemu-devel] [PATCH 5/5] megasas: LSI Megaraid SAS emulation Date: Tue, 5 Jul 2011 16:21:41 +0100 Message-ID: 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> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: qemu-devel@nongnu.org, Paolo Bonzini , Stefan Haynoczi , kvm@vger.kernel.org, Alexander Graf To: Hannes Reinecke Return-path: Received: from mail-yi0-f46.google.com ([209.85.218.46]:60993 "EHLO mail-yi0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754535Ab1GEPVm convert rfc822-to-8bit (ORCPT ); Tue, 5 Jul 2011 11:21:42 -0400 Received: by yia27 with SMTP id 27so2285125yia.19 for ; Tue, 05 Jul 2011 08:21:41 -0700 (PDT) In-Reply-To: <1309863815-28236-6-git-send-email-hare@suse.de> Sender: kvm-owner@vger.kernel.org List-ID: On Tue, Jul 5, 2011 at 12:03 PM, Hannes Reinecke wrote: > +static void megasas_unmap_sgl(struct megasas_cmd_t *cmd) > +{ > + =A0 =A0uint16_t flags =3D le16_to_cpu(cmd->frame->header.flags); > + =A0 =A0int i, is_write =3D (flags & MFI_FRAME_DIR_WRITE) ? 1 : 0; > + > + =A0 =A0for (i =3D 0; i < cmd->frame->header.sge_count; i++) { > + =A0 =A0 =A0 =A0cpu_physical_memory_unmap(cmd->iov[i].iov_base, cmd-= >iov[i].iov_len, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= is_write, cmd->iov[i].iov_len); > + =A0 =A0} 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=3D1 and then setting it to sge_count=3D255. We will perform invalid iov[] accesses. Stefan From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:54364) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qe7S3-0000ko-96 for qemu-devel@nongnu.org; Tue, 05 Jul 2011 11:22:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Qe7RW-0001t4-Gy for qemu-devel@nongnu.org; Tue, 05 Jul 2011 11:22:14 -0400 Received: from mail-yx0-f173.google.com ([209.85.213.173]:59280) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qe7RW-0001sl-44 for qemu-devel@nongnu.org; Tue, 05 Jul 2011 11:21:42 -0400 Received: by yxt3 with SMTP id 3so2383810yxt.4 for ; Tue, 05 Jul 2011 08:21:41 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1309863815-28236-6-git-send-email-hare@suse.de> 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> Date: Tue, 5 Jul 2011 16:21:41 +0100 Message-ID: From: Stefan Hajnoczi Content-Type: text/plain; charset=ISO-8859-1 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: Hannes Reinecke Cc: Paolo Bonzini , Alexander Graf , qemu-devel@nongnu.org, kvm@vger.kernel.org, Stefan Haynoczi On Tue, Jul 5, 2011 at 12:03 PM, Hannes Reinecke wrote: > +static void megasas_unmap_sgl(struct megasas_cmd_t *cmd) > +{ > + =A0 =A0uint16_t flags =3D le16_to_cpu(cmd->frame->header.flags); > + =A0 =A0int i, is_write =3D (flags & MFI_FRAME_DIR_WRITE) ? 1 : 0; > + > + =A0 =A0for (i =3D 0; i < cmd->frame->header.sge_count; i++) { > + =A0 =A0 =A0 =A0cpu_physical_memory_unmap(cmd->iov[i].iov_base, cmd->iov= [i].iov_len, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0is_w= rite, cmd->iov[i].iov_len); > + =A0 =A0} 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=3D1 and then setting it to sge_count=3D255. We will perform invalid iov[] accesses. Stefan