From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:58182)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1XP8tW-0002DU-Cv
	for qemu-devel@nongnu.org; Wed, 03 Sep 2014 07:38:40 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1XP8tQ-0002zh-73
	for qemu-devel@nongnu.org; Wed, 03 Sep 2014 07:38:34 -0400
Received: from mx1.redhat.com ([209.132.183.28]:53721)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1XP8tP-0002za-UK
	for qemu-devel@nongnu.org; Wed, 03 Sep 2014 07:38:28 -0400
Message-ID: <5406FD9D.6070207@redhat.com>
Date: Wed, 03 Sep 2014 13:38:05 +0200
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <1409743435-21155-1-git-send-email-famz@redhat.com>
	<1409743435-21155-8-git-send-email-famz@redhat.com>
In-Reply-To: <1409743435-21155-8-git-send-email-famz@redhat.com>
Content-Type: text/plain; charset=iso-8859-15
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v4 07/20] dma: Check iov pointer before
	unmap memory
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: Fam Zheng <famz@redhat.com>, qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, Chrysostomos Nanakos <cnanakos@grnet.gr>, Stefan Hajnoczi <stefanha@redhat.com>, Peter Lieven <pl@kamp.de>, Ronnie Sahlberg <ronniesahlberg@gmail.com>, Liu Yuan <namei.unix@gmail.com>, Josh Durgin <josh.durgin@inktank.com>, MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>, Benoit Canet <benoit@irqsave.net>

Il 03/09/2014 13:23, Fam Zheng ha scritto:
> Not all the iov elements are always valid.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  dma-helpers.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/dma-helpers.c b/dma-helpers.c
> index 499b52b..3655d88 100644
> --- a/dma-helpers.c
> +++ b/dma-helpers.c
> @@ -105,6 +105,9 @@ static void dma_bdrv_unmap(DMAAIOCB *dbs)
>      int i;
>  
>      for (i = 0; i < dbs->iov.niov; ++i) {
> +        if (!(dbs->iov.iov[i].iov_base && dbs->iov.iov[i].iov_len)) {
> +            break;
> +        }
>          dma_memory_unmap(dbs->sg->as, dbs->iov.iov[i].iov_base,
>                           dbs->iov.iov[i].iov_len, dbs->dir,
>                           dbs->iov.iov[i].iov_len);
> 

Why is this needed by this patch series?  Also, the only addition to iov
is here in dma_bdrv_cb:

        mem = dma_memory_map(dbs->sg->as, cur_addr, &cur_len, dbs->dir);
        if (!mem)
            break;
        qemu_iovec_add(&dbs->iov, mem, cur_len);

so iov_base cannot be NULL.

If cur_len is zero after dma_memory_map returns, mem should be NULL too.

If cur_len is zero before dma_memory_map is invoked, address_space_map
(and thus dma_memory_map) will return NULL too.  However, in this case
exiting the loop is wrong.  Perhaps it's better to add an if() in
dma_bdrv_cb that checks for cur_len == 0.

Paolo