| Message ID | 20230829220228.928506-9-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | linux-user: Implement VDSOs | expand |
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>
On 8/30/23 07:22, Philippe Mathieu-Daudé wrote: > 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. The next step after adding vdso's is to remove TARGET_ARCH_HAS_SIGTRAMP_PAGE. >> + for (unsigned i = 0, n = vdso->reloc_count; i < n; i++) { > > Do we really need 'n'? You should always have an loop invariant condition if possible. r~
On 30/8/23 18:17, Richard Henderson wrote: > On 8/30/23 07:22, Philippe Mathieu-Daudé wrote: >> 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. > > The next step after adding vdso's is to remove > TARGET_ARCH_HAS_SIGTRAMP_PAGE. Ah great. >>> + for (unsigned i = 0, n = vdso->reloc_count; i < n; i++) { >> >> Do we really need 'n'? > > You should always have an loop invariant condition if possible. vdso->reloc_count doesn't seem updated, but I get your point.
On 8/30/23 13:56, Philippe Mathieu-Daudé wrote: >>>> + for (unsigned i = 0, n = vdso->reloc_count; i < n; i++) { >>> >>> Do we really need 'n'? >> >> You should always have an loop invariant condition if possible. > > vdso->reloc_count doesn't seem updated, but I get your point. But the compiler doesn't know that -- it must assume that the store to *addr may overlap reloc_count. r~
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 +#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 /* from personality.h */ @@ -2292,7 +2305,8 @@ static abi_ulong loader_build_fdpic_loadmap(struct image_info *info, abi_ulong s static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc, struct elfhdr *exec, struct image_info *info, - struct image_info *interp_info) + struct image_info *interp_info, + struct image_info *vdso_info) { abi_ulong sp; abi_ulong u_argc, u_argv, u_envp, u_auxv; @@ -2380,10 +2394,15 @@ static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc, } size = (DLINFO_ITEMS + 1) * 2; - if (k_base_platform) + if (k_base_platform) { size += 2; - if (k_platform) + } + if (k_platform) { size += 2; + } + if (vdso_info) { + size += 2; + } #ifdef DLINFO_ARCH_ITEMS size += DLINFO_ARCH_ITEMS * 2; #endif @@ -2465,6 +2484,9 @@ static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc, if (u_platform) { NEW_AUX_ENT(AT_PLATFORM, u_platform); } + if (vdso_info) { + NEW_AUX_ENT(AT_SYSINFO_EHDR, vdso_info->load_addr); + } NEW_AUX_ENT (AT_NULL, 0); #undef NEW_AUX_ENT @@ -3342,6 +3364,49 @@ static void load_elf_interp(const char *filename, struct image_info *info, load_elf_image(filename, &src, info, &ehdr, NULL); } +#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++) { + 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); +} + static int symfind(const void *s0, const void *s1) { struct elf_sym *sym = (struct elf_sym *)s1; @@ -3547,7 +3612,7 @@ int load_elf_binary(struct linux_binprm *bprm, struct image_info *info) * and let elf_load_image do any swapping that may be required. */ struct elfhdr ehdr; - struct image_info interp_info; + struct image_info interp_info, vdso_info; char *elf_interpreter = NULL; char *scratch; @@ -3630,10 +3695,13 @@ int load_elf_binary(struct linux_binprm *bprm, struct image_info *info) } /* - * TODO: load a vdso, which would also contain the signal trampolines. - * Otherwise, allocate a private page to hold them. + * Load a vdso if available, which will amongst other things contain the + * signal trampolines. Otherwise, allocate a separate page for them. */ - if (TARGET_ARCH_HAS_SIGTRAMP_PAGE) { + const VdsoImageInfo *vdso = vdso_image_info(); + if (vdso) { + load_elf_vdso(&vdso_info, vdso); + } else if (TARGET_ARCH_HAS_SIGTRAMP_PAGE) { abi_long tramp_page = target_mmap(0, TARGET_PAGE_SIZE, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0); @@ -3645,8 +3713,9 @@ int load_elf_binary(struct linux_binprm *bprm, struct image_info *info) target_mprotect(tramp_page, TARGET_PAGE_SIZE, PROT_READ | PROT_EXEC); } - bprm->p = create_elf_tables(bprm->p, bprm->argc, bprm->envc, &ehdr, - info, (elf_interpreter ? &interp_info : NULL)); + bprm->p = create_elf_tables(bprm->p, bprm->argc, bprm->envc, &ehdr, info, + elf_interpreter ? &interp_info : NULL, + vdso ? &vdso_info : NULL); info->start_stack = bprm->p; /* If we have an interpreter, set that as the program's entry point.
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(-)