From 53a990579b2cc0e9fc40273cc9c8c5a742389931 Mon Sep 17 00:00:00 2001 From: Piotr Tworek Date: Thu, 5 Nov 2020 14:34:04 +0100 Subject: [PATCH] base: Fix UAF in Genode::Pd_session_component::free This was discovered when building the code with clang instead of GCC. In this setup the run/ping on base-hw/arm_v8a/virt_qemu would crash on shutdown due to uncaught Deref_unconstructed_object exception thrown for Genode::Reconstructible>. The specific instance throwing this exception was Pd_session_component::_ram_account. My investigation exposed the following problem: 1. The Pd_session_component has a _sliced_heap member backed by _constrained_ram_alloc which in turn uses Pd_session_component itself as its Ram_allocator. 2. When ~Pd_session_component is called it first destroys _ram_account, followed by _signal_broker. 3. The signal broker holds a reference to Pd_session_component::_sliced_heap as Signal_broker::_md_alloc. 4. The base-hw implementation of ~Signal_broker destroys some contexts and does this by calling Genode::destroy on some slabs using the _md_alloc (ref to Pd_session_component::_sliced_heap). 5. The Genode::Slab calls the Ram_allocator::free which ends up calling Pd_session_component::free. 6. The Pd_session_component::free can among other things call replenish method on Pd_session_component::_ram_account which has already been freed at this point. From my POV calling replenish at this point is basically an undefined behavior. The Genode::Constructible holding the Genode::Account was already detroyed at this point. GCC builds happen to somehow manage to go through the -> operator call without raising any alarms, while clang builds trip on the _check_constructed() call. This fix moves the _ram_account a bit higher in class declaration to ensure its destroyed after _sliced_heap. This seems like the simpliest solution for this problem. Fixes #3941 --- repos/base/src/core/include/pd_session_component.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/repos/base/src/core/include/pd_session_component.h b/repos/base/src/core/include/pd_session_component.h index 14f6230eee..f72ad45a9f 100644 --- a/repos/base/src/core/include/pd_session_component.h +++ b/repos/base/src/core/include/pd_session_component.h @@ -49,6 +49,9 @@ class Genode::Pd_session_component : public Session_object private: + Constructible > _cap_account { }; + Constructible > _ram_account { }; + Rpc_entrypoint &_ep; Constrained_ram_allocator _constrained_md_ram_alloc; Constrained_core_ram _constrained_core_ram_alloc; @@ -61,9 +64,6 @@ class Genode::Pd_session_component : public Session_object Constructible _pd { }; - Constructible > _cap_account { }; - Constructible > _ram_account { }; - Region_map_component _address_space; Region_map_component _stack_area; Region_map_component _linker_area;