ExternalVendorCode/genode
1
0
Fork 0
mirror of https://github.com/genodelabs/genode.git synced 2025-01-18 02:40:08 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

base: Fix UAF in Genode::Pd_session_component::free

Browse Source 
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<Genode::Account<Genode::Ram_quota>>. 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
This commit is contained in:
Piotr Tworek 2020-11-05 14:34:04 +01:00 committed by Christian Helmuth
parent a8d3cd9b15
commit 53a990579b
1 changed files with 3 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
repos/base/src/core/include/pd_session_component.h
View File

@ -49,6 +49,9 @@ class Genode::Pd_session_component : public Session_object<Pd_session>
private:
Constructible<Account<Cap_quota> > _cap_account { };
Constructible<Account<Ram_quota> > _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<Pd_session>
Constructible<Platform_pd> _pd { };
Constructible<Account<Cap_quota> > _cap_account { };
Constructible<Account<Ram_quota> > _ram_account { };
Region_map_component _address_space;
Region_map_component _stack_area;
Region_map_component _linker_area;