Skip to content

Commit

Permalink
memory: cleanup side effects of memory_region_init_foo() on failure
Browse files Browse the repository at this point in the history 
if MemoryRegion intialization fails it's left in semi-initialized state,
where it's size is not 0 and attached as child to owner object.
And this leds to crash in following use-case:
    (monitor) object_add memory-backend-file,id=mem1,size=99999G,mem-path=/tmp/foo,discard-data=yes
    memory.c:2083: memory_region_get_ram_ptr: Assertion `mr->ram_block' failed
    Aborted (core dumped)
it happens due to assumption that memory region is intialized when
   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.

Fix it by making sure that memory_region_init_foo() APIs cleanup externally
visible side effects on failure (like set size to 0 and unparenting object)

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <1536064777-42312-1-git-send-email-imammedo@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
  • Loading branch information
@bonzini
Igor Mammedov authored and bonzini committed Oct 2, 2018
1 parent e9b6be9 commit 1cd3d49
Showing 1 changed file with 42 additions and 6 deletions.
48 changes: 42 additions & 6 deletions memory.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1518,12 +1518,18 @@ void memory_region_init_ram_shared_nomigrate(MemoryRegion *mr,
bool share,
Error **errp)
{
Error *err = NULL;
memory_region_init(mr, owner, name, size);
mr->ram = true;
mr->terminates = true;
mr->destructor = memory_region_destructor_ram;
mr->ram_block = qemu_ram_alloc(size, share, mr, errp);
mr->ram_block = qemu_ram_alloc(size, share, mr, &err);
mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
if (err) {
mr->size = int128_zero();
object_unparent(OBJECT(mr));
error_propagate(errp, err);
}
}

void memory_region_init_resizeable_ram(MemoryRegion *mr,
Expand All @@ -1536,13 +1542,19 @@ void memory_region_init_resizeable_ram(MemoryRegion *mr,
void *host),
Error **errp)
{
Error *err = NULL;
memory_region_init(mr, owner, name, size);
mr->ram = true;
mr->terminates = true;
mr->destructor = memory_region_destructor_ram;
mr->ram_block = qemu_ram_alloc_resizeable(size, max_size, resized,
mr, errp);
mr, &err);
mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
if (err) {
mr->size = int128_zero();
object_unparent(OBJECT(mr));
error_propagate(errp, err);
}
}

#ifdef __linux__
Expand All @@ -1555,13 +1567,19 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
const char *path,
Error **errp)
{
Error *err = NULL;
memory_region_init(mr, owner, name, size);
mr->ram = true;
mr->terminates = true;
mr->destructor = memory_region_destructor_ram;
mr->align = align;
mr->ram_block = qemu_ram_alloc_from_file(size, mr, ram_flags, path, errp);
mr->ram_block = qemu_ram_alloc_from_file(size, mr, ram_flags, path, &err);
mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
if (err) {
mr->size = int128_zero();
object_unparent(OBJECT(mr));
error_propagate(errp, err);
}
}

void memory_region_init_ram_from_fd(MemoryRegion *mr,
Expand All @@ -1572,14 +1590,20 @@ void memory_region_init_ram_from_fd(MemoryRegion *mr,
int fd,
Error **errp)
{
Error *err = NULL;
memory_region_init(mr, owner, name, size);
mr->ram = true;
mr->terminates = true;
mr->destructor = memory_region_destructor_ram;
mr->ram_block = qemu_ram_alloc_from_fd(size, mr,
share ? RAM_SHARED : 0,
fd, errp);
fd, &err);
mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
if (err) {
mr->size = int128_zero();
object_unparent(OBJECT(mr));
error_propagate(errp, err);
}
}
#endif

Expand Down Expand Up @@ -1630,13 +1654,19 @@ void memory_region_init_rom_nomigrate(MemoryRegion *mr,
uint64_t size,
Error **errp)
{
Error *err = NULL;
memory_region_init(mr, owner, name, size);
mr->ram = true;
mr->readonly = true;
mr->terminates = true;
mr->destructor = memory_region_destructor_ram;
mr->ram_block = qemu_ram_alloc(size, false, mr, errp);
mr->ram_block = qemu_ram_alloc(size, false, mr, &err);
mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
if (err) {
mr->size = int128_zero();
object_unparent(OBJECT(mr));
error_propagate(errp, err);
}
}

void memory_region_init_rom_device_nomigrate(MemoryRegion *mr,
Expand All @@ -1647,14 +1677,20 @@ void memory_region_init_rom_device_nomigrate(MemoryRegion *mr,
uint64_t size,
Error **errp)
{
Error *err = NULL;
assert(ops);
memory_region_init(mr, owner, name, size);
mr->ops = ops;
mr->opaque = opaque;
mr->terminates = true;
mr->rom_device = true;
mr->destructor = memory_region_destructor_ram;
mr->ram_block = qemu_ram_alloc(size, false, mr, errp);
mr->ram_block = qemu_ram_alloc(size, false, mr, &err);
if (err) {
mr->size = int128_zero();
object_unparent(OBJECT(mr));
error_propagate(errp, err);
}
}

void memory_region_init_iommu(void *_iommu_mr,
Expand Down

0 comments on commit 1cd3d49

Please sign in to comment.