On 09/12/18 14:54, Peter Maydell wrote:
> On 12 September 2018 at 13:32, Li Qiang <liq...@gmail.com> wrote:
>> To avoid NULL-deref for the devices without read callbacks
>>
>> Signed-off-by: Li Qiang <liq...@gmail.com>
>> ---
>> memory.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/memory.c b/memory.c
>> index 9b73892768..48d025426b 100644
>> --- a/memory.c
>> +++ b/memory.c
>> @@ -406,6 +406,10 @@ static MemTxResult
>> memory_region_oldmmio_read_accessor(MemoryRegion *mr,
>> {
>> uint64_t tmp;
>>
>> + if (!mr->ops->old_mmio.read[ctz32(size)]) {
>> + return MEMTX_DECODE_ERROR;
>> + }
>> +
>> tmp = mr->ops->old_mmio.read[ctz32(size)](mr->opaque, addr);
>> if (mr->subpage) {
>> trace_memory_region_subpage_read(get_cpu_index(), mr, addr, tmp,
>> size);
>> --
>> 2.11.0
>>
>
> There's patches on-list which drop the old_mmio field from the MemoryRegion
> struct entirely, so I think this patch as it stands is obsolete.
>
> Currently our semantics are "you must provide both read and write, even
> if one of them just always returns 0 / does nothing / returns an error".
That's new to me. Has this always been the case? There are several
static MemoryRegionOps structures that don't conform. (See the end of my
other email:
<84da6f02-1f60-4bc7-92da-6a7f74deded3@redhat.com">http://mid.mail-archive.com/84da6f02-1f60-4bc7-92da-6a7f74deded3@redhat.com>.)
Beyond the one that Li Qiang reported directly ("fw_cfg_ctl_mem_read").
Are all of those ops guest-triggerable QEMU crashers?
> We could probably reasonably assert this at the point when the
> MemoryRegionOps is registered.
Apparently, we should have...
Thanks,
Laszlo
