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. +-- On Thu, 18 Feb 2021, Stefan Weil wrote --+ | Am 18.02.21 um 15:41 schrieb Peter Maydell: || + 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. | > http://www.intel.com/content/dam/doc/manual/8255x-10-100-mbps-ethernet-controller-software-dev-manual.pdf | | I agree with Peter. The hardware emulation in QEMU should try to do the same | as the real hardware. * Agreed. * While the manual does not say how to handle uint32_t overflow in above 'cb_address' calculation, I doubt if overflow is expected. + if (s->cb_address < s->cu_base) { + logout ("invalid cb_address: %s: %u\n", __func__, s->cb_address); + break; + } I tried above fix first, it does not seem to arrest the recurssion induced stack overflow. Hence assert(3). * I also tried to find out if there's any cap on the 'cu_offset' value OR number of command units (cu) that can be addressed. But in linear addressing mode offset is a 32bit value with base address set to zero(0). And in 32bit segmented addressing mode 'offset' is 16bit value with non-zero(0) base address. ie. maximum offset could be about ~4K for 16byte command block IIUC. I'm not sure if segmented addressing mode is supported. * I'd appreciate if you could suggest a right way to fix it OR propose/post another patch. I'm okay either way. Thank you. -- Prasad J Pandit / Red Hat Product Security Team 8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D
