Re: [PATCH] xen/x86: Remap text/data/bss with appropriate permissions

[Andrew Cooper <andrew.cooper3@citrix.com>]

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