Re: [PATCH] net: eepro100: validate various address values

[Stefan Weil]

Am 19.02.21 um 07:11 schrieb P J P:

>    Hello Alex, Stefan, all
>
> +-- On Thu, 18 Feb 2021, Alexander Bulekov wrote --+
> | Maybe the infinite loop mentioned in the commit message is actually a DMA
> | recursion issue? I'm providing a reproducer for a DMA re-entracy issue
> | below. With this patch applied, the reproducer triggers the assert(), rather
> | than overflowing the stack, so maybe it is the same issue? -Alex
> |
> | cat << EOF | ./qemu-system-i386 -display none -machine accel=qtest, -m \
> | 512M -device i82559er,netdev=net0 -netdev user,id=net0 -nodefaults \
> | -qtest stdio
> | outl 0xcf8 0x80001014
> | outl 0xcfc 0xc000
> | outl 0xcf8 0x80001010
> | outl 0xcfc 0xe0020000
> | outl 0xcf8 0x80001004
> | outw 0xcfc 0x7
> | write 0x1ffffc0b 0x1 0x55
> | write 0x1ffffc0c 0x1 0xfc
> | write 0x1ffffc0d 0x1 0x46
> | write 0x1ffffc0e 0x1 0x07
> | write 0x746fc59 0x1 0x02
> | write 0x746fc5b 0x1 0x02
> | write 0x746fc5c 0x1 0xe0
> | write 0x4 0x1 0x07
> | write 0x5 0x1 0xfc
> | write 0x6 0x1 0xff
> | write 0x7 0x1 0x1f
> | outw 0xc002 0x20
> | EOF
> |
>
> * Yes, it is an infinite recursion induced stack overflow. I should've said
>    recursion instead of loop.
>
>    Thank you for sharing a reproducer and the stack trace.


Okay, I can confirm the infinite recursion now.

The test case triggers memory writes by the hardware which cause new 
actions of the same hardware and so on.

I don't know how the real hardware would handle that case.

For QEMU we can extend the current code which tries to prevent endless 
loops: the device status EEPRO100State can be extended by a recursion 
counter to limit the number of recursions, or maybe a boolean flag could 
be used to stop any recursion of action_command(). I prefer the second 
variant (no recursion at all) and suggest to add a diagnostic message as 
well like it is done for the endless loop case.

Stefan