diff --git a/libsel4vm/src/guest_memory.c b/libsel4vm/src/guest_memory.c index 48e5be172..2b91b1f47 100644 --- a/libsel4vm/src/guest_memory.c +++ b/libsel4vm/src/guest_memory.c @@ -69,19 +69,18 @@ struct frames_map_iterator_cookie { static inline int reservation_node_cmp(res_tree *x, res_tree *y) { - if (x->addr < y->addr) { - if (x->addr + x->size > y->addr) { - /* The two regions intersect */ - return 0; - } else { - return -1; - } + if (x->addr < y->addr && y->addr - x->addr >= x->size) { + /* x is before y */ + return -1; } - if (x->addr < y->addr + y->size) { - /* The two regions intersect */ - return 0; + + if (x->addr > y->addr && x->addr - y->addr >= y->size) { + /* y is before x */ + return 1; } - return 1; + + /* regions intersect */ + return 0; } SGLIB_DEFINE_RBTREE_PROTOTYPES(res_tree, left, right, color_field, reservation_node_cmp); @@ -263,7 +262,7 @@ static vm_memory_reservation_t *find_anon_reservation_by_addr(uintptr_t addr, si for (int i = 0; i < num_anon_reservations; i++) { vm_memory_reservation_t *curr_res = reservations[i]; - if (curr_res->addr <= addr && curr_res->addr + curr_res->size >= addr + size) { + if (is_subregion(curr_res->addr, curr_res->size, addr, size)) { return curr_res; } } @@ -282,7 +281,7 @@ memory_fault_result_t vm_memory_handle_fault(vm_t *vm, vm_vcpu_t *vcpu, uintptr_ return FAULT_UNHANDLED; } - if ((reservation_node->addr + size) > (reservation_node->addr + reservation_node->size)) { + if (!is_subregion(reservation_node->addr, reservation_node->size, addr, size)) { ZF_LOGE("Failed to handle memory fault: Invalid fault region"); return FAULT_ERROR; } @@ -466,30 +465,42 @@ int vm_free_reserved_memory(vm_t *vm, vm_memory_reservation_t *reservation) int map_vm_memory_reservation(vm_t *vm, vm_memory_reservation_t *vm_reservation, memory_map_iterator_fn map_iterator, void *map_cookie) { - int err; - uintptr_t reservation_addr = vm_reservation->addr; - size_t reservation_size = vm_reservation->size; uintptr_t current_addr = vm_reservation->addr; + size_t bytes_left = vm_reservation->size; - while (current_addr < reservation_addr + reservation_size) { + while (bytes_left) { vm_frame_t reservation_frame = map_iterator(current_addr, map_cookie); + if (reservation_frame.cptr == seL4_CapNull) { - ZF_LOGE("Failed to get frame for reservation address 0x%lx", current_addr); + ZF_LOGE("Failed to get frame for reservation address 0x%"PRIxPTR, current_addr); break; } + + if (bytes_left < BIT(reservation_frame.size_bits)) { + ZF_LOGE("Mapping frame of size %zu to 0x%"PRIxPTR "overflows reservation %zu bytes at 0x%"PRIxPTR, + BIT(reservation_frame.size_bits), current_addr, + vm_reservation->size, vm_reservation->addr); + break; + } + int ret = vspace_deferred_rights_map_pages_at_vaddr(&vm->mem.vm_vspace, &reservation_frame.cptr, NULL, (void *)reservation_frame.vaddr, 1, reservation_frame.size_bits, reservation_frame.rights, vm_reservation->vspace_reservation); if (ret) { ZF_LOGE("Failed to map address 0x%"PRIxPTR" into guest vm vspace", reservation_frame.vaddr); - return -1; + break; } + + /* No underflow here: bytes_left >= frame size, see check above */ + bytes_left -= BIT(reservation_frame.size_bits); current_addr += BIT(reservation_frame.size_bits); } + vm_reservation->memory_map_iterator = NULL; vm_reservation->memory_iterator_cookie = NULL; - vm_reservation->is_mapped = true; - return 0; + vm_reservation->is_mapped = (bytes_left == 0); + + return vm_reservation->is_mapped ? 0 : -1; } int vm_map_reservation(vm_t *vm, vm_memory_reservation_t *reservation, diff --git a/libsel4vm/src/guest_memory.h b/libsel4vm/src/guest_memory.h index 28e55037f..945443543 100644 --- a/libsel4vm/src/guest_memory.h +++ b/libsel4vm/src/guest_memory.h @@ -9,6 +9,21 @@ #include #include +/** + * Check whether a region is a subregion of another region + * @param {uintptr_t} start Start of parent region + * @param {size_t} size Size of parent region + * @param {uintptr_t} subreg_start Start of potential subregion + * @param {size_t} subreg_size Size of potential subregion + * @return True if subregion fits in parent completely, false otherwise + */ +static inline bool is_subregion(uintptr_t start, size_t size, + uintptr_t subreg_start, size_t subreg_size) +{ + return (subreg_start >= start) && (subreg_size <= size) && + (subreg_start - start <= size - subreg_size); +} + /** * Handle a vm memory fault through searching previously created reservations and invoking the appropriate fault callback * @param {vm_t *} vm A handle to the VM diff --git a/libsel4vm/src/guest_ram.c b/libsel4vm/src/guest_ram.c index 5a10b86ba..3ae71f6d0 100644 --- a/libsel4vm/src/guest_ram.c +++ b/libsel4vm/src/guest_ram.c @@ -107,9 +107,10 @@ static bool is_ram_region(vm_t *vm, uintptr_t addr, size_t size) { vm_mem_t *guest_memory = &vm->mem; for (int i = 0; i < guest_memory->num_ram_regions; i++) { - if (guest_memory->ram_regions[i].start <= addr && - guest_memory->ram_regions[i].start + guest_memory->ram_regions[i].size >= addr + size) { - /* We are within a ram region*/ + if (is_subregion(guest_memory->ram_regions[i].start, + guest_memory->ram_regions[i].size, + addr, size)) { + /* We are within RAM region */ return true; } } @@ -208,8 +209,9 @@ void vm_ram_mark_allocated(vm_t *vm, uintptr_t start, size_t bytes) int i; int region = -1; for (i = 0; i < guest_memory->num_ram_regions; i++) { - if (guest_memory->ram_regions[i].start <= start && - guest_memory->ram_regions[i].start + guest_memory->ram_regions[i].size >= start + bytes) { + if (is_subregion(guest_memory->ram_regions[i].start, + guest_memory->ram_regions[i].size, + start, bytes)) { region = i; break; } @@ -299,23 +301,10 @@ static vm_frame_t ram_ut_alloc_iterator(uintptr_t addr, void *cookie) return frame_result; } -static int map_ram_reservation(vm_t *vm, vm_memory_reservation_t *ram_reservation, bool untyped) -{ - int err; - /* We map the reservation immediately, by-passing the deferred mapping functionality - * This allows us the allocate, touch and manipulate VM RAM prior to the region needing to be - * faulted upon first */ - if (untyped) { - err = map_vm_memory_reservation(vm, ram_reservation, ram_ut_alloc_iterator, (void *)vm); - } else { - err = map_vm_memory_reservation(vm, ram_reservation, ram_alloc_iterator, (void *)vm); - } - if (err) { - ZF_LOGE("Failed to map new ram reservation"); - return -1; - } - return 0; -} +/* vm_ram_register() family of functions map the reservation immediately, + * bypassing the deferred mapping functionality. This allows us to allocate, + * touch and manipulate VM RAM before any faults on the region. + */ uintptr_t vm_ram_register(vm_t *vm, size_t bytes) { @@ -330,8 +319,9 @@ uintptr_t vm_ram_register(vm_t *vm, size_t bytes) ZF_LOGE("Unable to reserve ram region of size 0x%zx", bytes); return 0; } - err = map_ram_reservation(vm, ram_reservation, false); + err = map_vm_memory_reservation(vm, ram_reservation, ram_alloc_iterator, vm); if (err) { + ZF_LOGE("Failed to map reservation %zu bytes at 0x%"PRIxPTR, bytes, base_addr); vm_free_reserved_memory(vm, ram_reservation); return 0; } @@ -347,27 +337,9 @@ uintptr_t vm_ram_register(vm_t *vm, size_t bytes) int vm_ram_register_at(vm_t *vm, uintptr_t start, size_t bytes, bool untyped) { - vm_memory_reservation_t *ram_reservation; - int err; - - ram_reservation = vm_reserve_memory_at(vm, start, bytes, default_ram_fault_callback, - NULL); - if (!ram_reservation) { - ZF_LOGE("Unable to reserve ram region at addr 0x%"PRIxPTR" of size 0x%zx", start, bytes); - return -1; - } - err = map_ram_reservation(vm, ram_reservation, untyped); - if (err) { - vm_free_reserved_memory(vm, ram_reservation); - return -1; - } - err = expand_guest_ram_region(vm, start, bytes); - if (err) { - ZF_LOGE("Failed to register new ram region"); - vm_free_reserved_memory(vm, ram_reservation); - return -1; - } - return 0; + return vm_ram_register_at_custom_iterator(vm, start, bytes, + untyped ? ram_ut_alloc_iterator : ram_alloc_iterator, + vm); } int vm_ram_register_at_custom_iterator(vm_t *vm, uintptr_t start, size_t bytes, memory_map_iterator_fn map_iterator, @@ -384,7 +356,7 @@ int vm_ram_register_at_custom_iterator(vm_t *vm, uintptr_t start, size_t bytes, } err = map_vm_memory_reservation(vm, ram_reservation, map_iterator, cookie); if (err) { - ZF_LOGE("failed to map vm memory reservation to dataport\n"); + ZF_LOGE("Failed to map reservation %zu bytes at 0x%"PRIxPTR, bytes, start); return -1; } err = expand_guest_ram_region(vm, start, bytes);