* [PATCH] xen/x86: Remap text/data/bss with appropriate permissions @ 2016-03-17 12:43 Andrew Cooper 2016-03-17 12:43 ` [PATCH] DO NOT APPLY - debug keys for inspecting Xen mappings Andrew Cooper 2016-03-17 14:31 ` [PATCH] xen/x86: Remap text/data/bss with appropriate permissions Jan Beulich 0 siblings, 2 replies; 8+ messages in thread From: Andrew Cooper @ 2016-03-17 12:43 UTC (permalink / raw) To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich c/s cf39362 "x86: use 2M superpages for text/data/bss mappings" served two purposes; to map the primary code and data with appropriate pagetable permissions (rather than unilaterally RWX), and to reduce the TLB pressure. The extra alignment exposed a SYSLinux issue, and was partly reverted by c/s 0b8a172 "x86: partially revert use of 2M mappings for hypervisor image". This change reinstates the pagetable permission improvements while avoiding the 2M alignment issue. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> --- xen/arch/x86/setup.c | 24 ++++++++++++++++++++++++ xen/arch/x86/xen.lds.S | 16 ++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c index c5c332d..e278a7b 100644 --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -529,9 +529,33 @@ static void noinline init_done(void) } else { + /* Mark .text as RX (avoiding the first 2M superpage). */ + map_pages_to_xen(XEN_VIRT_START + MB(2), + PFN_DOWN(__pa(XEN_VIRT_START + MB(2))), + PFN_DOWN(__2M_text_end - + (const char *)(XEN_VIRT_START + MB(2))), + PAGE_HYPERVISOR_RX); + + /* Mark .rodata as RO. */ + map_pages_to_xen((unsigned long)&__2M_rodata_start, + PFN_DOWN(__pa(__2M_rodata_start)), + PFN_DOWN(__2M_rodata_end - __2M_rodata_start), + PAGE_HYPERVISOR_RO); + + /* Free and reuse .init. */ destroy_xen_mappings((unsigned long)&__init_begin, (unsigned long)&__init_end); init_xenheap_pages(__pa(__init_begin), __pa(__init_end)); + + /* Mark .data and .bss as RW. */ + map_pages_to_xen((unsigned long)&__2M_rwdata_start, + PFN_DOWN(__pa(__2M_rwdata_start)), + PFN_DOWN(__2M_rwdata_end - __2M_rwdata_start), + PAGE_HYPERVISOR_RW); + + /* Drop the remaining mappings in the shattered superpage. */ + destroy_xen_mappings((unsigned long)&__2M_rwdata_end, + ROUNDUP((unsigned long)&__2M_rwdata_end, MB(2))); } printk("Freed %ldkB init memory.\n", (long)(__init_end-__init_begin)>>10); diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S index 961f48f..ab8ba74 100644 --- a/xen/arch/x86/xen.lds.S +++ b/xen/arch/x86/xen.lds.S @@ -55,6 +55,8 @@ SECTIONS #ifdef EFI . = ALIGN(MB(2)); +#else + . = ALIGN(PAGE_SIZE); #endif __2M_text_end = .; @@ -98,6 +100,8 @@ SECTIONS #ifdef EFI . = ALIGN(MB(2)); +#else + . = ALIGN(PAGE_SIZE); #endif __2M_rodata_end = .; @@ -167,6 +171,8 @@ SECTIONS #ifdef EFI . = ALIGN(MB(2)); +#else + . = ALIGN(PAGE_SIZE); #endif __2M_init_end = .; @@ -211,6 +217,8 @@ SECTIONS #ifdef EFI . = ALIGN(MB(2)); +#else + . = ALIGN(PAGE_SIZE); #endif __2M_rwdata_end = .; @@ -269,6 +277,14 @@ ASSERT(IS_ALIGNED(__2M_init_start, MB(2)), "__2M_init_start misaligned") ASSERT(IS_ALIGNED(__2M_init_end, MB(2)), "__2M_init_end misaligned") ASSERT(IS_ALIGNED(__2M_rwdata_start, MB(2)), "__2M_rwdata_start misaligned") ASSERT(IS_ALIGNED(__2M_rwdata_end, MB(2)), "__2M_rwdata_end misaligned") +#else +ASSERT(IS_ALIGNED(__2M_text_end, PAGE_SIZE), "__2M_text_end misaligned") +ASSERT(IS_ALIGNED(__2M_rodata_start, PAGE_SIZE), "__2M_rodata_start misaligned") +ASSERT(IS_ALIGNED(__2M_rodata_end, PAGE_SIZE), "__2M_rodata_end misaligned") +ASSERT(IS_ALIGNED(__2M_init_start, PAGE_SIZE), "__2M_init_start misaligned") +ASSERT(IS_ALIGNED(__2M_init_end, PAGE_SIZE), "__2M_init_end misaligned") +ASSERT(IS_ALIGNED(__2M_rwdata_start, PAGE_SIZE), "__2M_rwdata_start misaligned") +ASSERT(IS_ALIGNED(__2M_rwdata_end, PAGE_SIZE), "__2M_rwdata_end misaligned") #endif ASSERT(IS_ALIGNED(cpu0_stack, STACK_SIZE), "cpu0_stack misaligned") -- 2.1.4 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH] DO NOT APPLY - debug keys for inspecting Xen mappings 2016-03-17 12:43 [PATCH] xen/x86: Remap text/data/bss with appropriate permissions Andrew Cooper @ 2016-03-17 12:43 ` Andrew Cooper 2016-03-17 14:31 ` [PATCH] xen/x86: Remap text/data/bss with appropriate permissions Jan Beulich 1 sibling, 0 replies; 8+ messages in thread From: Andrew Cooper @ 2016-03-17 12:43 UTC (permalink / raw) To: Xen-devel; +Cc: Andrew Cooper --- xen/arch/x86/mm.c | 15 ++++++ xen/arch/x86/traps.c | 148 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 163 insertions(+) diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index c997b53..77752fe 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -123,6 +123,7 @@ #include <asm/fixmap.h> #include <asm/io_apic.h> #include <asm/pci.h> +#include <xen/console.h> /* Mapping of the fixmap space needed early. */ l1_pgentry_t __section(".bss.page_aligned") l1_fixmap[L1_PAGETABLE_ENTRIES]; @@ -5635,6 +5636,20 @@ int map_pages_to_xen( l1_pgentry_t *pl1e, ol1e; unsigned int i; + if ( XEN_VIRT_START >= virt && + XEN_VIRT_START <= (virt + (nr_mfns * PAGE_SIZE)) ) + { + console_start_sync(); + + printk("*** %s(%p, %p, %p, %#x)\n", + __func__, _p(virt), _p(mfn), _p(nr_mfns), flags); + printk("*** Called from %p %pS\n", + __builtin_return_address(0), + __builtin_return_address(0)); + + console_end_sync(); + } + #define flush_flags(oldf) do { \ unsigned int o_ = (oldf); \ if ( (o_) & _PAGE_GLOBAL ) \ diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c index 564a107..8381138 100644 --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -4124,6 +4124,154 @@ void asm_domain_crash_synchronous(unsigned long addr) __domain_crash_synchronous(); } +static const char *pte_flags_decode(unsigned int flags) +{ + static char buf[24]; + + snprintf(buf, sizeof buf, "%s%s%s%s%s%s", + flags & _PAGE_NX_BIT ? "NX" : "X", + flags & _PAGE_GLOBAL ? " Gl" : "", + flags & _PAGE_PSE ? " +" : "", + flags & _PAGE_USER ? " U" : " S", + flags & _PAGE_RW ? " RW" : " RO", + flags & _PAGE_PRESENT ? " P" : "" + ); + return buf; +} + +void dump_xen_mappings(bool_t dump_l1) +{ + unsigned long cr3 = read_cr3(); + + unsigned int i4 = l4_table_offset(XEN_VIRT_START); + l4_pgentry_t *l4 = &idle_pg_table[i4]; + unsigned int l4ef = l4e_get_flags(*l4); + + unsigned int i3 = l3_table_offset(XEN_VIRT_START); + l3_pgentry_t *l3 = l4e_to_l3e(*l4); + unsigned int l3ef = l3e_get_flags(l3[i3]); + + unsigned int i2; + l2_pgentry_t *l2 = l3e_to_l2e(l3[i3]); + + printk("*** Dumping Xen text/data/bss mappings from %p\n", + _p(XEN_VIRT_START)); + + printk("cr3 %p, idle_pg_table %p, pa %p\n", + _p(cr3), idle_pg_table, _p(__pa(idle_pg_table))); + + printk("l2_xenmap: %p, pa %p\n", l2_xenmap, _p(__pa(l2_xenmap))); + + printk(" L4[%03u] = %"PRIpte" %s\n", + i4, l4e_get_intpte(*l4), pte_flags_decode(l4ef)); + + printk(" L3[%03u] = %"PRIpte" %s\n", + i3, l3e_get_intpte(l3[i3]), pte_flags_decode(l3ef)); + + if ( l3e_get_paddr(l3[i3]) != __pa(l2_xenmap) ) + printk("** Unexpected - l3e not pointing at l2_xenmap\n"); + + for ( i2 = 0; i2 < L2_PAGETABLE_ENTRIES; ++i2 ) + { + unsigned int l2ef = l2e_get_flags(l2[i2]), i1; + l1_pgentry_t *l1; + + if ( !(l2ef & _PAGE_PRESENT) ) + continue; + + printk(" L2[%03u] = %"PRIpte" %s\n", + i2, l2e_get_intpte(l2[i2]), pte_flags_decode(l2ef)); + + if ( l2ef & _PAGE_PSE ) + continue; + + if ( !dump_l1 ) + { + printk(" L1 abbr\n"); + continue; + } + + l1 = l2e_to_l1e(l2[i2]); + + for ( i1 = 0; i1 < L1_PAGETABLE_ENTRIES; ++i1 ) + { + unsigned int l1ef = l1e_get_flags(l1[i1]); + + if ( !(l1ef & _PAGE_PRESENT) ) + continue; + + printk(" L1[%03u] = %"PRIpte" %s\n", + i1, l1e_get_intpte(l1[i1]), pte_flags_decode(l1ef)); + } + } +} + +static void read_idle(void) +{ + l4_pgentry_t *l4 = idle_pg_table; + unsigned int i4; + + for ( i4 = 0; i4 < L4_PAGETABLE_ENTRIES; ++i4 ) + { + unsigned int l4ef = l4e_get_flags(l4[i4]); + + if ( !(l4ef & _PAGE_PRESENT) ) + continue; + + printk(" L4[%03u] = %"PRIpte" %s\n", + i4, l4e_get_intpte(l4[i4]), pte_flags_decode(l4ef)); + } +} + +#include <xen/keyhandler.h> +#include <asm/setup.h> + +static void dump_offsets(const char *name, unsigned long addr) +{ + printk(" %-15s: %lu %lu %lu %lu\n", name, l4_table_offset(addr), + l3_table_offset(addr), l2_table_offset(addr), l1_table_offset(addr)); +} +#define DUMP(x) dump_offsets(#x, (unsigned long)&(x)) + +static void do_extreme_debug(unsigned char key, struct cpu_user_regs *regs) +{ + printk("'%c' pressed -> Extreme debugging in progress...\n", key); + + switch ( key ) + { + case '1': + dump_xen_mappings(0); + break; + + case '2': + dump_xen_mappings(1); + break; + + case '3': + read_idle(); + break; + + case '4': + DUMP(_stext); + DUMP(_etext); + DUMP(__2M_rodata_start); + DUMP(__2M_rodata_end); + DUMP(__2M_rwdata_start); + DUMP(__2M_rwdata_end); + break; + } +} + +static int __init extreme_debug_keyhandler_init(void) +{ + register_irq_keyhandler('1', &do_extreme_debug, "Extreme debugging 1", 0); + register_irq_keyhandler('2', &do_extreme_debug, "Extreme debugging 2", 0); + register_irq_keyhandler('3', &do_extreme_debug, "Extreme debugging 3", 0); + register_irq_keyhandler('4', &do_extreme_debug, "Extreme debugging 3", 0); + return 0; +} +__initcall(extreme_debug_keyhandler_init); + /* * Local variables: * mode: C -- 2.1.4 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] xen/x86: Remap text/data/bss with appropriate permissions 2016-03-17 12:43 [PATCH] xen/x86: Remap text/data/bss with appropriate permissions Andrew Cooper 2016-03-17 12:43 ` [PATCH] DO NOT APPLY - debug keys for inspecting Xen mappings Andrew Cooper @ 2016-03-17 14:31 ` Jan Beulich 2016-03-17 14:44 ` Andrew Cooper 2016-03-18 19:49 ` [PATCH v2] " Andrew Cooper 1 sibling, 2 replies; 8+ messages in thread From: Jan Beulich @ 2016-03-17 14:31 UTC (permalink / raw) To: Andrew Cooper; +Cc: Xen-devel >>> On 17.03.16 at 13:43, <andrew.cooper3@citrix.com> wrote: > --- a/xen/arch/x86/setup.c > +++ b/xen/arch/x86/setup.c > @@ -529,9 +529,33 @@ static void noinline init_done(void) > } > else > { > + /* Mark .text as RX (avoiding the first 2M superpage). */ > + map_pages_to_xen(XEN_VIRT_START + MB(2), > + PFN_DOWN(__pa(XEN_VIRT_START + MB(2))), > + PFN_DOWN(__2M_text_end - > + (const char *)(XEN_VIRT_START + MB(2))), > + PAGE_HYPERVISOR_RX); > + > + /* Mark .rodata as RO. */ > + map_pages_to_xen((unsigned long)&__2M_rodata_start, > + PFN_DOWN(__pa(__2M_rodata_start)), > + PFN_DOWN(__2M_rodata_end - __2M_rodata_start), > + PAGE_HYPERVISOR_RO); > + > + /* Free and reuse .init. */ > destroy_xen_mappings((unsigned long)&__init_begin, > (unsigned long)&__init_end); > init_xenheap_pages(__pa(__init_begin), __pa(__init_end)); > + > + /* Mark .data and .bss as RW. */ > + map_pages_to_xen((unsigned long)&__2M_rwdata_start, > + PFN_DOWN(__pa(__2M_rwdata_start)), > + PFN_DOWN(__2M_rwdata_end - __2M_rwdata_start), > + PAGE_HYPERVISOR_RW); > + > + /* Drop the remaining mappings in the shattered superpage. */ > + destroy_xen_mappings((unsigned long)&__2M_rwdata_end, > + ROUNDUP((unsigned long)&__2M_rwdata_end, MB(2))); > } I think this would be more appropriate to add to some __init function (which the discarding of .init.* naturally can't live in). Also - do we really want to make this code dependent on map_pages_to_xen() not intermediately zapping the mappings being changed? Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xen/x86: Remap text/data/bss with appropriate permissions 2016-03-17 14:31 ` [PATCH] xen/x86: Remap text/data/bss with appropriate permissions Jan Beulich @ 2016-03-17 14:44 ` Andrew Cooper 2016-03-17 15:32 ` Jan Beulich 2016-03-18 19:49 ` [PATCH v2] " Andrew Cooper 1 sibling, 1 reply; 8+ messages in thread From: Andrew Cooper @ 2016-03-17 14:44 UTC (permalink / raw) To: Jan Beulich; +Cc: Xen-devel On 17/03/16 14:31, Jan Beulich wrote: >>>> On 17.03.16 at 13:43, <andrew.cooper3@citrix.com> wrote: >> --- a/xen/arch/x86/setup.c >> +++ b/xen/arch/x86/setup.c >> @@ -529,9 +529,33 @@ static void noinline init_done(void) >> } >> else >> { >> + /* Mark .text as RX (avoiding the first 2M superpage). */ >> + map_pages_to_xen(XEN_VIRT_START + MB(2), >> + PFN_DOWN(__pa(XEN_VIRT_START + MB(2))), >> + PFN_DOWN(__2M_text_end - >> + (const char *)(XEN_VIRT_START + MB(2))), >> + PAGE_HYPERVISOR_RX); >> + >> + /* Mark .rodata as RO. */ >> + map_pages_to_xen((unsigned long)&__2M_rodata_start, >> + PFN_DOWN(__pa(__2M_rodata_start)), >> + PFN_DOWN(__2M_rodata_end - __2M_rodata_start), >> + PAGE_HYPERVISOR_RO); >> + >> + /* Free and reuse .init. */ >> destroy_xen_mappings((unsigned long)&__init_begin, >> (unsigned long)&__init_end); >> init_xenheap_pages(__pa(__init_begin), __pa(__init_end)); >> + >> + /* Mark .data and .bss as RW. */ >> + map_pages_to_xen((unsigned long)&__2M_rwdata_start, >> + PFN_DOWN(__pa(__2M_rwdata_start)), >> + PFN_DOWN(__2M_rwdata_end - __2M_rwdata_start), >> + PAGE_HYPERVISOR_RW); >> + >> + /* Drop the remaining mappings in the shattered superpage. */ >> + destroy_xen_mappings((unsigned long)&__2M_rwdata_end, >> + ROUNDUP((unsigned long)&__2M_rwdata_end, MB(2))); >> } > I think this would be more appropriate to add to some __init > function (which the discarding of .init.* naturally can't live in). I will see if I can pull it forwards to just after the relocation, to be as close to the permissions change in the 2M case as possible. > > Also - do we really want to make this code dependent on > map_pages_to_xen() not intermediately zapping the mappings > being changed? Do you mean "immediately"? As far as I can tell, it is guaranteed to be safe, even when remapping the code section. Updates to the live pagetables are using atomic writes, and I didn't spot a point which would end up with a transient non-present mapping. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xen/x86: Remap text/data/bss with appropriate permissions 2016-03-17 14:44 ` Andrew Cooper @ 2016-03-17 15:32 ` Jan Beulich 2016-03-17 16:15 ` Andrew Cooper 0 siblings, 1 reply; 8+ messages in thread From: Jan Beulich @ 2016-03-17 15:32 UTC (permalink / raw) To: Andrew Cooper; +Cc: Xen-devel >>> On 17.03.16 at 15:44, <andrew.cooper3@citrix.com> wrote: > On 17/03/16 14:31, Jan Beulich wrote: >> Also - do we really want to make this code dependent on >> map_pages_to_xen() not intermediately zapping the mappings >> being changed? > > Do you mean "immediately"? No. > As far as I can tell, it is guaranteed to be safe, even when remapping > the code section. Updates to the live pagetables are using atomic > writes, and I didn't spot a point which would end up with a transient > non-present mapping. But we may, at some point and for whatever reason, come to make the function zap the mapping (i.e. clear the present bit), flush, and only the re-establish the new mapping. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xen/x86: Remap text/data/bss with appropriate permissions 2016-03-17 15:32 ` Jan Beulich @ 2016-03-17 16:15 ` Andrew Cooper 2016-03-18 7:25 ` Jan Beulich 0 siblings, 1 reply; 8+ messages in thread From: Andrew Cooper @ 2016-03-17 16:15 UTC (permalink / raw) To: Jan Beulich; +Cc: Xen-devel On 17/03/16 15:32, Jan Beulich wrote: >>>> On 17.03.16 at 15:44, <andrew.cooper3@citrix.com> wrote: >> On 17/03/16 14:31, Jan Beulich wrote: >>> Also - do we really want to make this code dependent on >>> map_pages_to_xen() not intermediately zapping the mappings >>> being changed? >> Do you mean "immediately"? > No. > >> As far as I can tell, it is guaranteed to be safe, even when remapping >> the code section. Updates to the live pagetables are using atomic >> writes, and I didn't spot a point which would end up with a transient >> non-present mapping. > But we may, at some point and for whatever reason, come to make > the function zap the mapping (i.e. clear the present bit), flush, and > only the re-establish the new mapping. This change is temporary until I can fix the legacy boot issue and reintroduce the proper 2M functionality. If someone in the future wants to change the behaviour of map_pages_to_xen() then we can reconsider. However, I think it is unlikely that this will actually happen at all, and if it ever does, I hope to have already fixed the 2M alignment and deleted this change. This change is a big security improvement, and absolutely should be taken, especially as the current implementation of map_pages_to_xen() is safe. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xen/x86: Remap text/data/bss with appropriate permissions 2016-03-17 16:15 ` Andrew Cooper @ 2016-03-18 7:25 ` Jan Beulich 0 siblings, 0 replies; 8+ messages in thread From: Jan Beulich @ 2016-03-18 7:25 UTC (permalink / raw) To: Andrew Cooper; +Cc: Xen-devel >>> On 17.03.16 at 17:15, <andrew.cooper3@citrix.com> wrote: > On 17/03/16 15:32, Jan Beulich wrote: >>>>> On 17.03.16 at 15:44, <andrew.cooper3@citrix.com> wrote: >>> On 17/03/16 14:31, Jan Beulich wrote: >>>> Also - do we really want to make this code dependent on >>>> map_pages_to_xen() not intermediately zapping the mappings >>>> being changed? >>> Do you mean "immediately"? >> No. >> >>> As far as I can tell, it is guaranteed to be safe, even when remapping >>> the code section. Updates to the live pagetables are using atomic >>> writes, and I didn't spot a point which would end up with a transient >>> non-present mapping. >> But we may, at some point and for whatever reason, come to make >> the function zap the mapping (i.e. clear the present bit), flush, and >> only the re-establish the new mapping. > > This change is temporary until I can fix the legacy boot issue and > reintroduce the proper 2M functionality. > > If someone in the future wants to change the behaviour of > map_pages_to_xen() then we can reconsider. However, I think it is > unlikely that this will actually happen at all, and if it ever does, I > hope to have already fixed the 2M alignment and deleted this change. > > This change is a big security improvement, and absolutely should be > taken, especially as the current implementation of map_pages_to_xen() is > safe. I by no means intend to reject this change just because of this aspect - I merely wanted to make the slight concern explicit. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] xen/x86: Remap text/data/bss with appropriate permissions 2016-03-17 14:31 ` [PATCH] xen/x86: Remap text/data/bss with appropriate permissions Jan Beulich 2016-03-17 14:44 ` Andrew Cooper @ 2016-03-18 19:49 ` Andrew Cooper 1 sibling, 0 replies; 8+ messages in thread From: Andrew Cooper @ 2016-03-18 19:49 UTC (permalink / raw) To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich c/s cf39362 "x86: use 2M superpages for text/data/bss mappings" served two purposes; to map the primary code and data with appropriate pagetable permissions (rather than unilaterally RWX), and to reduce the TLB pressure. The extra alignment exposed a SYSLinux issue, and was partly reverted by c/s 0b8a172 "x86: partially revert use of 2M mappings for hypervisor image". This change reinstates the pagetable permission improvements while avoiding the 2M alignment issue. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> v2: Move the remapping earlier in the boot sequence. This is where memguart_init() used to reside, which itself used map_pages_to_xen() on the full virtual region. --- xen/arch/x86/setup.c | 31 +++++++++++++++++++++++++++++++ xen/arch/x86/xen.lds.S | 16 ++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c index 1876a28..ee65f55 100644 --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -1205,6 +1205,37 @@ void __init noreturn __start_xen(unsigned long mbi_p) ~((1UL << L2_PAGETABLE_SHIFT) - 1); destroy_xen_mappings(xen_virt_end, XEN_VIRT_START + BOOTSTRAP_MAP_BASE); + /* + * If not using 2M mappings to gain suitable pagetable permissions + * directly from the relocation above, remap the code/data + * sections with decreased permissions. + */ + if ( !using_2M_mapping() ) + { + /* Mark .text as RX (avoiding the first 2M superpage). */ + map_pages_to_xen(XEN_VIRT_START + MB(2), + PFN_DOWN(__pa(XEN_VIRT_START + MB(2))), + PFN_DOWN(__2M_text_end - + (const char *)(XEN_VIRT_START + MB(2))), + PAGE_HYPERVISOR_RX); + + /* Mark .rodata as RO. */ + map_pages_to_xen((unsigned long)&__2M_rodata_start, + PFN_DOWN(__pa(__2M_rodata_start)), + PFN_DOWN(__2M_rodata_end - __2M_rodata_start), + PAGE_HYPERVISOR_RO); + + /* Mark .data and .bss as RW. */ + map_pages_to_xen((unsigned long)&__2M_rwdata_start, + PFN_DOWN(__pa(__2M_rwdata_start)), + PFN_DOWN(__2M_rwdata_end - __2M_rwdata_start), + PAGE_HYPERVISOR_RW); + + /* Drop the remaining mappings in the shattered superpage. */ + destroy_xen_mappings((unsigned long)&__2M_rwdata_end, + ROUNDUP((unsigned long)&__2M_rwdata_end, MB(2))); + } + nr_pages = 0; for ( i = 0; i < e820.nr_map; i++ ) if ( e820.map[i].type == E820_RAM ) diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S index e47771c..5eb825e 100644 --- a/xen/arch/x86/xen.lds.S +++ b/xen/arch/x86/xen.lds.S @@ -56,6 +56,8 @@ SECTIONS #ifdef EFI . = ALIGN(MB(2)); +#else + . = ALIGN(PAGE_SIZE); #endif __2M_text_end = .; @@ -99,6 +101,8 @@ SECTIONS #ifdef EFI . = ALIGN(MB(2)); +#else + . = ALIGN(PAGE_SIZE); #endif __2M_rodata_end = .; @@ -168,6 +172,8 @@ SECTIONS #ifdef EFI . = ALIGN(MB(2)); +#else + . = ALIGN(PAGE_SIZE); #endif __2M_init_end = .; @@ -212,6 +218,8 @@ SECTIONS #ifdef EFI . = ALIGN(MB(2)); +#else + . = ALIGN(PAGE_SIZE); #endif __2M_rwdata_end = .; @@ -270,6 +278,14 @@ ASSERT(IS_ALIGNED(__2M_init_start, MB(2)), "__2M_init_start misaligned") ASSERT(IS_ALIGNED(__2M_init_end, MB(2)), "__2M_init_end misaligned") ASSERT(IS_ALIGNED(__2M_rwdata_start, MB(2)), "__2M_rwdata_start misaligned") ASSERT(IS_ALIGNED(__2M_rwdata_end, MB(2)), "__2M_rwdata_end misaligned") +#else +ASSERT(IS_ALIGNED(__2M_text_end, PAGE_SIZE), "__2M_text_end misaligned") +ASSERT(IS_ALIGNED(__2M_rodata_start, PAGE_SIZE), "__2M_rodata_start misaligned") +ASSERT(IS_ALIGNED(__2M_rodata_end, PAGE_SIZE), "__2M_rodata_end misaligned") +ASSERT(IS_ALIGNED(__2M_init_start, PAGE_SIZE), "__2M_init_start misaligned") +ASSERT(IS_ALIGNED(__2M_init_end, PAGE_SIZE), "__2M_init_end misaligned") +ASSERT(IS_ALIGNED(__2M_rwdata_start, PAGE_SIZE), "__2M_rwdata_start misaligned") +ASSERT(IS_ALIGNED(__2M_rwdata_end, PAGE_SIZE), "__2M_rwdata_end misaligned") #endif ASSERT(IS_ALIGNED(cpu0_stack, STACK_SIZE), "cpu0_stack misaligned") -- 2.1.4 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-03-18 19:49 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-03-17 12:43 [PATCH] xen/x86: Remap text/data/bss with appropriate permissions Andrew Cooper 2016-03-17 12:43 ` [PATCH] DO NOT APPLY - debug keys for inspecting Xen mappings Andrew Cooper 2016-03-17 14:31 ` [PATCH] xen/x86: Remap text/data/bss with appropriate permissions Jan Beulich 2016-03-17 14:44 ` Andrew Cooper 2016-03-17 15:32 ` Jan Beulich 2016-03-17 16:15 ` Andrew Cooper 2016-03-18 7:25 ` Jan Beulich 2016-03-18 19:49 ` [PATCH v2] " Andrew Cooper
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).