From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:56006)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1aBRQz-0000h6-Ni
	for qemu-devel@nongnu.org; Tue, 22 Dec 2015 13:13:18 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1aBRQw-00046m-I1
	for qemu-devel@nongnu.org; Tue, 22 Dec 2015 13:13:17 -0500
Received: from mail-wm0-x22d.google.com ([2a00:1450:400c:c09::22d]:33366)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1aBRQw-00046d-BV
	for qemu-devel@nongnu.org; Tue, 22 Dec 2015 13:13:14 -0500
Received: by mail-wm0-x22d.google.com with SMTP id p187so118201810wmp.0
	for <qemu-devel@nongnu.org>; Tue, 22 Dec 2015 10:13:14 -0800 (PST)
Sender: Paolo Bonzini <paolo.bonzini@gmail.com>
References: <alpine.LFD.2.20.1512221830330.5001@wniryva>
	<56796238.9070803@redhat.com>
	<alpine.LFD.2.20.1512222248450.5574@wniryva>
From: Paolo Bonzini <pbonzini@redhat.com>
Message-ID: <567992B7.1020408@redhat.com>
Date: Tue, 22 Dec 2015 19:13:11 +0100
MIME-Version: 1.0
In-Reply-To: <alpine.LFD.2.20.1512222248450.5574@wniryva>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH] net: rocker: fix an incorrect array bounds
	check
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: Qinghao Tang <luodalongde@gmail.com>, Jiri Pirko <jiri@resnulli.us>, Scott Feldman <sfeldma@gmail.com>, qemu-devel@nongnu.org



On 22/12/2015 18:26, P J P wrote:
> +-- On Tue, 22 Dec 2015, Paolo Bonzini wrote --+
> | > -        if (++iovcnt > ROCKER_TX_FRAGS_MAX) {
> | > +        if (++iovcnt >= ROCKER_TX_FRAGS_MAX) {
> | 
> | Doesn't this forbid some valid ROCKER_TX_FRAGS_MAX-element iovecs?
> 
>   forbid..?  Sorry, I did not get the question.
> 
> | The check should be moved before the assignment to iov[iovcnt].iov_len.
> 
>   You mean like below..?
> 
> ===
> diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
> index c57f1a6..2e77e50 100644
> --- a/hw/net/rocker/rocker.c
> +++ b/hw/net/rocker/rocker.c
> @@ -232,6 +232,9 @@ static int tx_consume(Rocker *r, DescInfo *info)
>          frag_addr = rocker_tlv_get_le64(tlvs[ROCKER_TLV_TX_FRAG_ATTR_ADDR]);
>          frag_len = rocker_tlv_get_le16(tlvs[ROCKER_TLV_TX_FRAG_ATTR_LEN]);
>  
> +        if (iovcnt >= ROCKER_TX_FRAGS_MAX) {
> +            goto err_too_many_frags;
> +        }
>          iov[iovcnt].iov_len = frag_len;
>          iov[iovcnt].iov_base = g_malloc(frag_len);
>          if (!iov[iovcnt].iov_base) {
> @@ -244,10 +247,7 @@ static int tx_consume(Rocker *r, DescInfo *info)
>              err = -ROCKER_ENXIO;
>              goto err_bad_io;
>          }
> -
> -        if (++iovcnt > ROCKER_TX_FRAGS_MAX) {
> -            goto err_too_many_frags;
> -        }
> +        iovcnt++;
>      }
>  
>      if (iovcnt) {
> ===
> 
> Thank you.
> --
> Prasad J Pandit / Red Hat Product Security Team
> 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
> 
> 

Yes, can you submit it as v2?

Paolo