While we've been diligent to ensure that the main text/data/rodata mappings have suitable restrictions, their aliases via the directmap were left fully RW. Worse, we even had pieces of code making use of this as a feature.
Restrict the permissions, as we have no legitimate need for writeability of these areas via the directmap alias. Note that the pagetables and cpu0_stack do get written through their directmap alias, so we can't just read-only the whole image. Signed-off-by: Andrew Cooper <[email protected]> --- CC: Jan Beulich <[email protected]> CC: Roger Pau Monné <[email protected]> CC: Wei Liu <[email protected]> Ever so slightly RFC, as it has only had light testing. Notes: * The stubs are still have RX via one alias, RW via another, and these need to stay. Hardening options include splitting the stubs so the SYSCALL ones can be read-only after setup, and/or expanding the stub size to 4k per CPU so we really can keep the writeable alias as not present when the stub isn't in active use. * Future CPUs with Protection Key Supervisor (Sapphire Rapids and later) would be able to inhibit writeability outside of a permitted region, and because the protection key is per logical thread, we woulnd't need to expand the stubs to 4k per CPU. * At the time of writing, PV Shim still makes use of .rodata's read/write alias in the directmap to patch the hypercall table, but that runs earlier on boot. Also, there are patches out to address this. * For backporting, this patch depends on c/s e7f147bf4ac7 ("x86/crash: Drop manual hooking of exception_table[]"), and nothing would break at compile time if the dependency was missing. --- xen/arch/x86/setup.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c index f40a9fe5d351..c8641c227d9a 100644 --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -1566,6 +1566,14 @@ void __init noreturn __start_xen(unsigned long mbi_p) destroy_xen_mappings((unsigned long)&__2M_rwdata_end, ROUNDUP((unsigned long)&__2M_rwdata_end, MB(2))); + /* + * Mark all of .text and .rodata as RO in the directmap - we don't want + * these sections writeable via any alias. + */ + modify_xen_mappings((unsigned long)__va(__pa(_start)), + (unsigned long)__va(__pa(__2M_rodata_end)), + PAGE_HYPERVISOR_RO); + nr_pages = 0; for ( i = 0; i < e820.nr_map; i++ ) if ( e820.map[i].type == E820_RAM ) -- 2.11.0
