Re: [Qemu-devel] [PATCH] memory: move MemoryRegion::size cleanup to memory_region_finalize()

[Laszlo Ersek]

On 10/10/18 20:56, Paolo Bonzini wrote:
> On 09/10/2018 19:09, Laszlo Ersek wrote:
>>>    memory_region_size() != 0
>>> and therefore it's ok to access it in
>>>    file_backend_unparent()
>>>       if (memory_region_size() != 0)
>>>           memory_region_get_ram_ptr()
>>>
>>> which happens when object_add fails and unparents failed backend making
>>> file_backend_unparent() access invalid memory region.
>>
>> I think it makes sense to zero out the size even if unparenting
>> would, in itself, prevent the above crash. Because, in
>> host_memory_backend_mr_inited(), we have:
>>
>>     /*
>>      * NOTE: We forbid zero-length memory backend, so here zero means
>>      * "we haven't inited the backend memory region yet".
>>      */
>>
>> I'm unsure how general that invariant is, but it can't hurt to honor
>> it everywhere. (Especially if we can do the zeroing in one common
>> place.)
> 
> Yeah, that's the part that I'm not sure about.  If we do it in finalize,
> no one should be able to observe that we are zeroing it; finalize runs
> just before the object is g_free-d.  I agree with Igor that it's nicer
> to leave the object in good state, but the right place to zero is
> exactly where the first patch placed it, i.e. where the error is
> detected and the initialization of memory_region_init is unwound.

OK.

Thanks
Laszlo