Dexuan Cui <decui@xxxxxxxxxxxxx> writes:
>> From: linux-hyperv-owner@xxxxxxxxxxxxxxx
>> <linux-hyperv-owner@xxxxxxxxxxxxxxx> On Behalf Of Dexuan Cui
>> Sent: Monday, June 15, 2020 10:42 AM
>> > >
>> > > Hi hch,
>> > > The patch is merged into the mainine recently, but unluckily we noticed
>> > > a warning with CONFIG_DEBUG_WX=y
>> > >
>> > > Should we revert this patch, or figure out a way to ask the DEBUG_WX
>> > > code to ignore this page?
>> >
>> > Are you sure it is hv_hypercall_pg?
>> Yes, 100% sure. I printed the value of hv_hypercall_pg and and it matched the
>> address in the warning line " x86/mm: Found insecure W+X mapping at
>> address".
>
> I did this experiment:
> 1. export vmalloc_exec and ptdump_walk_pgd_level_checkwx.
> 2. write a test module that calls them.
> 3. It turns out that every call of vmalloc_exec() triggers such a warning.
>
> vmalloc_exec() uses PAGE_KERNEL_EXEC, which is defined as
> (__PP|__RW| 0|___A| 0|___D| 0|___G)
>
> It looks the logic in note_page() is: for_each_RW_page, if the NX bit is unset,
> then report the page as an insecure W+X mapping. IMO this explains the
> warning?
Yea, bummer.
it seems we need something like PAGE_KERNEL_READONLY_EXEC but we don't
seem to have one on x86. Hypercall page is special in a way that the
guest doesn't need to write there at all. vmalloc_exec() seems to have
only one other user on x86: module_alloc() and it has other needs. On
ARM, alloc_insn_page() does the following:
arch/arm64/kernel/probes/kprobes.c: page = vmalloc_exec(PAGE_SIZE);
arch/arm64/kernel/probes/kprobes.c- if (page) {
arch/arm64/kernel/probes/kprobes.c- set_memory_ro((unsigned long)page, 1);
arch/arm64/kernel/probes/kprobes.c- set_vm_flush_reset_perms(page);
arch/arm64/kernel/probes/kprobes.c- }
What if we do the same? (almost untested):
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index e2137070386a..31aadfea589b 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -23,6 +23,7 @@
#include <linux/kernel.h>
#include <linux/cpuhotplug.h>
#include <linux/syscore_ops.h>
+#include <linux/set_memory.h>
#include <clocksource/hyperv_timer.h>
void *hv_hypercall_pg;
@@ -383,6 +384,8 @@ void __init hyperv_init(void)
wrmsrl(HV_X64_MSR_GUEST_OS_ID, 0);
goto remove_cpuhp_state;
}
+ set_memory_ro((unsigned long)hv_hypercall_pg, 1);
+ set_vm_flush_reset_perms(hv_hypercall_pg);
rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
hypercall_msr.enable = 1;
--
Vitaly
