Re: [PATCH v5 08/20] linux-user: Load vdso image if available

[Philippe Mathieu-Daudé]

Hi Richard,

On 30/8/23 00:02, Richard Henderson wrote:
> The vdso image will be pre-processed into a C data array, with
> a simple list of relocations to perform, and identifying the
> location of signal trampolines.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   linux-user/elfload.c | 87 +++++++++++++++++++++++++++++++++++++++-----
>   1 file changed, 78 insertions(+), 9 deletions(-)
>
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index f34fb64c0c..2a6adebb4a 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -33,6 +33,19 @@
>   #undef ELF_ARCH
>   #endif
>   
> +#ifndef TARGET_ARCH_HAS_SIGTRAMP_PAGE
> +#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 0

I'd rather #error here so new targets are added with
a clearly defined TARGET_ARCH_HAS_SIGTRAMP_PAGE.

> +#endif
> +
> +typedef struct {
> +    const uint8_t *image;
> +    const uint32_t *relocs;
> +    unsigned image_size;
> +    unsigned reloc_count;
> +    unsigned sigreturn_ofs;
> +    unsigned rt_sigreturn_ofs;
> +} VdsoImageInfo;
> +
>   #define ELF_OSABI   ELFOSABI_SYSV


> +#ifndef vdso_image_info
> +#define vdso_image_info()    NULL
> +#endif
> +
> +static void load_elf_vdso(struct image_info *info, const VdsoImageInfo *vdso)
> +{
> +    ImageSource src;
> +    struct elfhdr ehdr;
> +    abi_ulong load_bias, load_addr;
> +
> +    src.fd = -1;
> +    src.cache = vdso->image;
> +    src.cache_size = vdso->image_size;
> +
> +    load_elf_image("<internal-vdso>", &src, info, &ehdr, NULL);
> +    load_addr = info->load_addr;
> +    load_bias = info->load_bias;
> +
> +    /*
> +     * We need to relocate the VDSO image.  The one built into the kernel
> +     * is built for a fixed address.  The one built for QEMU is not, since
> +     * that requires close control of the guest address space.
> +     * We pre-processed the image to locate all of the addresses that need
> +     * to be updated.
> +     */
> +    for (unsigned i = 0, n = vdso->reloc_count; i < n; i++) {

Do we really need 'n'?

> +        abi_ulong *addr = g2h_untagged(load_addr + vdso->relocs[i]);
> +        *addr = tswapal(tswapal(*addr) + load_bias);
> +    }
> +
> +    /* Install signal trampolines, if present. */
> +    if (vdso->sigreturn_ofs) {
> +        default_sigreturn = load_addr + vdso->sigreturn_ofs;
> +    }
> +    if (vdso->rt_sigreturn_ofs) {
> +        default_rt_sigreturn = load_addr + vdso->rt_sigreturn_ofs;
> +    }
> +
> +    /* Remove write from VDSO segment. */
> +    target_mprotect(info->start_data, info->end_data - info->start_data,
> +                    PROT_READ | PROT_EXEC);
> +}

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>