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. Paolo
