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 <p...@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 <luodalon...@gmail.com>
> Signed-off-by: Prasad J Pandit <p...@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;
> }
> }
