Re: [Qemu-devel] [PATCH for-2.5] eepro100: Prevent two endless loops

[Stefan Weil]

Am 20.11.2015 um 09:39 schrieb P J P:
> +-- On Fri, 20 Nov 2015, Stefan Weil wrote --+
> | diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
> | index 60333b7..685a478 100644
> | --- a/hw/net/eepro100.c
> | +++ b/hw/net/eepro100.c
> | @@ -774,6 +774,11 @@ static void tx_command(EEPRO100State *s)
> |  #if 0
> |          uint16_t tx_buffer_el = lduw_le_pci_dma(&s->dev, tbd_address + 6);
> |  #endif
> | +        if (tx_buffer_size == 0) {
> | +            /* Prevent an endless loop. */
> | +            logout("loop in %s:%u\n", __FILE__, __LINE__);
> | +            break;
> | +        }
>
>    Yes, looks good. Where is 'lduw_le_pci_dma' routine defined?

include/hw/pci/pci.h:    static inline uint##_bits##_t
ld##_l##_pci_dma(PCIDevice *dev,      \

>
> |  static void action_command(EEPRO100State *s)
> |  {
> | +    /* The loop below won't stop if it gets special handcrafted data.
> | +       Therefore we limit the number of iterations. */
> | +    unsigned max_loop_count = 16;
> | +
>
>    Is '16' a lower count for the command list? Earilier Jason mentioned 256. 
> Not sure what is an ideal count.

Is there an ideal count? If it is too low, it might break some use cases.
If it is too high, it will take longer until the loop is finished.

I don't think EEPRO100 emulation is used in critical production
applications.
Therefore a lower value and a debug message when this value is exceeded
might be helpful to find out which lowest value is acceptable. If you want
to avoid this risk, the value should be set to 256, 10000, 65536 or any
other higher value. Feel free to change this when you apply the patch.

Stefan