[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] memory: move MemoryRegion::size cleanup to memo
From: |
Laszlo Ersek |
Subject: |
Re: [Qemu-devel] [PATCH] memory: move MemoryRegion::size cleanup to memory_region_finalize() |
Date: |
Wed, 10 Oct 2018 21:03:21 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
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