Re: [PATCH 3/3] KVM: PPC: Fix mmio length message

Nicholas Piggin <[email protected]> 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 <[email protected]>
>
> 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.