linux-user/gen-vdso-elfn.c.inc | 89 ++++++++++++++++++++++------------ linux-user/gen-vdso.c | 40 +++++++-------- 2 files changed, 79 insertions(+), 50 deletions(-)
When debugging qemu-user processes using gdbstub, the following warning
appears every time:
warning: BFD: warning: system-supplied DSO at 0x7f8253cc3000 has a corrupt string table index
The reason is that QEMU does not map the VDSO's section headers. The
VDSO's ELF header's e_shoff points to zeros, which GDB fails to parse.
The difference with the kernel's VDSO is that the latter is mapped as a
blob, ignoring program headers - which also don't cover the section
table. QEMU, on the other hand, loads it as an ELF file.
There are multiple ways to resolve it:
- Copy VDSO as a blob in load_elf_vdso(). This would require creating
specialized loader logic, that duplicates parts of load_elf_image().
- Fix up VDSO's PHDR size in load_elf_vdso(). This would require either
duplicating the parsing logic, or adding an ugly parameter to
load_elf_image().
- Fix up VDSO's PHDR size in gen-vdso. This is the simplest solution,
so do it. The only tricky part is byte-swaps: they need to be either
done on local copies or in-place, and then reverted in the end. To
preserve the existing code structure, do the former for Sym and Dyn,
and the latter for Ehdr, Phdr, and Shdr.
To verify this change, I diffed the on-disk and the loaded VDSO; the
result does not show anything unusual, except for what seems to be an
existing oversight (which should probably be fixed separately):
│ Symbol table '.dynsym' contains 8 entries:
│ Num: Value Size Type Bind Vis Ndx Name
│ - 0: 0000000000000000 0 NOTYPE LOCAL DEFAULT UND
│ - 6: 0000000000000000 0 OBJECT GLOBAL DEFAULT ABS LINUX_2.6.29
│ + 0: 00007f61075bf000 0 NOTYPE LOCAL DEFAULT UND
│ + 6: 00007f61075bf000 0 OBJECT GLOBAL DEFAULT ABS LINUX_2.6.29
Fixes: 2fa536d10797 ("linux-user: Add gen-vdso tool")
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
linux-user/gen-vdso-elfn.c.inc | 89 ++++++++++++++++++++++------------
linux-user/gen-vdso.c | 40 +++++++--------
2 files changed, 79 insertions(+), 50 deletions(-)
diff --git a/linux-user/gen-vdso-elfn.c.inc b/linux-user/gen-vdso-elfn.c.inc
index 95856eb839b..82002df4fc0 100644
--- a/linux-user/gen-vdso-elfn.c.inc
+++ b/linux-user/gen-vdso-elfn.c.inc
@@ -68,28 +68,45 @@ static void elfN(search_symtab)(ElfN(Shdr) *shdr, unsigned sym_idx,
void *buf, bool need_bswap)
{
unsigned str_idx = shdr[sym_idx].sh_link;
- ElfN(Sym) *sym = buf + shdr[sym_idx].sh_offset;
- unsigned sym_n = shdr[sym_idx].sh_size / sizeof(*sym);
+ ElfN(Sym) *target_sym = buf + shdr[sym_idx].sh_offset;
+ unsigned sym_n = shdr[sym_idx].sh_size / sizeof(*target_sym);
const char *str = buf + shdr[str_idx].sh_offset;
for (unsigned i = 0; i < sym_n; ++i) {
const char *name;
+ ElfN(Sym) sym;
+ memcpy(&sym, &target_sym[i], sizeof(sym));
if (need_bswap) {
- elfN(bswap_sym)(sym + i);
+ elfN(bswap_sym)(&sym);
}
- name = str + sym[i].st_name;
+ name = str + sym.st_name;
if (sigreturn_sym && strcmp(sigreturn_sym, name) == 0) {
- sigreturn_addr = sym[i].st_value;
+ sigreturn_addr = sym.st_value;
}
if (rt_sigreturn_sym && strcmp(rt_sigreturn_sym, name) == 0) {
- rt_sigreturn_addr = sym[i].st_value;
+ rt_sigreturn_addr = sym.st_value;
}
}
}
-static void elfN(process)(FILE *outf, void *buf, bool need_bswap)
+static void elfN(bswap_ps_hdrs)(ElfN(Ehdr) *ehdr)
+{
+ ElfN(Phdr) *phdr = (void *)ehdr + ehdr->e_phoff;
+ ElfN(Shdr) *shdr = (void *)ehdr + ehdr->e_shoff;
+ ElfN(Half) i;
+
+ for (i = 0; i < ehdr->e_phnum; ++i) {
+ elfN(bswap_phdr)(&phdr[i]);
+ }
+
+ for (i = 0; i < ehdr->e_shnum; ++i) {
+ elfN(bswap_shdr)(&shdr[i]);
+ }
+}
+
+static void elfN(process)(FILE *outf, void *buf, long len, bool need_bswap)
{
ElfN(Ehdr) *ehdr = buf;
ElfN(Phdr) *phdr;
@@ -103,24 +120,14 @@ static void elfN(process)(FILE *outf, void *buf, bool need_bswap)
int errors = 0;
if (need_bswap) {
- elfN(bswap_ehdr)(ehdr);
+ elfN(bswap_ehdr)(buf);
+ elfN(bswap_ps_hdrs)(buf);
}
phnum = ehdr->e_phnum;
phdr = buf + ehdr->e_phoff;
- if (need_bswap) {
- for (unsigned i = 0; i < phnum; ++i) {
- elfN(bswap_phdr)(phdr + i);
- }
- }
-
shnum = ehdr->e_shnum;
shdr = buf + ehdr->e_shoff;
- if (need_bswap) {
- for (unsigned i = 0; i < shnum; ++i) {
- elfN(bswap_shdr)(shdr + i);
- }
- }
for (unsigned i = 0; i < shnum; ++i) {
switch (shdr[i].sh_type) {
case SHT_SYMTAB:
@@ -154,6 +161,16 @@ static void elfN(process)(FILE *outf, void *buf, bool need_bswap)
fprintf(stderr, "LOAD segment not loaded at address 0\n");
errors++;
}
+ /*
+ * Extend the program header to cover the entire VDSO, so that
+ * load_elf_vdso() loads everything, including section headers.
+ */
+ if (len > phdr[i].p_filesz) {
+ phdr[i].p_filesz = len;
+ }
+ if (len > phdr[i].p_memsz) {
+ phdr[i].p_memsz = len;
+ }
first_segsz = phdr[i].p_filesz;
if (first_segsz < ehdr->e_phoff + phnum * sizeof(*phdr)) {
fprintf(stderr, "LOAD segment does not cover PHDRs\n");
@@ -197,17 +214,24 @@ static void elfN(process)(FILE *outf, void *buf, bool need_bswap)
output_reloc(outf, buf, &phdr[i].p_paddr);
}
+ /* Relocate the section headers. */
+ for (unsigned i = 0; i < shnum; ++i) {
+ output_reloc(outf, buf, &shdr[i].sh_addr);
+ }
+
/* Relocate the DYNAMIC entries. */
if (dynamic_addr) {
- ElfN(Dyn) *dyn = buf + dynamic_ofs;
- __typeof(dyn->d_tag) tag;
+ ElfN(Dyn) *target_dyn = buf + dynamic_ofs;
+ __typeof(((ElfN(Dyn) *)target_dyn)->d_tag) tag;
do {
+ ElfN(Dyn) dyn;
+ memcpy(&dyn, target_dyn, sizeof(dyn));
if (need_bswap) {
- elfN(bswap_dyn)(dyn);
+ elfN(bswap_dyn)(&dyn);
}
- tag = dyn->d_tag;
+ tag = dyn.d_tag;
switch (tag) {
case DT_HASH:
@@ -218,7 +242,7 @@ static void elfN(process)(FILE *outf, void *buf, bool need_bswap)
case DT_PLTGOT:
case DT_ADDRRNGLO ... DT_ADDRRNGHI:
/* These entries store an address in the entry. */
- output_reloc(outf, buf, &dyn->d_un.d_val);
+ output_reloc(outf, buf, &target_dyn->d_un.d_val);
break;
case DT_NULL:
@@ -235,7 +259,7 @@ static void elfN(process)(FILE *outf, void *buf, bool need_bswap)
break;
case DT_SYMENT:
- if (dyn->d_un.d_val != sizeof(ElfN(Sym))) {
+ if (dyn.d_un.d_val != sizeof(ElfN(Sym))) {
fprintf(stderr, "VDSO has incorrect dynamic symbol size\n");
errors++;
}
@@ -251,7 +275,7 @@ static void elfN(process)(FILE *outf, void *buf, bool need_bswap)
* ??? The RISC-V toolchain will emit these even when there
* are no relocations. Validate zeros.
*/
- if (dyn->d_un.d_val != 0) {
+ if (dyn.d_un.d_val != 0) {
fprintf(stderr, "VDSO has dynamic relocations\n");
errors++;
}
@@ -287,7 +311,7 @@ static void elfN(process)(FILE *outf, void *buf, bool need_bswap)
errors++;
break;
}
- dyn++;
+ target_dyn++;
} while (tag != DT_NULL);
if (errors) {
exit(EXIT_FAILURE);
@@ -296,11 +320,11 @@ static void elfN(process)(FILE *outf, void *buf, bool need_bswap)
/* Relocate the dynamic symbol table. */
if (dynsym_idx) {
- ElfN(Sym) *sym = buf + shdr[dynsym_idx].sh_offset;
- unsigned sym_n = shdr[dynsym_idx].sh_size / sizeof(*sym);
+ ElfN(Sym) *target_sym = buf + shdr[dynsym_idx].sh_offset;
+ unsigned sym_n = shdr[dynsym_idx].sh_size / sizeof(*target_sym);
for (unsigned i = 0; i < sym_n; ++i) {
- output_reloc(outf, buf, &sym[i].st_value);
+ output_reloc(outf, buf, &target_sym[i].st_value);
}
}
@@ -311,4 +335,9 @@ static void elfN(process)(FILE *outf, void *buf, bool need_bswap)
if (symtab_idx) {
elfN(search_symtab)(shdr, symtab_idx, buf, need_bswap);
}
+
+ if (need_bswap) {
+ elfN(bswap_ps_hdrs)(buf);
+ elfN(bswap_ehdr)(buf);
+ }
}
diff --git a/linux-user/gen-vdso.c b/linux-user/gen-vdso.c
index 31e333be802..721f38d5a3b 100644
--- a/linux-user/gen-vdso.c
+++ b/linux-user/gen-vdso.c
@@ -131,23 +131,6 @@ int main(int argc, char **argv)
}
fclose(inf);
- /*
- * Write out the vdso image now, before we make local changes.
- */
-
- fprintf(outf,
- "/* Automatically generated from linux-user/gen-vdso.c. */\n"
- "\n"
- "static const uint8_t %s_image[] = {",
- prefix);
- for (long i = 0; i < total_len; ++i) {
- if (i % 12 == 0) {
- fputs("\n ", outf);
- }
- fprintf(outf, " 0x%02x,", buf[i]);
- }
- fprintf(outf, "\n};\n\n");
-
/*
* Identify which elf flavor we're processing.
* The first 16 bytes of the file are e_ident.
@@ -179,14 +162,17 @@ int main(int argc, char **argv)
* Output relocation addresses as we go.
*/
- fprintf(outf, "static const unsigned %s_relocs[] = {\n", prefix);
+ fprintf(outf,
+ "/* Automatically generated by linux-user/gen-vdso.c. */\n"
+ "\n"
+ "static const unsigned %s_relocs[] = {\n", prefix);
switch (buf[EI_CLASS]) {
case ELFCLASS32:
- elf32_process(outf, buf, need_bswap);
+ elf32_process(outf, buf, total_len, need_bswap);
break;
case ELFCLASS64:
- elf64_process(outf, buf, need_bswap);
+ elf64_process(outf, buf, total_len, need_bswap);
break;
default:
fprintf(stderr, "%s: invalid elf EI_CLASS (%u)\n",
@@ -196,6 +182,20 @@ int main(int argc, char **argv)
fprintf(outf, "};\n\n"); /* end vdso_relocs. */
+ /*
+ * Write out the vdso image now, after we made local changes.
+ */
+ fprintf(outf,
+ "static const uint8_t %s_image[] = {",
+ prefix);
+ for (long i = 0; i < total_len; ++i) {
+ if (i % 12 == 0) {
+ fputs("\n ", outf);
+ }
+ fprintf(outf, " 0x%02x,", buf[i]);
+ }
+ fprintf(outf, "\n};\n\n");
+
fprintf(outf, "static const VdsoImageInfo %s_image_info = {\n", prefix);
fprintf(outf, " .image = %s_image,\n", prefix);
fprintf(outf, " .relocs = %s_relocs,\n", prefix);
--
2.47.0
On 10/23/24 07:46, Ilya Leoshkevich wrote:
> When debugging qemu-user processes using gdbstub, the following warning
> appears every time:
>
> warning: BFD: warning: system-supplied DSO at 0x7f8253cc3000 has a corrupt string table index
>
> The reason is that QEMU does not map the VDSO's section headers. The
> VDSO's ELF header's e_shoff points to zeros, which GDB fails to parse.
Interesting. I had wondered where this came from, but never looked.
> - Fix up VDSO's PHDR size in gen-vdso. This is the simplest solution,
> so do it. The only tricky part is byte-swaps: they need to be either
> done on local copies or in-place, and then reverted in the end. To
> preserve the existing code structure, do the former for Sym and Dyn,
> and the latter for Ehdr, Phdr, and Shdr.
Or adjust the linker script, to mark those sections loaded.
This may or may not be easier, considering the rest of the changes.
> @@ -154,6 +161,16 @@ static void elfN(process)(FILE *outf, void *buf, bool need_bswap)
> fprintf(stderr, "LOAD segment not loaded at address 0\n");
> errors++;
> }
> + /*
> + * Extend the program header to cover the entire VDSO, so that
> + * load_elf_vdso() loads everything, including section headers.
> + */
> + if (len > phdr[i].p_filesz) {
> + phdr[i].p_filesz = len;
> + }
> + if (len > phdr[i].p_memsz) {
> + phdr[i].p_memsz = len;
> + }
There should be no .bss, so these two numbers had better be identical. Certainly this
adjustment *requires* that there be no .bss. I think we should error out if the two
numbers differ.
r~
On Wed, 2024-10-23 at 12:03 -0700, Richard Henderson wrote:
> On 10/23/24 07:46, Ilya Leoshkevich wrote:
> >
[...]
>
> > - Fix up VDSO's PHDR size in gen-vdso. This is the simplest
> > solution,
> > so do it. The only tricky part is byte-swaps: they need to be
> > either
> > done on local copies or in-place, and then reverted in the end.
> > To
> > preserve the existing code structure, do the former for Sym and
> > Dyn,
> > and the latter for Ehdr, Phdr, and Shdr.
>
> Or adjust the linker script, to mark those sections loaded.
> This may or may not be easier, considering the rest of the changes.
I forgot to mention that I investigated this too. The problem, as I see
it, is that there appears to be no way to place section headers inside
a section, and, therefore, no way to refer to them in a linker
script. I guess someone could add the ability to do this by defining
SHDRS keyword in addition to the existing FILEHDR and PHDRS. Also, ld
hardcodes section headers to be non-loadable:
static bool
_bfd_elf_assign_file_positions_for_non_load (bfd *abfd)
{
[...]
/* Place the section headers. */
i_ehdrp = elf_elfheader (abfd);
off = BFD_ALIGN (off, 1u << bed->s->log_file_align);
i_ehdrp->e_shoff = off;
off += i_ehdrp->e_shnum * i_ehdrp->e_shentsize;
elf_next_file_pos (abfd) = off;
So I figured it would be better to stay as close as possible to what
the kernel is doing without changing the existing loader design too
much.
> > @@ -154,6 +161,16 @@ static void elfN(process)(FILE *outf, void
> > *buf, bool need_bswap)
> > fprintf(stderr, "LOAD segment not loaded at address
> > 0\n");
> > errors++;
> > }
> > + /*
> > + * Extend the program header to cover the entire VDSO, so
> > that
> > + * load_elf_vdso() loads everything, including section
> > headers.
> > + */
> > + if (len > phdr[i].p_filesz) {
> > + phdr[i].p_filesz = len;
> > + }
> > + if (len > phdr[i].p_memsz) {
> > + phdr[i].p_memsz = len;
> > + }
>
> There should be no .bss, so these two numbers had better be
> identical. Certainly this
> adjustment *requires* that there be no .bss. I think we should error
> out if the two
> numbers differ.
Sounds reasonable, I'll send a v2 with the check.
[...]
© 2016 - 2024 Red Hat, Inc.