Re: [PATCH v2 11/13] xsplice: Add support for bug frames. (v2)

[Ross Lagerwall <ross.lagerwall@citrix.com>]

On 01/14/2016 09:47 PM, Konrad Rzeszutek Wilk wrote:
> From: Ross Lagerwall <ross.lagerwall@citrix.com>
>
> Add support for handling bug frames contained with xsplice modules. If a
> trap occurs search either the kernel bug table or an applied payload's
> bug table depending on the instruction pointer.
>
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
snip

> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

> diff --git a/xen/common/xsplice.c b/xen/common/xsplice.c
> index 5abeb28..02cb4a8 100644
> --- a/xen/common/xsplice.c
> +++ b/xen/common/xsplice.c
> @@ -43,7 +43,10 @@ struct payload {
>       struct list_head applied_list;       /* Linked to 'applied_list'. */
>       struct xsplice_patch_func *funcs;    /* The array of functions to patch. */
>       unsigned int nfuncs;                 /* Nr of functions to patch. */
> -
> +    size_t core_size;                    /* Only .text size. */
> +    size_t core_text_size;               /* Everything else - .data,.rodata, etc. */

These comments are the wrong way around.

> +    struct bug_frame *start_bug_frames[BUGFRAME_NR]; /* .bug.frame patching. */
> +    struct bug_frame *stop_bug_frames[BUGFRAME_NR];
>       char name[XEN_XSPLICE_NAME_SIZE + 1];/* Name of it. */
>   };
>
> @@ -544,26 +547,27 @@ static void free_payload_data(struct payload *payload)
>       payload->payload_pages = 0;
>   }
>
> -static void calc_section(struct xsplice_elf_sec *sec, size_t *core_size)
> +static void calc_section(struct xsplice_elf_sec *sec, size_t *size)
>   {
> -    size_t align_size = ROUNDUP(*core_size, sec->sec->sh_addralign);
> +    size_t align_size = ROUNDUP(*size, sec->sec->sh_addralign);
>       sec->sec->sh_entsize = align_size;
> -    *core_size = sec->sec->sh_size + align_size;
> +    *size = sec->sec->sh_size + align_size;
>   }
>
>   static int move_payload(struct payload *payload, struct xsplice_elf *elf)
>   {
>       uint8_t *buf;
>       unsigned int i;
> -    size_t core_size = 0;
> +    size_t size = 0;
>
>       /* Compute text regions */
>       for ( i = 0; i < elf->hdr->e_shnum; i++ )
>       {
>           if ( (elf->sec[i].sec->sh_flags & (SHF_ALLOC|SHF_EXECINSTR)) ==
>                (SHF_ALLOC|SHF_EXECINSTR) )
> -            calc_section(&elf->sec[i], &core_size);
> +            calc_section(&elf->sec[i], &size);
>       }
> +    payload->core_text_size = size;
>
>       /* Compute rw data */
>       for ( i = 0; i < elf->hdr->e_shnum; i++ )
> @@ -571,7 +575,7 @@ static int move_payload(struct payload *payload, struct xsplice_elf *elf)
>           if ( (elf->sec[i].sec->sh_flags & SHF_ALLOC) &&
>                !(elf->sec[i].sec->sh_flags & SHF_EXECINSTR) &&
>                (elf->sec[i].sec->sh_flags & SHF_WRITE) )
> -            calc_section(&elf->sec[i], &core_size);
> +            calc_section(&elf->sec[i], &size);
>       }
>
>       /* Compute ro data */
> @@ -580,16 +584,17 @@ static int move_payload(struct payload *payload, struct xsplice_elf *elf)
>           if ( (elf->sec[i].sec->sh_flags & SHF_ALLOC) &&
>                !(elf->sec[i].sec->sh_flags & SHF_EXECINSTR) &&
>                !(elf->sec[i].sec->sh_flags & SHF_WRITE) )
> -            calc_section(&elf->sec[i], &core_size);
> +            calc_section(&elf->sec[i], &size);
>       }
> +    payload->core_size = size;
>
> -    buf = alloc_payload(core_size);
> +    buf = alloc_payload(size);
>       if ( !buf ) {
>           printk(XENLOG_ERR "%s: Could not allocate memory for module\n",
>                  elf->name);
>           return -ENOMEM;
>       }
> -    memset(buf, 0, core_size);
> +    memset(buf, 0, size);
>
>       for ( i = 0; i < elf->hdr->e_shnum; i++ )
>       {
> @@ -604,7 +609,7 @@ static int move_payload(struct payload *payload, struct xsplice_elf *elf)
>       }
>
>       payload->payload_address = buf;
> -    payload->payload_pages = PFN_UP(core_size);
> +    payload->payload_pages = PFN_UP(size);

These renames should be folded into the originating patch (patch 8) or 
dropped.

>
>       return 0;
>   }
> @@ -647,6 +652,22 @@ static int find_special_sections(struct payload *payload,
>               if ( f->pad[j] )
>                   return -EINVAL;
>       }
> +    for ( i = 0; i < BUGFRAME_NR; i++ )
> +    {
> +        char str[14];
> +
> +        snprintf(str, sizeof str, ".bug_frames.%d", i);
> +        sec = xsplice_elf_sec_by_name(elf, str);
> +        if ( !sec )
> +            continue;
> +
> +        if ( ( !sec->sec->sh_size ) ||
> +             ( sec->sec->sh_size % sizeof (struct bug_frame) ) )
> +            return -EINVAL;
> +
> +        payload->start_bug_frames[i] = (struct bug_frame *)sec->load_addr;
> +        payload->stop_bug_frames[i] = (struct bug_frame *)(sec->load_addr + sec->sec->sh_size);
> +    }
>       return 0;
>   }
>
> @@ -942,6 +963,72 @@ void do_xsplice(void)
>       }
>   }
>
> +
> +/*
> + * Functions for handling special sections.
> + */
> +struct bug_frame *xsplice_find_bug(const char *eip, int *id)
> +{
> +    struct payload *data;
> +    struct bug_frame *bug;
> +    int i;
> +
> +    /* No locking since this list is only ever changed during apply or revert
> +     * context. */
> +    list_for_each_entry ( data, &applied_list, applied_list )
> +    {
> +        for (i = 0; i < 4; i++) {

BUGFRAME_NR

> +            if (!data->start_bug_frames[i])
> +                continue;
> +            if ( !((void *)eip >= data->payload_address &&
> +                   (void *)eip < (data->payload_address + data->core_text_size)))
> +                continue;
> +
> +            for ( bug = data->start_bug_frames[i]; bug != data->stop_bug_frames[i]; ++bug ) {
> +                if ( bug_loc(bug) == eip )
> +                {
> +                    *id = i;
> +                    return bug;
> +                }
> +            }
> +        }
> +    }
> +
> +    return NULL;
> +}
> +

-- 
Ross Lagerwall