Nicholas Piggin <npiggin@xxxxxxxxx> writes: > Excerpts from Fabiano Rosas's message of December 24, 2021 7:15 am: >> We check against 'bytes' but print 'run->mmio.len' which at that point >> has an old value. >> >> e.g. 16-byte load: >> >> before: >> __kvmppc_handle_load: bad MMIO length: 8 >> >> now: >> __kvmppc_handle_load: bad MMIO length: 16 >> >> Signed-off-by: Fabiano Rosas <farosas@xxxxxxxxxxxxx> > > This patch fine, but in the case of overflow we continue anyway here. > Can that overwrite some other memory in the kvm_run struct? I tested this and QEMU will indeed overwrite the subsequent fields of kvm_run. A `lq` on this data: mmio_test_data: .long 0xdeadbeef .long 0x8badf00d .long 0x1337c0de .long 0x01abcdef produces: __kvmppc_handle_load: bad MMIO length: 16 kvmppc_complete_mmio_load data: 0x8badf00ddeadbeef bad MMIO length: 322420958 <-- mmio.len got nuked But then we return from kvmppc_complete_mmio_load without writing to the registers. > > This is familiar, maybe something Alexey has noticed in the past too? > What was the consensus on fixing it? (at least it should have a comment > if it's not a problem IMO) My plan was to just add quadword support. And whatever else might missing. But I got sidetracked with how to test this so I'm just now coming back to it. Perhaps a more immediate fix is needed before that? We could block loads and stores larger than 8 bytes earlier at kvmppc_emulate_loadstore for instance.
