hw: restrict page-table lookup to rw-pages

The `lookup_translation` function got introduced and is used only in the
context to proof whether a cache maintainance function can be executed
safely by the kernel. Unfortunately, it did not checked write permissions,
which can lead to permission faults. This commit restricts the lookup
function to only succeed when the target page is writeable. Consequently,
the lookup function gets renamed to `lookup_rw_translation`.

Fix genodelabs/genode#4348
This commit is contained in:
Stefan Kalkowski 2022-02-17 13:50:05 +01:00 committed by Norman Feske
parent e1a2b5c8d4
commit 0c67d0838a
7 changed files with 36 additions and 25 deletions

View File

@ -70,14 +70,14 @@ bool Hw::Address_space::insert_translation(addr_t virt, addr_t phys,
}
bool Hw::Address_space::lookup_translation(addr_t const virt, addr_t & phys)
bool Hw::Address_space::lookup_rw_translation(addr_t const virt, addr_t & phys)
{
/** FIXME: for the time-being we use it without lock,
* because it is used directly by the kernel when cache_coherent_region
* gets called. In future it would be better that core provides an API
* for it, and does the lookup with the hold lock
*/
return _tt.lookup_translation(virt, phys, _tt_alloc);
return _tt.lookup_rw_translation(virt, phys, _tt_alloc);
}

View File

@ -122,8 +122,8 @@ class Hw::Address_space : public Genode::Address_space
bool insert_translation(Genode::addr_t virt, Genode::addr_t phys,
Genode::size_t size, Genode::Page_flags flags);
bool lookup_translation(Genode::addr_t const virt,
Genode::addr_t & phys);
bool lookup_rw_translation(Genode::addr_t const virt,
Genode::addr_t & phys);
/*****************************
** Address-space interface **

View File

@ -37,15 +37,15 @@ static void for_cachelines(addr_t base,
}
/**
* Lookup whether the page is backed, and if so make the memory coherent
* in between I-, and D-cache
* Lookup whether the page is backed and writeable,
* and if so make the memory coherent in between I-, and D-cache
*/
addr_t phys = 0;
if (thread.pd().platform_pd().lookup_translation(base, phys)) {
if (thread.pd().platform_pd().lookup_rw_translation(base, phys)) {
fn(base, size);
} else {
Genode::raw(thread, " tried to make invalid address ",
(void*)base, " cache coherent");
Genode::raw(thread, " tried to do cache maintainance at ",
"unallowed address ", (void*)base);
}
}

View File

@ -388,7 +388,10 @@ class Hw::Level_3_translation_table :
if (!Descriptor::valid(desc))
return;
phys = Block_descriptor::Output_address::masked(desc);
found = true;
typename Block_descriptor::access_t ap =
Block_descriptor::Access_permission::get(desc);
found = ap == Block_descriptor::Access_permission::PRIVILEGED_RW ||
ap == Block_descriptor::Access_permission::USER_RW;
}
};
@ -409,7 +412,7 @@ class Hw::Level_3_translation_table :
_range_op(vo, pa, size, Remove_func());
}
bool lookup_translation(addr_t vo, addr_t & pa, Allocator&)
bool lookup_rw_translation(addr_t vo, addr_t & pa, Allocator&)
{
size_t page_size = 1 << SIZE_LOG2_4KB;
Lookup_func functor {};
@ -542,15 +545,18 @@ class Hw::Level_x_translation_table :
case Descriptor::BLOCK:
{
phys = Block_descriptor::Output_address::masked(desc);
found = true;
typename Block_descriptor::access_t ap =
Block_descriptor::Access_permission::get(desc);
found = ap == Block_descriptor::Access_permission::PRIVILEGED_RW ||
ap == Block_descriptor::Access_permission::USER_RW;
return;
};
case Descriptor::TABLE:
{
/* use allocator to retrieve virt address of table */
E & table = alloc.virt_addr<E>(Nt::masked(desc));
found = table.lookup_translation(vo - (vo & Base::BLOCK_MASK),
phys, alloc);
found = table.lookup_rw_translation(vo - (vo & Base::BLOCK_MASK),
phys, alloc);
return;
};
case Descriptor::INVALID: return;
@ -596,13 +602,13 @@ class Hw::Level_x_translation_table :
}
/**
* Lookup translation
* Lookup writeable translation
*
* \param virt region offset within the tables virtual region
* \param phys region size
* \param alloc second level translation table allocator
*/
bool lookup_translation(addr_t const virt, addr_t & phys,
bool lookup_rw_translation(addr_t const virt, addr_t & phys,
Allocator & alloc)
{
size_t page_size = 1 << SIZE_LOG2_4KB;

View File

@ -261,13 +261,13 @@ class Hw::Page_table
}
/**
* Lookup translation
* Lookup writeable translation
*
* \param virt virtual address offset to look for
* \param phys physical address to return
* \returns true if lookup was successful, otherwise false
*/
bool lookup_translation(addr_t const virt, addr_t & phys)
bool lookup_rw_translation(addr_t const virt, addr_t & phys)
{
unsigned idx = 0;
if (!_index_by_vo(idx, virt)) return false;
@ -275,8 +275,10 @@ class Hw::Page_table
switch (Descriptor::type(_entries[idx])) {
case Descriptor::SMALL_PAGE:
{
Small_page::access_t ap =
Small_page::Ap::get(_entries[idx]);
phys = Small_page::Pa::masked(_entries[idx]);
return true;
return ap == 1 || ap == 3;
}
default: return false;
}
@ -607,14 +609,15 @@ class Hw::Page_table
}
/**
* Lookup translation
* Lookup writeable translation
*
* \param virt virtual address to look at
* \param phys physical address to return
* \param alloc second level translation table allocator
* \returns true if a translation was found, otherwise false
*/
bool lookup_translation(addr_t const virt, addr_t & phys, Allocator & alloc)
bool lookup_rw_translation(addr_t const virt, addr_t & phys,
Allocator & alloc)
{
unsigned idx = 0;
if (!_index_by_vo(idx, virt)) return false;
@ -623,8 +626,10 @@ class Hw::Page_table
case Descriptor::SECTION:
{
Section::access_t ap =
Section::Ap::get(_entries[idx]);
phys = Section::Pa::masked(_entries[idx]);
return true;
return ap == 1 || ap == 3;
}
case Descriptor::PAGE_TABLE:
@ -636,7 +641,7 @@ class Hw::Page_table
alloc.virt_addr<Pt>(Ptd::Pa::masked(_entries[idx]));
addr_t const offset = virt - Section::Pa::masked(virt);
return pt.lookup_translation(offset, phys);
return pt.lookup_rw_translation(offset, phys);
}
default: return false;
};

View File

@ -351,7 +351,7 @@ class Sv39::Level_x_translation_table
_range_op(vo, 0, size, Remove_func<ENTRY>(alloc));
}
bool lookup_translation(addr_t, addr_t &, Allocator &)
bool lookup_rw_translation(addr_t, addr_t &, Allocator &)
{
Genode::raw(__func__, " not implemented yet");
return false;

View File

@ -680,7 +680,7 @@ class Hw::Pml4_table
_range_op(vo, 0, size, Remove_func(alloc));
}
bool lookup_translation(addr_t const, addr_t &, Allocator &)
bool lookup_rw_translation(addr_t const, addr_t &, Allocator &)
{
Genode::raw(__func__, " not implemented yet");
return false;