Re: [Qemu-devel] [PATCH] eepro100: prevent an infinite loop over same command block

[Jason Wang <jasowang@redhat.com>]

On 10/17/2015 01:19 AM, P J P wrote:
> +-- On Fri, 16 Oct 2015, Paolo Bonzini wrote --+
> | > +        if (s->tx.link == s->cu_offset)
> | > +            break;
> | 
> | Please update the patch to conform to QEMU's coding standards; braces
> | are required even around single-statement blocks.
>
>   Done. Please see an updated patch below.
>
> ===
> From bbf7b8914a984b09242e1cafc258bd71cecc47c8 Mon Sep 17 00:00:00 2001
> From: Prasad J Pandit <pjp@fedoraproject.org>
> Date: Fri, 16 Oct 2015 22:43:29 +0530
> Subject: eepro100: prevent an infinite loop over same command block
>
> action_command() routine executes a chain of commands located
> in the Command Block List(CBL). Each Command Block(CB) has a
> link to the next CB in the list, given by 's->tx.link'.
> This is used in conjunction with the base address 's->cu_base'.
>
> An infinite loop unfolds if the 'link' to the next CB is
> same as the previous one, the loop ends up executing the same
> command over and over again.
>
> Reported-by: Qinghao Tang <luodalongde@gmail.com>
> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
> ---
>  hw/net/eepro100.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
> index 60333b7..0e4ad4e 100644
> --- a/hw/net/eepro100.c
> +++ b/hw/net/eepro100.c
> @@ -863,6 +863,9 @@ static void action_command(EEPRO100State *s)
>          uint16_t ok_status = STATUS_OK;
>          s->cb_address = s->cu_base + s->cu_offset;
>          read_cb(s);
> +        if (s->tx.link == s->cu_offset) {
> +            break;
> +        }
>          bit_el = ((s->tx.command & COMMAND_EL) != 0);
>          bit_s = ((s->tx.command & COMMAND_S) != 0);
>          bit_i = ((s->tx.command & COMMAND_I) != 0);

Can this survive if we had a chain like?

A->B->A

If not, looks like we need to limit the maximum number of commands in a
chain? (e.g 256)