From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:42396)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1aBOCm-0000LM-S3
	for qemu-devel@nongnu.org; Tue, 22 Dec 2015 09:46:25 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1aBOCh-0007ZX-0g
	for qemu-devel@nongnu.org; Tue, 22 Dec 2015 09:46:24 -0500
Received: from mail-wm0-x236.google.com ([2a00:1450:400c:c09::236]:33700)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1aBOCg-0007ZT-Qr
	for qemu-devel@nongnu.org; Tue, 22 Dec 2015 09:46:18 -0500
Received: by mail-wm0-x236.google.com with SMTP id p187so110961832wmp.0
	for <qemu-devel@nongnu.org>; Tue, 22 Dec 2015 06:46:18 -0800 (PST)
Sender: Paolo Bonzini <paolo.bonzini@gmail.com>
References: <alpine.LFD.2.20.1512221830330.5001@wniryva>
From: Paolo Bonzini <pbonzini@redhat.com>
Message-ID: <56796238.9070803@redhat.com>
Date: Tue, 22 Dec 2015 15:46:16 +0100
MIME-Version: 1.0
In-Reply-To: <alpine.LFD.2.20.1512221830330.5001@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>, qemu-devel@nongnu.org
Cc: Qinghao Tang <luodalongde@gmail.com>, Scott Feldman <sfeldma@gmail.com>, Jiri Pirko <jiri@resnulli.us>



On 22/12/2015 14:07, P J P wrote:
>   Hello Scott, Jiri
> 
> A stack overflow issue was reported by Mr Qinghao Tang, CC'd here. It
> occurs while processing transmit(tx) descriptors in tx_consume()
> routine. If a descriptor was to have more than
> allowed(ROCKER_TX_FRAGS_MAX=16) packet fragments, the processing loop
> suffers an off-by-one error. Thus leading to OOB memory access and
> leakage of host memory.
> 
> Please see below a proposed patch to fix this issue. Does it look okay?
> 
> ===
> From f3461d8098a0572786f5a2d7a492863090c73134 Mon Sep 17 00:00:00 2001
> From: Prasad J Pandit <pjp@fedoraproject.org>
> Date: Tue, 22 Dec 2015 18:21:00 +0530
> Subject: [PATCH] net: rocker: fix an incorrect array bounds check
> 
> While processing transmit(tx) descriptors in 'tx_consume' routine
> the switch emulator suffers from an off-by-one error, if a
> descriptor was to have more than allowed(ROCKER_TX_FRAGS_MAX)
> fragments. Fix an incorrect bounds check to avoid it.
> 
> Reported-by: Qinghao Tang <luodalongde@gmail.com>
> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
> ---
>  hw/net/rocker/rocker.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
> index c57f1a6..05102bd 100644
> --- a/hw/net/rocker/rocker.c
> +++ b/hw/net/rocker/rocker.c
> @@ -245,7 +245,7 @@ static int tx_consume(Rocker *r, DescInfo *info)
>              goto err_bad_io;
>          }
> 
> -        if (++iovcnt > ROCKER_TX_FRAGS_MAX) {
> +        if (++iovcnt >= ROCKER_TX_FRAGS_MAX) {

Doesn't this forbid some valid ROCKER_TX_FRAGS_MAX-element iovecs?  The
check should be moved before the assignment to iov[iovcnt].iov_len.

Paolo

>              goto err_too_many_frags;
>          }
>      }