Am 18.02.21 um 15:41 schrieb Peter Maydell:
On Thu, 18 Feb 2021 at 14:13, P J P <ppan...@redhat.com> wrote:
From: Prasad J Pandit <p...@fedoraproject.org>
While processing controller commands, eepro100 emulator gets
command unit(CU) base address OR receive unit (RU) base address
OR command block (CB) address from guest. If these values are not
checked, it may lead to an infinite loop kind of issues. Add checks
to avoid it.
Reported-by: Ruhr-University Bochum <bugs-sys...@rub.de>
Signed-off-by: Prasad J Pandit <p...@fedoraproject.org>
---
hw/net/eepro100.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index 16e95ef9cc..afa1c9b2aa 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -843,7 +843,8 @@ static void action_command(EEPRO100State *s)
bool bit_i;
bool bit_nc;
uint16_t ok_status = STATUS_OK;
- s->cb_address = s->cu_base + s->cu_offset;
+ s->cb_address = s->cu_base + s->cu_offset; /* uint32_t overflow */
+ assert (s->cb_address >= s->cu_base);
We get these values from the guest; you can't just assert() on them.
You need to do something else.
My reading of the 8255x data sheet is that there is nothing
in the hardware that forbids the guest from programming the
device such that the cu_base + cu_offset wraps around:
http://www.intel.com/content/dam/doc/manual/8255x-10-100-mbps-ethernet-controller-software-dev-manual.pdf
-- page 30 says that this is all doing 32-bit arithmetic
on addresses and doesn't say that there is any special case
handling by the device of overflow of that addition.
Your commit message isn't very clear about what the failure
case is here, but I think the fix has to be something
different from this.
I agree with Peter. The hardware emulation in QEMU should try to do the
same as the real hardware.
Assertions (not with assert(), but with g_assert() and related
functions) are good to catch implementation errors in QEMU code, but not
to catch bad programming in guest code.
In this special case the emulation code already includes a hack to catch
endless loops caused by buggy guest code: it has a limit of 16 cycles
and then prints a log message. This is a compromise to emulate the real
hardware as good as possible without making QEMU running an endless loop
or requiring an extra thread to emulate the hardware loop until it is
stopped by new I/O operations.
Stefan
