Re: [RFC PATCH-for-5.1? v3 1/2] memory: Allow monkey-patching MemoryRegion access sizes

[Philippe Mathieu-Daudé]

On 7/21/20 2:33 PM, Peter Maydell wrote:
> On Tue, 21 Jul 2020 at 13:31, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> To fixes CVE-2020-13754, commit 5d971f9e67 refuses mismatching
>> sizes in memory_region_access_valid(). This gives troubles when
>> a device is on an ISA bus, because the CPU is free to use
>> 8/16-bit accesses on the bus (or up to 32-bit on EISA bus),
>> regardless what range is valid for the device.
>>
>> To allow surgical change for the 5.1 release, allow monkey
>> patching of the MemoryRegionOps (by making the MemoryRegion
>> field not const). This should be reverted after the release
>> and fixed in a more elegant manner.
>>
>> Fixes: 5d971f9e67 ('memory: Revert "accept mismatching sizes in 
>> memory_region_access_valid"')
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  include/exec/memory.h |  7 ++++++-
>>  softmmu/memory.c      | 12 ++++++++----
>>  2 files changed, 14 insertions(+), 5 deletions(-)
>>
>> diff --git a/softmmu/memory.c b/softmmu/memory.c
>> index 9200b20130..84b5c617e2 100644
>> --- a/softmmu/memory.c
>> +++ b/softmmu/memory.c
>> @@ -1218,7 +1218,7 @@ static void memory_region_initfn(Object *obj)
>>      MemoryRegion *mr = MEMORY_REGION(obj);
>>      ObjectProperty *op;
>>
>> -    mr->ops = &unassigned_mem_ops;
>> +    mr->ops = g_memdup(&unassigned_mem_ops, sizeof(MemoryRegionOps));
>>      mr->enabled = true;
>>      mr->romd_mode = true;
>>      mr->global_locking = true;
> 
> Don't you now need to g_memfree() mr->ops somewhere? Otherwise
> you've leaked it if the device which owned this MemoryRegion
> is hot-unplugged, I think.

I haven't thinking of hot-unplug. I went with the simplest fix
considering we are in freeze, and fixing the bug was more
important that a leak at this point.
I'll have a look at freeing this memory, hoping it is still less
disruptive than a proper architectural change to fix this problem.

> 
> thanks
> -- PMM
>