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

[Stefan Weil <sw@weilnetz.de>]

Am 16.10.2015 um 19:19 schrieb P J P:
> +-- 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(+)
>

Hi,

is this just a theoretical assumption or did you see problems
with some guest operating system?

To trigger a potential infinite loop, you'll need buggy device
drivers in the guest.

I just had a look at the Intel developer manual
(http://www.intel.com/content/dam/doc/manual/8255x-10-100-mbps-ethernet-controller-software-dev-manual.pdf).
The command block list (CBL) description is in chapter 6.4
of that manual. I did not find a hint that there is a break
condition like the one introduced by your patch.

Maybe real hardware will run an endless loop?
Or the "endless" loop is terminated because the driver
changes the link while the loop is running?

The goal of eepro100.c should be emulation of the
real hardware, even of a potential design weakness.

Regards
Stefan W.