From db3a647c6d8075305e60d637b515df4a52477de2 Mon Sep 17 00:00:00 2001 From: Johannes Schlatow Date: Mon, 31 Jan 2022 15:51:11 +0100 Subject: [PATCH] allocator_avl: use Attempt for size_at Fixes ambiguous interpretation of returned 0. genodelabs/genode#4393 --- repos/base-nova/src/core/include/platform.h | 10 ++++++-- repos/base-sel4/src/core/include/platform.h | 10 ++++++-- repos/base/include/base/allocator_avl.h | 11 +++++++- repos/base/src/lib/base/allocator_avl.cc | 9 ++++--- repos/base/src/lib/base/heap.cc | 25 ++++++++++++++----- repos/dde_linux/src/lib/lx_kit/memory.cc | 15 ++++++++--- .../src/lib/libc/internal/mem_alloc.h | 7 ++++-- repos/libports/src/lib/libc/libc_mem_alloc.cc | 2 +- repos/libports/src/lib/libc/vfs_plugin.cc | 15 +++++++++-- repos/ports/src/virtualbox5/libc.cc | 6 ++++- 10 files changed, 86 insertions(+), 24 deletions(-) diff --git a/repos/base-nova/src/core/include/platform.h b/repos/base-nova/src/core/include/platform.h index 71ea0d15fc..d992f32b40 100644 --- a/repos/base-nova/src/core/include/platform.h +++ b/repos/base-nova/src/core/include/platform.h @@ -104,8 +104,14 @@ namespace Genode { * Determine size of a core local mapping required for a * core_rm_session detach(). */ - size_t region_alloc_size_at(void * addr) { - return (_core_mem_alloc.virt_alloc())()->size_at(addr); } + size_t region_alloc_size_at(void * addr) + { + using Size_at_error = Allocator_avl::Size_at_error; + + return (_core_mem_alloc.virt_alloc())()->size_at(addr).convert( + [ ] (size_t s) { return s; }, + [ ] (Size_at_error) { return 0U; }); + } /** * Return kernel CPU ID for given Genode CPU diff --git a/repos/base-sel4/src/core/include/platform.h b/repos/base-sel4/src/core/include/platform.h index 5169925946..6822fe1fca 100644 --- a/repos/base-sel4/src/core/include/platform.h +++ b/repos/base-sel4/src/core/include/platform.h @@ -276,8 +276,14 @@ class Genode::Platform : public Platform_generic * Determine size of a core local mapping required for a * core_rm_session detach(). */ - size_t region_alloc_size_at(void * addr) { - return (_core_mem_alloc.virt_alloc())()->size_at(addr); } + size_t region_alloc_size_at(void * addr) + { + using Size_at_error = Allocator_avl::Size_at_error; + + return (_core_mem_alloc.virt_alloc())()->size_at(addr).convert( + [ ] (size_t s) { return s; }, + [ ] (Size_at_error) { return 0U; }); + } size_t max_caps() const override { diff --git a/repos/base/include/base/allocator_avl.h b/repos/base/include/base/allocator_avl.h index 81e73d430e..a74f1d93ce 100644 --- a/repos/base/include/base/allocator_avl.h +++ b/repos/base/include/base/allocator_avl.h @@ -45,6 +45,15 @@ namespace Genode { class Genode::Allocator_avl_base : public Range_allocator { + public: + + enum class Size_at_error { + UNKNOWN_ADDR, /* no allocation at specified address */ + MISMATCHING_ADDR, /* specified address is not the start of a block */ + }; + + using Size_at_result = Attempt; + private: static bool _sum_in_range(addr_t addr, addr_t offset) { @@ -287,7 +296,7 @@ class Genode::Allocator_avl_base : public Range_allocator /** * Return size of block at specified address */ - size_t size_at(void const *addr) const; + Size_at_result size_at(void const *addr) const; /** * Return the memory overhead per Block diff --git a/repos/base/src/lib/base/allocator_avl.cc b/repos/base/src/lib/base/allocator_avl.cc index 6a51036ebe..76ce55f19a 100644 --- a/repos/base/src/lib/base/allocator_avl.cc +++ b/repos/base/src/lib/base/allocator_avl.cc @@ -394,15 +394,18 @@ void Allocator_avl_base::free(void *addr) } -size_t Allocator_avl_base::size_at(void const *addr) const +Allocator_avl_base::Size_at_result Allocator_avl_base::size_at(void const *addr) const { /* lookup corresponding block */ Block *b = _find_by_address(reinterpret_cast(addr)); if (b && (b->addr() != (addr_t)addr)) - return 0; + return Size_at_error::MISMATCHING_ADDR; - return (b && b->used()) ? b->size() : 0; + if (b && b->used()) + return b->size(); + + return Size_at_error::UNKNOWN_ADDR; } diff --git a/repos/base/src/lib/base/heap.cc b/repos/base/src/lib/base/heap.cc index 7c04c3ae29..8cf9e45ed8 100644 --- a/repos/base/src/lib/base/heap.cc +++ b/repos/base/src/lib/base/heap.cc @@ -255,14 +255,27 @@ void Heap::free(void *addr, size_t) /* serialize access of heap functions */ Mutex::Guard guard(_mutex); - /* try to find the size in our local allocator */ - size_t const size = _alloc->size_at(addr); + using Size_at_error = Allocator_avl::Size_at_error; - if (size != 0) { + Allocator_avl::Size_at_result size_at_result = _alloc->size_at(addr); + if (size_at_result.ok()) { /* forward request to our local allocator */ - _alloc->free(addr, size); - _quota_used -= size; + size_at_result.with_result( + [&] (size_t size) { + /* forward request to our local allocator */ + _alloc->free(addr, size); + _quota_used -= size; + }, + [&] (Size_at_error) { }); + + return; + } + + if (size_at_result == Size_at_error::MISMATCHING_ADDR) { + /* address was found in local allocator but is not a block start address */ + error("heap could not free memory block: given address ", addr, + " is not a block start adress"); return; } @@ -278,7 +291,7 @@ void Heap::free(void *addr, size_t) break; if (!ds) { - warning("heap could not free memory block"); + warning("heap could not free memory block: invalid address"); return; } diff --git a/repos/dde_linux/src/lib/lx_kit/memory.cc b/repos/dde_linux/src/lib/lx_kit/memory.cc index f6118287b4..0136f815c5 100644 --- a/repos/dde_linux/src/lib/lx_kit/memory.cc +++ b/repos/dde_linux/src/lib/lx_kit/memory.cc @@ -173,17 +173,24 @@ bool Lx_kit::Mem_allocator::free(const void * ptr) if (!_mem.valid_addr((addr_t)ptr)) return false; - if (!_mem.size_at(ptr)) - return true; + using Size_at_error = Allocator_avl::Size_at_error; + + _mem.size_at(ptr).with_result( + [&] (size_t) { _mem.free(const_cast(ptr)); }, + [ ] (Size_at_error) { }); - _mem.free(const_cast(ptr)); return true; } Genode::size_t Lx_kit::Mem_allocator::size(const void * ptr) { - return ptr ? _mem.size_at(ptr) : 0; + if (!ptr) return 0; + + using Size_at_error = Allocator_avl::Size_at_error; + + return _mem.size_at(ptr).convert([ ] (size_t s) { return s; }, + [ ] (Size_at_error) { return 0U; }); } diff --git a/repos/libports/src/lib/libc/internal/mem_alloc.h b/repos/libports/src/lib/libc/internal/mem_alloc.h index d88e47951e..bbc8908050 100644 --- a/repos/libports/src/lib/libc/internal/mem_alloc.h +++ b/repos/libports/src/lib/libc/internal/mem_alloc.h @@ -28,9 +28,12 @@ namespace Libc { struct Mem_alloc { + using Size_at_error = Allocator_avl::Size_at_error; + using Size_at_result = Allocator_avl::Size_at_result; + virtual void *alloc(size_t size, size_t align_log2) = 0; virtual void free(void *ptr) = 0; - virtual size_t size_at(void const *ptr) const = 0; + virtual Size_at_result size_at(void const *ptr) const = 0; }; /** @@ -124,7 +127,7 @@ namespace Libc { void *alloc(size_t size, size_t align_log2); void free(void *ptr); - size_t size_at(void const *ptr) const; + Size_at_result size_at(void const *ptr) const; }; } diff --git a/repos/libports/src/lib/libc/libc_mem_alloc.cc b/repos/libports/src/lib/libc/libc_mem_alloc.cc index 23e0d8cbfb..6e2746d70c 100644 --- a/repos/libports/src/lib/libc/libc_mem_alloc.cc +++ b/repos/libports/src/lib/libc/libc_mem_alloc.cc @@ -143,7 +143,7 @@ void Libc::Mem_alloc_impl::free(void *addr) } -size_t Libc::Mem_alloc_impl::size_at(void const *addr) const +Libc::Mem_alloc::Size_at_result Libc::Mem_alloc_impl::size_at(void const *addr) const { /* serialize access of heap functions */ Mutex::Guard guard(_mutex); diff --git a/repos/libports/src/lib/libc/vfs_plugin.cc b/repos/libports/src/lib/libc/vfs_plugin.cc index 939c4cddbc..6dc8204d96 100644 --- a/repos/libports/src/lib/libc/vfs_plugin.cc +++ b/repos/libports/src/lib/libc/vfs_plugin.cc @@ -2429,12 +2429,23 @@ void *Libc::Vfs_plugin::mmap(void *addr_in, ::size_t length, int prot, int flags int Libc::Vfs_plugin::munmap(void *addr, ::size_t) { - if (mem_alloc()->size_at(addr) > 0) { + using Size_at_error = Mem_alloc::Size_at_error; + + Mem_alloc::Size_at_result const size_at_result = mem_alloc()->size_at(addr); + + if (size_at_result.ok()) { /* private mapping */ - mem_alloc()->free(addr); + size_at_result.with_result( + [&] (size_t) { mem_alloc()->free(addr); }, + [&] (Size_at_error) { }); + return 0; } + /* return error if addr is not a block start address */ + if (size_at_result == Size_at_error::MISMATCHING_ADDR) + return Errno(EINVAL); + /* shared mapping */ Vfs::Vfs_handle *reference_handle = nullptr; diff --git a/repos/ports/src/virtualbox5/libc.cc b/repos/ports/src/virtualbox5/libc.cc index e85b1dcc18..577cb015aa 100644 --- a/repos/ports/src/virtualbox5/libc.cc +++ b/repos/ports/src/virtualbox5/libc.cc @@ -109,8 +109,12 @@ extern "C" void *realloc(void *ptr, ::size_t size) unsigned long old_size = size; if (!initial_memory(ptr)) { + using Size_at_error = Libc::Mem_alloc::Size_at_error; + /* determine size of old block content (without header) */ - old_size = memory()->size_at(ptr); + old_size = memory()->size_at(ptr).convert( + [ ] (size_t s) { return s; }, + [ ] (Size_at_error) { return 0U; }); /* do not reallocate if new size is less than the current size */ if (size <= old_size)