Re: [PATCH for-4.17] xen/arm: Support properly __ro_after_init on Arm

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.08.2022 20:59, Julien Grall wrote:
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -75,10 +75,24 @@ domid_t __read_mostly max_init_domid;
>  
>  static __used void init_done(void)
>  {
> +    int rc;
> +
>      /* Must be done past setting system_state. */
>      unregister_init_virtual_region();
>  
>      free_init_memory();
> +
> +    /*
> +     * We have finished to boot. Mark the section .data.ro_after_init
> +     * read-only.
> +     */
> +    rc = modify_xen_mappings((unsigned long)&__ro_after_init_start,
> +                             (unsigned long)&__ro_after_init_end,
> +                             PAGE_HYPERVISOR_RO);
> +    if ( rc )
> +        panic("Unable to mark the .data.ro_after_init section read-only (rc 
> = %d)\n",
> +              rc);

Just wondering - is this really worth panic()ing?

> --- a/xen/arch/arm/xen.lds.S
> +++ b/xen/arch/arm/xen.lds.S
> @@ -83,6 +83,13 @@ SECTIONS
>    _erodata = .;                /* End of read-only data */
>  
>    . = ALIGN(PAGE_SIZE);
> +  .data.ro_after_init : {
> +      __ro_after_init_start = .;
> +      *(.data.ro_after_init)
> +      . = ALIGN(PAGE_SIZE);
> +      __ro_after_init_end = .;
> +  } : text

Again just wondering: Wouldn't it be an option to avoid the initial
page size alignment (and the resulting padding) here, simply making
.data.ro_after_init part of .rodata and do the earlier write-protection
only up to (but excluding) the page containing __ro_after_init_start?

Jan