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
