From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:38446)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <agraf@suse.de>) id 1b5XNZ-0008Vy-QB
	for qemu-devel@nongnu.org; Wed, 25 May 2016 07:53:38 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <agraf@suse.de>) id 1b5XNW-0007qX-La
	for qemu-devel@nongnu.org; Wed, 25 May 2016 07:53:37 -0400
Received: from mx2.suse.de ([195.135.220.15]:39263)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <agraf@suse.de>) id 1b5XNW-0007ps-Ev
	for qemu-devel@nongnu.org; Wed, 25 May 2016 07:53:34 -0400
References: <1464172291-2856-1-git-send-email-ppandit@redhat.com>
	<1464172291-2856-2-git-send-email-ppandit@redhat.com>
	<5745873C.90400@suse.de> <alpine.LFD.2.20.1605251718250.15974@wniryva>
From: Alexander Graf <agraf@suse.de>
Message-ID: <5745923B.4060102@suse.de>
Date: Wed, 25 May 2016 13:53:31 +0200
MIME-Version: 1.0
In-Reply-To: <alpine.LFD.2.20.1605251718250.15974@wniryva>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 1/3] scsi: megasas: use appropriate
 property buffer size
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: P J P <ppandit@redhat.com>
Cc: Qemu Developers <qemu-devel@nongnu.org>, Paolo Bonzini <pbonzini@redhat.com>, Li Qiang <liqiang6-s@360.cn>, Hannes Reinecke <hare@suse.de>

On 05/25/2016 01:51 PM, P J P wrote:
>    Hello Alex,
>
> +-- On Wed, 25 May 2016, Alexander Graf wrote --+
> | > -    dma_buf_write((uint8_t *)&info, cmd->iov_size, &cmd->qsg);
> | > +    dma_buf_write((uint8_t *)&info, dcmd_size, &cmd->qsg);
> |
> | This looks odd - can dcmd_size be bigger than iov_size? Wouldn't we overwrite
> | guest memory then? And where does dcmd_size come from? I don't see it in
> | master.
>
>    struct mfi_ctrl_props info;
>    size_t dcmd_size = sizeof(info);
>    
>    -> http://git.qemu.org/?p=qemu.git;a=blob;f=hw/scsi/megasas.c;h=a63a581550a328d0326ddee4f7fe1c4ffdecc194;hb=HEAD#l1439
>
> 'dcmd_size' is same as that of 'info' object.

Ok, then this patch is definitely bogus. The guest may receive less than 
the size of the info object. So we really want to have a MIN() between 
the maximum allowed transfer size (sizeof(info)) and the requested size 
(cmd->iov_size) here.

Alex