The MIPS GIC does a check for a guest error in the write path for the SH_MAP*_VP registers which triggers a Coverity complaint because it assigns -1 to a uint64_t. The code doesn't misbehave because the -1 case will be caught by the following OFFSET_CHECK(), but the code could be improved: * there is no need to special case to avoid passing 0 to ctz64(), because (unlike the compiler builtins) QEMU defines that this has a specific behaviour, returning 64 * the OFFSET_CHECK() macro will go to the "bad_offset" label and print an error implying that the guest wrote to an invalid register offset. This is misleading about the actual problem, which is that the guest wrote a bogus value to a valid register offset
Make the error check print a better log message, and avoid the special casing on ctz64(); in passing, this should also make Coverity happier. CID: 1547545 Signed-off-by: Peter Maydell <[email protected]> --- hw/intc/mips_gic.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/hw/intc/mips_gic.c b/hw/intc/mips_gic.c index ad9363a4c7..d337a76603 100644 --- a/hw/intc/mips_gic.c +++ b/hw/intc/mips_gic.c @@ -326,9 +326,13 @@ static void gic_write(void *opaque, hwaddr addr, uint64_t data, unsigned size) /* up to 32 bytes per a pin */ irq_src = (addr - GIC_SH_MAP0_VP_OFS) / 32; OFFSET_CHECK(irq_src < gic->num_irq); - data = data ? ctz64(data) : -1; - OFFSET_CHECK(data < gic->num_vps); - gic->irq_state[irq_src].map_vp = data; + if (ctz64(data) >= gic->num_vps) { + qemu_log_mask(LOG_GUEST_ERROR, "Bad data value 0x%" PRIx64 + " at MAP VP register offset 0x%" PRIx64 "\n", + data, addr); + break; + } + gic->irq_state[irq_src].map_vp = ctz64(data); break; case VP_LOCAL_SECTION_OFS ... (VP_LOCAL_SECTION_OFS + GIC_VL_BRK_GROUP): gic_write_vp(gic, vp_index, addr - VP_LOCAL_SECTION_OFS, data, size); -- 2.43.0
