2014-06-18 23:00:08

by Andy Lutomirski

[permalink] [raw]
Subject: [PATCH 0/5] x86,vdso: Restore a bunch of section headers

This series makes me sad. It brings the 64-bit vdso back above 4kB,
like it was in 3.15. It's also just silly, but it seems to be
needed to keep binutils happy when debugging the vdso.

Patch 5 isn't really necessary, and is likely not 3.16 material.
It's useful for testing the rest of this series, though.

Andy Lutomirski (5):
x86,vdso: Discard the __bug_table section
x86,vdso2c: Use better macros for ELF bitness
x86,vdso: Improve the fake section headers
x86,vdso: Remove some redundant in-memory section headers
x86,vdso: Create .build-id links for unstripped vdso files

arch/x86/vdso/Makefile | 13 +-
arch/x86/vdso/vdso-fakesections.c | 41 +++----
arch/x86/vdso/vdso-layout.lds.S | 64 +++++++---
arch/x86/vdso/vdso.lds.S | 2 +
arch/x86/vdso/vdso2c.c | 73 ++++++------
arch/x86/vdso/vdso2c.h | 197 ++++++++++++++++++++++++++-----
arch/x86/vdso/vdso32/vdso-fakesections.c | 1 +
arch/x86/vdso/vdsox32.lds.S | 2 +
8 files changed, 279 insertions(+), 114 deletions(-)
create mode 100644 arch/x86/vdso/vdso32/vdso-fakesections.c

--
1.9.3


2014-06-18 23:00:11

by Andy Lutomirski

[permalink] [raw]
Subject: [PATCH 1/5] x86,vdso: Discard the __bug_table section

It serves no purpose in user code.

Signed-off-by: Andy Lutomirski <[email protected]>
---
arch/x86/vdso/vdso-layout.lds.S | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/x86/vdso/vdso-layout.lds.S b/arch/x86/vdso/vdso-layout.lds.S
index 2ec72f6..c84166c 100644
--- a/arch/x86/vdso/vdso-layout.lds.S
+++ b/arch/x86/vdso/vdso-layout.lds.S
@@ -75,6 +75,7 @@ SECTIONS
/DISCARD/ : {
*(.discard)
*(.discard.*)
+ *(__bug_table)
}
}

--
1.9.3

2014-06-18 23:00:18

by Andy Lutomirski

[permalink] [raw]
Subject: [PATCH 3/5] x86,vdso: Improve the fake section headers

Fully stripping the vDSO has other unfortunate side effects:

- binutils is unable to find ELF notes without a SHT_NOTE section.

- Even elfutils has trouble: it can find ELF notes without a section
table at all, but if a section table is present, it won't look for
PT_NOTE.

- gdb wants section names to match between stripped DSOs and their
symbols; otherwise it will corrupt symbol addresses.

We're also breaking the rules: section 0 is supposed to be SHT_NULL.

Fix these problems by building a better fake section table. While
we're at it, we might as well let buggy Go versions keep working well
by giving the SHT_DYNSYM entry the correct size.

This is a bit unfortunate: it adds quite a bit of size to the vdso
image.

If/when binutils improves and the improved versions become widespread,
it would be worth considering dropping most of this.

Signed-off-by: Andy Lutomirski <[email protected]>
---
arch/x86/vdso/Makefile | 4 +-
arch/x86/vdso/vdso-fakesections.c | 44 ++++----
arch/x86/vdso/vdso-layout.lds.S | 40 +++++--
arch/x86/vdso/vdso.lds.S | 2 +
arch/x86/vdso/vdso2c.c | 31 ++++--
arch/x86/vdso/vdso2c.h | 180 +++++++++++++++++++++++++++----
arch/x86/vdso/vdso32/vdso-fakesections.c | 1 +
arch/x86/vdso/vdsox32.lds.S | 2 +
8 files changed, 237 insertions(+), 67 deletions(-)
create mode 100644 arch/x86/vdso/vdso32/vdso-fakesections.c

diff --git a/arch/x86/vdso/Makefile b/arch/x86/vdso/Makefile
index 3c0809a..2c1ca98 100644
--- a/arch/x86/vdso/Makefile
+++ b/arch/x86/vdso/Makefile
@@ -11,7 +11,6 @@ VDSO32-$(CONFIG_COMPAT) := y

# files to link into the vdso
vobjs-y := vdso-note.o vclock_gettime.o vgetcpu.o vdso-fakesections.o
-vobjs-nox32 := vdso-fakesections.o

# files to link into kernel
obj-y += vma.o
@@ -134,7 +133,7 @@ override obj-dirs = $(dir $(obj)) $(obj)/vdso32/

targets += vdso32/vdso32.lds
targets += vdso32/note.o vdso32/vclock_gettime.o $(vdso32.so-y:%=vdso32/%.o)
-targets += vdso32/vclock_gettime.o
+targets += vdso32/vclock_gettime.o vdso32/vdso-fakesections.o

$(obj)/vdso32.o: $(vdso32-images:%=$(obj)/%)

@@ -155,6 +154,7 @@ $(vdso32-images:%=$(obj)/%.dbg): KBUILD_CFLAGS = $(KBUILD_CFLAGS_32)
$(vdso32-images:%=$(obj)/%.dbg): $(obj)/vdso32-%.so.dbg: FORCE \
$(obj)/vdso32/vdso32.lds \
$(obj)/vdso32/vclock_gettime.o \
+ $(obj)/vdso32/vdso-fakesections.o \
$(obj)/vdso32/note.o \
$(obj)/vdso32/%.o
$(call if_changed,vdso)
diff --git a/arch/x86/vdso/vdso-fakesections.c b/arch/x86/vdso/vdso-fakesections.c
index cb8a8d7..56927a7 100644
--- a/arch/x86/vdso/vdso-fakesections.c
+++ b/arch/x86/vdso/vdso-fakesections.c
@@ -2,31 +2,23 @@
* Copyright 2014 Andy Lutomirski
* Subject to the GNU Public License, v.2
*
- * Hack to keep broken Go programs working.
- *
- * The Go runtime had a couple of bugs: it would read the section table to try
- * to figure out how many dynamic symbols there were (it shouldn't have looked
- * at the section table at all) and, if there were no SHT_SYNDYM section table
- * entry, it would use an uninitialized value for the number of symbols. As a
- * workaround, we supply a minimal section table. vdso2c will adjust the
- * in-memory image so that "vdso_fake_sections" becomes the section table.
- *
- * The bug was introduced by:
- * https://code.google.com/p/go/source/detail?r=56ea40aac72b (2012-08-31)
- * and is being addressed in the Go runtime in this issue:
- * https://code.google.com/p/go/issues/detail?id=8197
+ * String table for loadable section headers. See vdso2c.h for why
+ * this exists.
*/

-#ifndef __x86_64__
-#error This hack is specific to the 64-bit vDSO
-#endif
-
-#include <linux/elf.h>
-
-extern const __visible struct elf64_shdr vdso_fake_sections[];
-const __visible struct elf64_shdr vdso_fake_sections[] = {
- {
- .sh_type = SHT_DYNSYM,
- .sh_entsize = sizeof(Elf64_Sym),
- }
-};
+const char fake_shstrtab[] __attribute__((section(".fake_shstrtab"))) =
+ ".hash\0"
+ ".dynsym\0"
+ ".dynstr\0"
+ ".gnu.version\0"
+ ".gnu.version_d\0"
+ ".dynamic\0"
+ ".rodata\0"
+ ".fake_shstrtab\0" /* Yay, self-referential code. */
+ ".note\0"
+ ".data\0"
+ ".altinstructions\0"
+ ".altinstr_replacement\0"
+ ".eh_frame_hdr\0"
+ ".eh_frame\0"
+ ".text";
diff --git a/arch/x86/vdso/vdso-layout.lds.S b/arch/x86/vdso/vdso-layout.lds.S
index c84166c..e4cbc21 100644
--- a/arch/x86/vdso/vdso-layout.lds.S
+++ b/arch/x86/vdso/vdso-layout.lds.S
@@ -6,6 +6,16 @@
* This script controls its layout.
*/

+#if defined(BUILD_VDSO64)
+# define SHDR_SIZE 64
+#elif defined(BUILD_VDSO32) || defined(BUILD_VDSOX32)
+# define SHDR_SIZE 40
+#else
+# error unknown VDSO target
+#endif
+
+#define NUM_FAKE_SHDRS 16
+
SECTIONS
{
. = SIZEOF_HEADERS;
@@ -25,15 +35,29 @@ SECTIONS

.dynamic : { *(.dynamic) } :text :dynamic

- .rodata : { *(.rodata*) } :text
+ .rodata : {
+ *(.rodata*)
+
+ /*
+ * Ideally this would live in a C file, but that won't
+ * work cleanly for x32 until we start building the x32
+ * C code using an x32 toolchain.
+ */
+ VDSO_FAKE_SECTION_TABLE_START = .;
+ . = . + NUM_FAKE_SHDRS * SHDR_SIZE;
+ VDSO_FAKE_SECTION_TABLE_END = .;
+ } :text
+
+ .fake_shstrtab : { *(.fake_shstrtab) } :text
+
.data : {
- *(.data*)
- *(.sdata*)
- *(.got.plt) *(.got)
- *(.gnu.linkonce.d.*)
- *(.bss*)
- *(.dynbss*)
- *(.gnu.linkonce.b.*)
+ *(.data*)
+ *(.sdata*)
+ *(.got.plt) *(.got)
+ *(.gnu.linkonce.d.*)
+ *(.bss*)
+ *(.dynbss*)
+ *(.gnu.linkonce.b.*)
}

.altinstructions : { *(.altinstructions) }
diff --git a/arch/x86/vdso/vdso.lds.S b/arch/x86/vdso/vdso.lds.S
index 75e3404..6807932 100644
--- a/arch/x86/vdso/vdso.lds.S
+++ b/arch/x86/vdso/vdso.lds.S
@@ -6,6 +6,8 @@
* the DSO.
*/

+#define BUILD_VDSO64
+
#include "vdso-layout.lds.S"

/*
diff --git a/arch/x86/vdso/vdso2c.c b/arch/x86/vdso/vdso2c.c
index 7343899..238dbe82 100644
--- a/arch/x86/vdso/vdso2c.c
+++ b/arch/x86/vdso/vdso2c.c
@@ -23,6 +23,8 @@ enum {
sym_vvar_page,
sym_hpet_page,
sym_end_mapping,
+ sym_VDSO_FAKE_SECTION_TABLE_START,
+ sym_VDSO_FAKE_SECTION_TABLE_END,
};

const int special_pages[] = {
@@ -30,15 +32,26 @@ const int special_pages[] = {
sym_hpet_page,
};

-char const * const required_syms[] = {
- [sym_vvar_page] = "vvar_page",
- [sym_hpet_page] = "hpet_page",
- [sym_end_mapping] = "end_mapping",
- "VDSO32_NOTE_MASK",
- "VDSO32_SYSENTER_RETURN",
- "__kernel_vsyscall",
- "__kernel_sigreturn",
- "__kernel_rt_sigreturn",
+struct vdso_sym {
+ const char *name;
+ bool export;
+};
+
+struct vdso_sym required_syms[] = {
+ [sym_vvar_page] = {"vvar_page", true},
+ [sym_hpet_page] = {"hpet_page", true},
+ [sym_end_mapping] = {"end_mapping", true},
+ [sym_VDSO_FAKE_SECTION_TABLE_START] = {
+ "VDSO_FAKE_SECTION_TABLE_START", false
+ },
+ [sym_VDSO_FAKE_SECTION_TABLE_END] = {
+ "VDSO_FAKE_SECTION_TABLE_END", false
+ },
+ {"VDSO32_NOTE_MASK", true},
+ {"VDSO32_SYSENTER_RETURN", true},
+ {"__kernel_vsyscall", true},
+ {"__kernel_sigreturn", true},
+ {"__kernel_rt_sigreturn", true},
};

__attribute__((format(printf, 1, 2))) __attribute__((noreturn))
diff --git a/arch/x86/vdso/vdso2c.h b/arch/x86/vdso/vdso2c.h
index 8e185ce..f01ed4b 100644
--- a/arch/x86/vdso/vdso2c.h
+++ b/arch/x86/vdso/vdso2c.h
@@ -4,6 +4,116 @@
* are built for 32-bit userspace.
*/

+/*
+ * We're writing a section table for a few reasons:
+ *
+ * The Go runtime had a couple of bugs: it would read the section
+ * table to try to figure out how many dynamic symbols there were (it
+ * shouldn't have looked at the section table at all) and, if there
+ * were no SHT_SYNDYM section table entry, it would use an
+ * uninitialized value for the number of symbols. An empty DYNSYM
+ * table would work, but I see no reason not to write a valid one (and
+ * keep full performance for old Go programs). This hack is only
+ * needed on x86_64.
+ *
+ * The bug was introduced on 2012-08-31 by:
+ * https://code.google.com/p/go/source/detail?r=56ea40aac72b
+ * and was fixed on 2014-06-13 by:
+ * https://code.google.com/p/go/source/detail?r=fc1cd5e12595
+ *
+ * Binutils has issues debugging the vDSO: it reads the section table to
+ * find SHT_NOTE; it won't look at PT_NOTE for the in-memory vDSO, which
+ * would break build-id if we removed the section table. Binutils
+ * also requires that shstrndx != 0. See:
+ * https://sourceware.org/bugzilla/show_bug.cgi?id=17064
+ *
+ * elfutils might not look for PT_NOTE if there is a section table at
+ * all. I don't know whether this matters for any practical purpose.
+ *
+ * For simplicity, rather than hacking up a partial section table, we
+ * just write a mostly complete one. We omit non-dynamic symbols,
+ * though, since they're rather large.
+ *
+ * Once binutils gets fixed, we might be able to drop this for all but
+ * the 64-bit vdso, since build-id only works in kernel RPMs, and
+ * systems that update to new enough kernel RPMs will likely update
+ * binutils in sync. build-id has never worked for home-built kernel
+ * RPMs without manual symlinking, and I suspect that no one ever does
+ * that.
+ */
+struct BITSFUNC(fake_sections)
+{
+ ELF(Shdr) *table;
+ unsigned long table_offset;
+ int count, max_count;
+
+ int in_shstrndx;
+ unsigned long shstr_offset;
+ const char *shstrtab;
+ size_t shstrtab_len;
+
+ int out_shstrndx;
+};
+
+static unsigned int BITSFUNC(find_shname)(struct BITSFUNC(fake_sections) *out,
+ const char *name)
+{
+ const char *outname = out->shstrtab;
+ while (outname - out->shstrtab < out->shstrtab_len) {
+ if (!strcmp(name, outname))
+ return (outname - out->shstrtab) + out->shstr_offset;
+ outname += strlen(outname) + 1;
+ }
+
+ if (*name)
+ printf("Warning: could not find output name \"%s\"\n", name);
+ return out->shstr_offset + out->shstrtab_len - 1; /* Use a null. */
+}
+
+static void BITSFUNC(init_sections)(struct BITSFUNC(fake_sections) *out)
+{
+ if (!out->in_shstrndx)
+ fail("didn't find the fake shstrndx\n");
+
+ memset(out->table, 0, out->max_count * sizeof(ELF(Shdr)));
+
+ if (out->max_count < 1)
+ fail("we need at least two fake output sections\n");
+
+ PUT_LE(&out->table[0].sh_type, SHT_NULL);
+ PUT_LE(&out->table[0].sh_name, BITSFUNC(find_shname)(out, ""));
+
+ out->count = 1;
+}
+
+static void BITSFUNC(copy_section)(struct BITSFUNC(fake_sections) *out,
+ int in_idx, const ELF(Shdr) *in,
+ const char *name)
+{
+ uint64_t flags = GET_LE(&in->sh_flags);
+
+ bool copy = flags & SHF_ALLOC;
+
+ if (!copy)
+ return;
+
+ if (out->count >= out->max_count)
+ fail("too many copied sections (max = %d)\n", out->max_count);
+
+ if (in_idx == out->in_shstrndx)
+ out->out_shstrndx = out->count;
+
+ out->table[out->count] = *in;
+ PUT_LE(&out->table[out->count].sh_name,
+ BITSFUNC(find_shname)(out, name));
+
+ /* elfutils requires that a strtab have the correct type. */
+ if (!strcmp(name, ".fake_shstrtab"))
+ PUT_LE(&out->table[out->count].sh_type, SHT_STRTAB);
+
+ out->count++;
+}
+
static void BITSFUNC(go)(void *addr, size_t len,
FILE *outfile, const char *name)
{
@@ -19,7 +129,7 @@ static void BITSFUNC(go)(void *addr, size_t len,
const char *secstrings;
uint64_t syms[NSYMS] = {};

- uint64_t fake_sections_value = 0, fake_sections_size = 0;
+ struct BITSFUNC(fake_sections) fake_sections = {};

ELF(Phdr) *pt = (ELF(Phdr) *)(addr + GET_LE(&hdr->e_phoff));

@@ -89,23 +199,57 @@ static void BITSFUNC(go)(void *addr, size_t len,
GET_LE(&sym->st_name);

for (k = 0; k < NSYMS; k++) {
- if (!strcmp(name, required_syms[k])) {
+ if (!strcmp(name, required_syms[k].name)) {
if (syms[k]) {
fail("duplicate symbol %s\n",
- required_syms[k]);
+ required_syms[k].name);
}
syms[k] = GET_LE(&sym->st_value);
}
}

- if (!strcmp(name, "vdso_fake_sections")) {
- if (fake_sections_value)
- fail("duplicate vdso_fake_sections\n");
- fake_sections_value = GET_LE(&sym->st_value);
- fake_sections_size = GET_LE(&sym->st_size);
+ if (!strcmp(name, "fake_shstrtab")) {
+ ELF(Shdr) *sh;
+
+ fake_sections.in_shstrndx = GET_LE(&sym->st_shndx);
+ fake_sections.shstrtab = addr + GET_LE(&sym->st_value);
+ fake_sections.shstrtab_len = GET_LE(&sym->st_size);
+ sh = addr + GET_LE(&hdr->e_shoff) +
+ GET_LE(&hdr->e_shentsize) *
+ fake_sections.in_shstrndx;
+ fake_sections.shstr_offset = GET_LE(&sym->st_value) -
+ GET_LE(&sh->sh_addr);
}
}

+ /* Build the output section table. */
+ if (!syms[sym_VDSO_FAKE_SECTION_TABLE_START] ||
+ !syms[sym_VDSO_FAKE_SECTION_TABLE_END])
+ fail("couldn't find fake section table\n");
+ if ((syms[sym_VDSO_FAKE_SECTION_TABLE_END] -
+ syms[sym_VDSO_FAKE_SECTION_TABLE_START]) % sizeof(ELF(Shdr)))
+ fail("fake section table size isn't a multiple of sizeof(Shdr)\n");
+ fake_sections.table = addr + syms[sym_VDSO_FAKE_SECTION_TABLE_START];
+ fake_sections.table_offset = syms[sym_VDSO_FAKE_SECTION_TABLE_START];
+ fake_sections.max_count = (syms[sym_VDSO_FAKE_SECTION_TABLE_END] -
+ syms[sym_VDSO_FAKE_SECTION_TABLE_START]) /
+ sizeof(ELF(Shdr));
+
+ BITSFUNC(init_sections)(&fake_sections);
+ for (i = 0; i < GET_LE(&hdr->e_shnum); i++) {
+ ELF(Shdr) *sh = addr + GET_LE(&hdr->e_shoff) +
+ GET_LE(&hdr->e_shentsize) * i;
+ BITSFUNC(copy_section)(&fake_sections, i, sh,
+ secstrings + GET_LE(&sh->sh_name));
+ }
+ if (!fake_sections.out_shstrndx)
+ fail("didn't generate shstrndx?!?\n");
+
+ PUT_LE(&hdr->e_shoff, fake_sections.table_offset);
+ PUT_LE(&hdr->e_shentsize, sizeof(ELF(Shdr)));
+ PUT_LE(&hdr->e_shnum, fake_sections.count);
+ PUT_LE(&hdr->e_shstrndx, fake_sections.out_shstrndx);
+
/* Validate mapping addresses. */
for (i = 0; i < sizeof(special_pages) / sizeof(special_pages[0]); i++) {
if (!syms[i])
@@ -113,25 +257,17 @@ static void BITSFUNC(go)(void *addr, size_t len,

if (syms[i] % 4096)
fail("%s must be a multiple of 4096\n",
- required_syms[i]);
+ required_syms[i].name);
if (syms[i] < data_size)
fail("%s must be after the text mapping\n",
- required_syms[i]);
+ required_syms[i].name);
if (syms[sym_end_mapping] < syms[i] + 4096)
- fail("%s overruns end_mapping\n", required_syms[i]);
+ fail("%s overruns end_mapping\n",
+ required_syms[i].name);
}
if (syms[sym_end_mapping] % 4096)
fail("end_mapping must be a multiple of 4096\n");

- /* Remove sections or use fakes */
- if (fake_sections_size % sizeof(ELF(Shdr)))
- fail("vdso_fake_sections size is not a multiple of %ld\n",
- (long)sizeof(ELF(Shdr)));
- PUT_LE(&hdr->e_shoff, fake_sections_value);
- PUT_LE(&hdr->e_shentsize, fake_sections_value ? sizeof(ELF(Shdr)) : 0);
- PUT_LE(&hdr->e_shnum, fake_sections_size / sizeof(ELF(Shdr)));
- PUT_LE(&hdr->e_shstrndx, SHN_UNDEF);
-
if (!name) {
fwrite(addr, load_size, 1, outfile);
return;
@@ -169,9 +305,9 @@ static void BITSFUNC(go)(void *addr, size_t len,
(unsigned long)GET_LE(&alt_sec->sh_size));
}
for (i = 0; i < NSYMS; i++) {
- if (syms[i])
+ if (required_syms[i].export && syms[i])
fprintf(outfile, "\t.sym_%s = 0x%" PRIx64 ",\n",
- required_syms[i], syms[i]);
+ required_syms[i].name, syms[i]);
}
fprintf(outfile, "};\n");
}
diff --git a/arch/x86/vdso/vdso32/vdso-fakesections.c b/arch/x86/vdso/vdso32/vdso-fakesections.c
new file mode 100644
index 0000000..541468e
--- /dev/null
+++ b/arch/x86/vdso/vdso32/vdso-fakesections.c
@@ -0,0 +1 @@
+#include "../vdso-fakesections.c"
diff --git a/arch/x86/vdso/vdsox32.lds.S b/arch/x86/vdso/vdsox32.lds.S
index 46b991b..697c11e 100644
--- a/arch/x86/vdso/vdsox32.lds.S
+++ b/arch/x86/vdso/vdsox32.lds.S
@@ -6,6 +6,8 @@
* the DSO.
*/

+#define BUILD_VDSOX32
+
#include "vdso-layout.lds.S"

/*
--
1.9.3

2014-06-18 23:00:30

by Andy Lutomirski

[permalink] [raw]
Subject: [PATCH 5/5] x86,vdso: Create .build-id links for unstripped vdso files

With this change, doing 'make vdso_install' and telling gdb:

set debug-file-directory /lib/modules/KVER/vdso

will enable vdso debugging with symbols. This is useful for
testing, but kernel RPM builds will probably want to manually delete
these symlinks or otherwise do something sensible when they strip
the vdso/*.so files.

This may break make vdso_install with binutils before 2.17.50.0.17.
On the other hand, make vdso_install was probably never useful with
earlier binutils, since the installed files are AFAIK completely
useless without build-id.

Signed-off-by: Andy Lutomirski <[email protected]>
---
arch/x86/vdso/Makefile | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/vdso/Makefile b/arch/x86/vdso/Makefile
index 2c1ca98..ac16c17 100644
--- a/arch/x86/vdso/Makefile
+++ b/arch/x86/vdso/Makefile
@@ -176,7 +176,14 @@ GCOV_PROFILE := n
# Install the unstripped copies of vdso*.so.
#
quiet_cmd_vdso_install = INSTALL $(@:install_%=%)
- cmd_vdso_install = cp $< $(MODLIB)/vdso/$(@:install_%=%)
+define cmd_vdso_install
+ buildid=`readelf -n $< |grep 'Build ID' |sed -e 's/^.*Build ID: \(.*\)$$/\1/'`; \
+ first=`echo $$buildid | cut -b-2`; \
+ last=`echo $$buildid | cut -b3-`; \
+ cp $< "$(MODLIB)/vdso/$(@:install_%=%)"; \
+ mkdir -p "$(MODLIB)/vdso/.build-id/$$first"; \
+ ln -sf "../../$(@:install_%=%)" "$(MODLIB)/vdso/.build-id/$$first/$$last.debug"
+endef

vdso_img_insttargets := $(vdso_img_sodbg:%.dbg=install_%)

--
1.9.3

2014-06-18 23:00:58

by Andy Lutomirski

[permalink] [raw]
Subject: [PATCH 4/5] x86,vdso: Remove some redundant in-memory section headers

.data doesn't need to be separate from .rodata: they're both readonly.

.altinstructions and .altinstr_replacement aren't needed by anything
except vdso2c; strip them from the final image.

While we're at it, rather than aligning the actual executable text,
just shove some unused-at-runtime data in between real data and
text.

My vdso image is still above 4k, but I'm disinclined to try to
trim it harder for 3.16. For future trimming, I suspect that these
sections could be moved to later in the file and dropped from
the in-memory image:

.gnu.version and .gnu.version_d (this may lose versions in gdb)
.eh_frame (should be harmless)
.eh_frame_hdr (I'm not really sure)
.hash (AFAIK nothing needs this section header)

Signed-off-by: Andy Lutomirski <[email protected]>
---
arch/x86/vdso/vdso-fakesections.c | 3 ---
arch/x86/vdso/vdso-layout.lds.S | 43 +++++++++++++++++++++------------------
arch/x86/vdso/vdso2c.h | 4 +++-
3 files changed, 26 insertions(+), 24 deletions(-)

diff --git a/arch/x86/vdso/vdso-fakesections.c b/arch/x86/vdso/vdso-fakesections.c
index 56927a7..aa5fbfa 100644
--- a/arch/x86/vdso/vdso-fakesections.c
+++ b/arch/x86/vdso/vdso-fakesections.c
@@ -16,9 +16,6 @@ const char fake_shstrtab[] __attribute__((section(".fake_shstrtab"))) =
".rodata\0"
".fake_shstrtab\0" /* Yay, self-referential code. */
".note\0"
- ".data\0"
- ".altinstructions\0"
- ".altinstr_replacement\0"
".eh_frame_hdr\0"
".eh_frame\0"
".text";
diff --git a/arch/x86/vdso/vdso-layout.lds.S b/arch/x86/vdso/vdso-layout.lds.S
index e4cbc21..9197544 100644
--- a/arch/x86/vdso/vdso-layout.lds.S
+++ b/arch/x86/vdso/vdso-layout.lds.S
@@ -14,7 +14,7 @@
# error unknown VDSO target
#endif

-#define NUM_FAKE_SHDRS 16
+#define NUM_FAKE_SHDRS 13

SECTIONS
{
@@ -28,15 +28,17 @@ SECTIONS
.gnu.version_d : { *(.gnu.version_d) }
.gnu.version_r : { *(.gnu.version_r) }

- .note : { *(.note.*) } :text :note
-
- .eh_frame_hdr : { *(.eh_frame_hdr) } :text :eh_frame_hdr
- .eh_frame : { KEEP (*(.eh_frame)) } :text
-
.dynamic : { *(.dynamic) } :text :dynamic

.rodata : {
*(.rodata*)
+ *(.data*)
+ *(.sdata*)
+ *(.got.plt) *(.got)
+ *(.gnu.linkonce.d.*)
+ *(.bss*)
+ *(.dynbss*)
+ *(.gnu.linkonce.b.*)

/*
* Ideally this would live in a C file, but that won't
@@ -50,28 +52,29 @@ SECTIONS

.fake_shstrtab : { *(.fake_shstrtab) } :text

- .data : {
- *(.data*)
- *(.sdata*)
- *(.got.plt) *(.got)
- *(.gnu.linkonce.d.*)
- *(.bss*)
- *(.dynbss*)
- *(.gnu.linkonce.b.*)
- }

- .altinstructions : { *(.altinstructions) }
- .altinstr_replacement : { *(.altinstr_replacement) }
+ .note : { *(.note.*) } :text :note
+
+ .eh_frame_hdr : { *(.eh_frame_hdr) } :text :eh_frame_hdr
+ .eh_frame : { KEEP (*(.eh_frame)) } :text
+

/*
- * Align the actual code well away from the non-instruction data.
- * This is the best thing for the I-cache.
+ * Text is well-separated from actual data: there's plenty of
+ * stuff that isn't used at runtime in between.
*/
- . = ALIGN(0x100);

.text : { *(.text*) } :text =0x90909090,

/*
+ * At the end so that eu-elflint stays happy when vdso2c strips
+ * these. A better implementation would avoid allocating space
+ * for these.
+ */
+ .altinstructions : { *(.altinstructions) } :text
+ .altinstr_replacement : { *(.altinstr_replacement) } :text
+
+ /*
* The remainder of the vDSO consists of special pages that are
* shared between the kernel and userspace. It needs to be at the
* end so that it doesn't overlap the mapping of the actual
diff --git a/arch/x86/vdso/vdso2c.h b/arch/x86/vdso/vdso2c.h
index f01ed4b..f42e2dd 100644
--- a/arch/x86/vdso/vdso2c.h
+++ b/arch/x86/vdso/vdso2c.h
@@ -92,7 +92,9 @@ static void BITSFUNC(copy_section)(struct BITSFUNC(fake_sections) *out,
{
uint64_t flags = GET_LE(&in->sh_flags);

- bool copy = flags & SHF_ALLOC;
+ bool copy = flags & SHF_ALLOC &&
+ strcmp(name, ".altinstructions") &&
+ strcmp(name, ".altinstr_replacement");

if (!copy)
return;
--
1.9.3

2014-06-18 23:01:43

by Andy Lutomirski

[permalink] [raw]
Subject: [PATCH 2/5] x86,vdso2c: Use better macros for ELF bitness

Rather than using a separate macro for each replacement, use generic
macros.

Signed-off-by: Andy Lutomirski <[email protected]>
---
arch/x86/vdso/vdso2c.c | 42 +++++++++++++-----------------------------
arch/x86/vdso/vdso2c.h | 23 ++++++++++++-----------
2 files changed, 25 insertions(+), 40 deletions(-)

diff --git a/arch/x86/vdso/vdso2c.c b/arch/x86/vdso/vdso2c.c
index 7a6bf50..7343899 100644
--- a/arch/x86/vdso/vdso2c.c
+++ b/arch/x86/vdso/vdso2c.c
@@ -83,37 +83,21 @@ extern void bad_put_le(void);

#define NSYMS (sizeof(required_syms) / sizeof(required_syms[0]))

-#define BITS 64
-#define GOFUNC go64
-#define Elf_Ehdr Elf64_Ehdr
-#define Elf_Shdr Elf64_Shdr
-#define Elf_Phdr Elf64_Phdr
-#define Elf_Sym Elf64_Sym
-#define Elf_Dyn Elf64_Dyn
+#define BITSFUNC3(name, bits) name##bits
+#define BITSFUNC2(name, bits) BITSFUNC3(name, bits)
+#define BITSFUNC(name) BITSFUNC2(name, ELF_BITS)
+
+#define ELF_BITS_XFORM2(bits, x) Elf##bits##_##x
+#define ELF_BITS_XFORM(bits, x) ELF_BITS_XFORM2(bits, x)
+#define ELF(x) ELF_BITS_XFORM(ELF_BITS, x)
+
+#define ELF_BITS 64
#include "vdso2c.h"
-#undef BITS
-#undef GOFUNC
-#undef Elf_Ehdr
-#undef Elf_Shdr
-#undef Elf_Phdr
-#undef Elf_Sym
-#undef Elf_Dyn
-
-#define BITS 32
-#define GOFUNC go32
-#define Elf_Ehdr Elf32_Ehdr
-#define Elf_Shdr Elf32_Shdr
-#define Elf_Phdr Elf32_Phdr
-#define Elf_Sym Elf32_Sym
-#define Elf_Dyn Elf32_Dyn
+#undef ELF_BITS
+
+#define ELF_BITS 32
#include "vdso2c.h"
-#undef BITS
-#undef GOFUNC
-#undef Elf_Ehdr
-#undef Elf_Shdr
-#undef Elf_Phdr
-#undef Elf_Sym
-#undef Elf_Dyn
+#undef ELF_BITS

static void go(void *addr, size_t len, FILE *outfile, const char *name)
{
diff --git a/arch/x86/vdso/vdso2c.h b/arch/x86/vdso/vdso2c.h
index c6eefaf..8e185ce 100644
--- a/arch/x86/vdso/vdso2c.h
+++ b/arch/x86/vdso/vdso2c.h
@@ -4,23 +4,24 @@
* are built for 32-bit userspace.
*/

-static void GOFUNC(void *addr, size_t len, FILE *outfile, const char *name)
+static void BITSFUNC(go)(void *addr, size_t len,
+ FILE *outfile, const char *name)
{
int found_load = 0;
unsigned long load_size = -1; /* Work around bogus warning */
unsigned long data_size;
- Elf_Ehdr *hdr = (Elf_Ehdr *)addr;
+ ELF(Ehdr) *hdr = (ELF(Ehdr) *)addr;
int i;
unsigned long j;
- Elf_Shdr *symtab_hdr = NULL, *strtab_hdr, *secstrings_hdr,
+ ELF(Shdr) *symtab_hdr = NULL, *strtab_hdr, *secstrings_hdr,
*alt_sec = NULL;
- Elf_Dyn *dyn = 0, *dyn_end = 0;
+ ELF(Dyn) *dyn = 0, *dyn_end = 0;
const char *secstrings;
uint64_t syms[NSYMS] = {};

uint64_t fake_sections_value = 0, fake_sections_size = 0;

- Elf_Phdr *pt = (Elf_Phdr *)(addr + GET_LE(&hdr->e_phoff));
+ ELF(Phdr) *pt = (ELF(Phdr) *)(addr + GET_LE(&hdr->e_phoff));

/* Walk the segment table. */
for (i = 0; i < GET_LE(&hdr->e_phnum); i++) {
@@ -61,7 +62,7 @@ static void GOFUNC(void *addr, size_t len, FILE *outfile, const char *name)
GET_LE(&hdr->e_shentsize)*GET_LE(&hdr->e_shstrndx);
secstrings = addr + GET_LE(&secstrings_hdr->sh_offset);
for (i = 0; i < GET_LE(&hdr->e_shnum); i++) {
- Elf_Shdr *sh = addr + GET_LE(&hdr->e_shoff) +
+ ELF(Shdr) *sh = addr + GET_LE(&hdr->e_shoff) +
GET_LE(&hdr->e_shentsize) * i;
if (GET_LE(&sh->sh_type) == SHT_SYMTAB)
symtab_hdr = sh;
@@ -82,7 +83,7 @@ static void GOFUNC(void *addr, size_t len, FILE *outfile, const char *name)
i < GET_LE(&symtab_hdr->sh_size) / GET_LE(&symtab_hdr->sh_entsize);
i++) {
int k;
- Elf_Sym *sym = addr + GET_LE(&symtab_hdr->sh_offset) +
+ ELF(Sym) *sym = addr + GET_LE(&symtab_hdr->sh_offset) +
GET_LE(&symtab_hdr->sh_entsize) * i;
const char *name = addr + GET_LE(&strtab_hdr->sh_offset) +
GET_LE(&sym->st_name);
@@ -123,12 +124,12 @@ static void GOFUNC(void *addr, size_t len, FILE *outfile, const char *name)
fail("end_mapping must be a multiple of 4096\n");

/* Remove sections or use fakes */
- if (fake_sections_size % sizeof(Elf_Shdr))
+ if (fake_sections_size % sizeof(ELF(Shdr)))
fail("vdso_fake_sections size is not a multiple of %ld\n",
- (long)sizeof(Elf_Shdr));
+ (long)sizeof(ELF(Shdr)));
PUT_LE(&hdr->e_shoff, fake_sections_value);
- PUT_LE(&hdr->e_shentsize, fake_sections_value ? sizeof(Elf_Shdr) : 0);
- PUT_LE(&hdr->e_shnum, fake_sections_size / sizeof(Elf_Shdr));
+ PUT_LE(&hdr->e_shentsize, fake_sections_value ? sizeof(ELF(Shdr)) : 0);
+ PUT_LE(&hdr->e_shnum, fake_sections_size / sizeof(ELF(Shdr)));
PUT_LE(&hdr->e_shstrndx, SHN_UNDEF);

if (!name) {
--
1.9.3

2014-06-19 22:42:25

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH 0/5] x86,vdso: Restore a bunch of section headers

On 06/18/2014 03:59 PM, Andy Lutomirski wrote:
> This series makes me sad. It brings the 64-bit vdso back above 4kB,
> like it was in 3.15. It's also just silly, but it seems to be
> needed to keep binutils happy when debugging the vdso.

Oh well... it is only a single page across the entire kernel... there
are worse things in the world.

-hpa

2014-06-19 22:46:19

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH 5/5] x86,vdso: Create .build-id links for unstripped vdso files

On 06/18/2014 03:59 PM, Andy Lutomirski wrote:
>
> This may break make vdso_install with binutils before 2.17.50.0.17.
> On the other hand, make vdso_install was probably never useful with
> earlier binutils, since the installed files are AFAIK completely
> useless without build-id.
>

How does it fail?

-hpa

2014-06-20 00:00:17

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH 5/5] x86,vdso: Create .build-id links for unstripped vdso files

On Thu, Jun 19, 2014 at 3:46 PM, H. Peter Anvin <[email protected]> wrote:
> On 06/18/2014 03:59 PM, Andy Lutomirski wrote:
>>
>> This may break make vdso_install with binutils before 2.17.50.0.17.
>> On the other hand, make vdso_install was probably never useful with
>> earlier binutils, since the installed files are AFAIK completely
>> useless without build-id.
>>
>
> How does it fail?

If --build-id=none on modern binutils accurately replicates old
binutils, it fails by silently creating a broken symlink called
.debug. If you want this patch for 3.16, I can send a better version
that just doesn't create the links on old binutils.

>
> -hpa
>
>



--
Andy Lutomirski
AMA Capital Management, LLC

Subject: [tip:x86/urgent] x86/vdso: Discard the __bug_table section

Commit-ID: 5f56e7167e6d438324fcba87018255d81e201383
Gitweb: http://git.kernel.org/tip/5f56e7167e6d438324fcba87018255d81e201383
Author: Andy Lutomirski <[email protected]>
AuthorDate: Wed, 18 Jun 2014 15:59:46 -0700
Committer: H. Peter Anvin <[email protected]>
CommitDate: Thu, 19 Jun 2014 15:44:51 -0700

x86/vdso: Discard the __bug_table section

It serves no purpose in user code.

Signed-off-by: Andy Lutomirski <[email protected]>
Link: http://lkml.kernel.org/r/2a5bebff42defd8a5e81d96f7dc00f21143c80e8.1403129369.git.luto@amacapital.net
Signed-off-by: H. Peter Anvin <[email protected]>
---
arch/x86/vdso/vdso-layout.lds.S | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/x86/vdso/vdso-layout.lds.S b/arch/x86/vdso/vdso-layout.lds.S
index 2ec72f6..c84166c 100644
--- a/arch/x86/vdso/vdso-layout.lds.S
+++ b/arch/x86/vdso/vdso-layout.lds.S
@@ -75,6 +75,7 @@ SECTIONS
/DISCARD/ : {
*(.discard)
*(.discard.*)
+ *(__bug_table)
}
}

Subject: [tip:x86/urgent] x86/vdso2c: Use better macros for ELF bitness

Commit-ID: c1979c370273fd9f7326ffa27a63b9ddb0f495f4
Gitweb: http://git.kernel.org/tip/c1979c370273fd9f7326ffa27a63b9ddb0f495f4
Author: Andy Lutomirski <[email protected]>
AuthorDate: Wed, 18 Jun 2014 15:59:47 -0700
Committer: H. Peter Anvin <[email protected]>
CommitDate: Thu, 19 Jun 2014 15:44:59 -0700

x86/vdso2c: Use better macros for ELF bitness

Rather than using a separate macro for each replacement, use generic
macros.

Signed-off-by: Andy Lutomirski <[email protected]>
Link: http://lkml.kernel.org/r/d953cd2e70ceee1400985d091188cdd65fba2f05.1403129369.git.luto@amacapital.net
Signed-off-by: H. Peter Anvin <[email protected]>
---
arch/x86/vdso/vdso2c.c | 42 +++++++++++++-----------------------------
arch/x86/vdso/vdso2c.h | 23 ++++++++++++-----------
2 files changed, 25 insertions(+), 40 deletions(-)

diff --git a/arch/x86/vdso/vdso2c.c b/arch/x86/vdso/vdso2c.c
index 7a6bf50..7343899 100644
--- a/arch/x86/vdso/vdso2c.c
+++ b/arch/x86/vdso/vdso2c.c
@@ -83,37 +83,21 @@ extern void bad_put_le(void);

#define NSYMS (sizeof(required_syms) / sizeof(required_syms[0]))

-#define BITS 64
-#define GOFUNC go64
-#define Elf_Ehdr Elf64_Ehdr
-#define Elf_Shdr Elf64_Shdr
-#define Elf_Phdr Elf64_Phdr
-#define Elf_Sym Elf64_Sym
-#define Elf_Dyn Elf64_Dyn
+#define BITSFUNC3(name, bits) name##bits
+#define BITSFUNC2(name, bits) BITSFUNC3(name, bits)
+#define BITSFUNC(name) BITSFUNC2(name, ELF_BITS)
+
+#define ELF_BITS_XFORM2(bits, x) Elf##bits##_##x
+#define ELF_BITS_XFORM(bits, x) ELF_BITS_XFORM2(bits, x)
+#define ELF(x) ELF_BITS_XFORM(ELF_BITS, x)
+
+#define ELF_BITS 64
#include "vdso2c.h"
-#undef BITS
-#undef GOFUNC
-#undef Elf_Ehdr
-#undef Elf_Shdr
-#undef Elf_Phdr
-#undef Elf_Sym
-#undef Elf_Dyn
-
-#define BITS 32
-#define GOFUNC go32
-#define Elf_Ehdr Elf32_Ehdr
-#define Elf_Shdr Elf32_Shdr
-#define Elf_Phdr Elf32_Phdr
-#define Elf_Sym Elf32_Sym
-#define Elf_Dyn Elf32_Dyn
+#undef ELF_BITS
+
+#define ELF_BITS 32
#include "vdso2c.h"
-#undef BITS
-#undef GOFUNC
-#undef Elf_Ehdr
-#undef Elf_Shdr
-#undef Elf_Phdr
-#undef Elf_Sym
-#undef Elf_Dyn
+#undef ELF_BITS

static void go(void *addr, size_t len, FILE *outfile, const char *name)
{
diff --git a/arch/x86/vdso/vdso2c.h b/arch/x86/vdso/vdso2c.h
index c6eefaf..8e185ce 100644
--- a/arch/x86/vdso/vdso2c.h
+++ b/arch/x86/vdso/vdso2c.h
@@ -4,23 +4,24 @@
* are built for 32-bit userspace.
*/

-static void GOFUNC(void *addr, size_t len, FILE *outfile, const char *name)
+static void BITSFUNC(go)(void *addr, size_t len,
+ FILE *outfile, const char *name)
{
int found_load = 0;
unsigned long load_size = -1; /* Work around bogus warning */
unsigned long data_size;
- Elf_Ehdr *hdr = (Elf_Ehdr *)addr;
+ ELF(Ehdr) *hdr = (ELF(Ehdr) *)addr;
int i;
unsigned long j;
- Elf_Shdr *symtab_hdr = NULL, *strtab_hdr, *secstrings_hdr,
+ ELF(Shdr) *symtab_hdr = NULL, *strtab_hdr, *secstrings_hdr,
*alt_sec = NULL;
- Elf_Dyn *dyn = 0, *dyn_end = 0;
+ ELF(Dyn) *dyn = 0, *dyn_end = 0;
const char *secstrings;
uint64_t syms[NSYMS] = {};

uint64_t fake_sections_value = 0, fake_sections_size = 0;

- Elf_Phdr *pt = (Elf_Phdr *)(addr + GET_LE(&hdr->e_phoff));
+ ELF(Phdr) *pt = (ELF(Phdr) *)(addr + GET_LE(&hdr->e_phoff));

/* Walk the segment table. */
for (i = 0; i < GET_LE(&hdr->e_phnum); i++) {
@@ -61,7 +62,7 @@ static void GOFUNC(void *addr, size_t len, FILE *outfile, const char *name)
GET_LE(&hdr->e_shentsize)*GET_LE(&hdr->e_shstrndx);
secstrings = addr + GET_LE(&secstrings_hdr->sh_offset);
for (i = 0; i < GET_LE(&hdr->e_shnum); i++) {
- Elf_Shdr *sh = addr + GET_LE(&hdr->e_shoff) +
+ ELF(Shdr) *sh = addr + GET_LE(&hdr->e_shoff) +
GET_LE(&hdr->e_shentsize) * i;
if (GET_LE(&sh->sh_type) == SHT_SYMTAB)
symtab_hdr = sh;
@@ -82,7 +83,7 @@ static void GOFUNC(void *addr, size_t len, FILE *outfile, const char *name)
i < GET_LE(&symtab_hdr->sh_size) / GET_LE(&symtab_hdr->sh_entsize);
i++) {
int k;
- Elf_Sym *sym = addr + GET_LE(&symtab_hdr->sh_offset) +
+ ELF(Sym) *sym = addr + GET_LE(&symtab_hdr->sh_offset) +
GET_LE(&symtab_hdr->sh_entsize) * i;
const char *name = addr + GET_LE(&strtab_hdr->sh_offset) +
GET_LE(&sym->st_name);
@@ -123,12 +124,12 @@ static void GOFUNC(void *addr, size_t len, FILE *outfile, const char *name)
fail("end_mapping must be a multiple of 4096\n");

/* Remove sections or use fakes */
- if (fake_sections_size % sizeof(Elf_Shdr))
+ if (fake_sections_size % sizeof(ELF(Shdr)))
fail("vdso_fake_sections size is not a multiple of %ld\n",
- (long)sizeof(Elf_Shdr));
+ (long)sizeof(ELF(Shdr)));
PUT_LE(&hdr->e_shoff, fake_sections_value);
- PUT_LE(&hdr->e_shentsize, fake_sections_value ? sizeof(Elf_Shdr) : 0);
- PUT_LE(&hdr->e_shnum, fake_sections_size / sizeof(Elf_Shdr));
+ PUT_LE(&hdr->e_shentsize, fake_sections_value ? sizeof(ELF(Shdr)) : 0);
+ PUT_LE(&hdr->e_shnum, fake_sections_size / sizeof(ELF(Shdr)));
PUT_LE(&hdr->e_shstrndx, SHN_UNDEF);

if (!name) {

Subject: [tip:x86/urgent] x86/vdso: Improve the fake section headers

Commit-ID: bfad381c0d1e19cae8461e105d8d4387dd2a14fe
Gitweb: http://git.kernel.org/tip/bfad381c0d1e19cae8461e105d8d4387dd2a14fe
Author: Andy Lutomirski <[email protected]>
AuthorDate: Wed, 18 Jun 2014 15:59:48 -0700
Committer: H. Peter Anvin <[email protected]>
CommitDate: Thu, 19 Jun 2014 15:45:12 -0700

x86/vdso: Improve the fake section headers

Fully stripping the vDSO has other unfortunate side effects:

- binutils is unable to find ELF notes without a SHT_NOTE section.

- Even elfutils has trouble: it can find ELF notes without a section
table at all, but if a section table is present, it won't look for
PT_NOTE.

- gdb wants section names to match between stripped DSOs and their
symbols; otherwise it will corrupt symbol addresses.

We're also breaking the rules: section 0 is supposed to be SHT_NULL.

Fix these problems by building a better fake section table. While
we're at it, we might as well let buggy Go versions keep working well
by giving the SHT_DYNSYM entry the correct size.

This is a bit unfortunate: it adds quite a bit of size to the vdso
image.

If/when binutils improves and the improved versions become widespread,
it would be worth considering dropping most of this.

Signed-off-by: Andy Lutomirski <[email protected]>
Link: http://lkml.kernel.org/r/0e546a5eeaafdf1840e6ee654a55c1e727c26663.1403129369.git.luto@amacapital.net
Signed-off-by: H. Peter Anvin <[email protected]>
---
arch/x86/vdso/Makefile | 4 +-
arch/x86/vdso/vdso-fakesections.c | 44 ++++----
arch/x86/vdso/vdso-layout.lds.S | 40 +++++--
arch/x86/vdso/vdso.lds.S | 2 +
arch/x86/vdso/vdso2c.c | 31 ++++--
arch/x86/vdso/vdso2c.h | 180 +++++++++++++++++++++++++++----
arch/x86/vdso/vdso32/vdso-fakesections.c | 1 +
arch/x86/vdso/vdsox32.lds.S | 2 +
8 files changed, 237 insertions(+), 67 deletions(-)

diff --git a/arch/x86/vdso/Makefile b/arch/x86/vdso/Makefile
index 3c0809a..2c1ca98 100644
--- a/arch/x86/vdso/Makefile
+++ b/arch/x86/vdso/Makefile
@@ -11,7 +11,6 @@ VDSO32-$(CONFIG_COMPAT) := y

# files to link into the vdso
vobjs-y := vdso-note.o vclock_gettime.o vgetcpu.o vdso-fakesections.o
-vobjs-nox32 := vdso-fakesections.o

# files to link into kernel
obj-y += vma.o
@@ -134,7 +133,7 @@ override obj-dirs = $(dir $(obj)) $(obj)/vdso32/

targets += vdso32/vdso32.lds
targets += vdso32/note.o vdso32/vclock_gettime.o $(vdso32.so-y:%=vdso32/%.o)
-targets += vdso32/vclock_gettime.o
+targets += vdso32/vclock_gettime.o vdso32/vdso-fakesections.o

$(obj)/vdso32.o: $(vdso32-images:%=$(obj)/%)

@@ -155,6 +154,7 @@ $(vdso32-images:%=$(obj)/%.dbg): KBUILD_CFLAGS = $(KBUILD_CFLAGS_32)
$(vdso32-images:%=$(obj)/%.dbg): $(obj)/vdso32-%.so.dbg: FORCE \
$(obj)/vdso32/vdso32.lds \
$(obj)/vdso32/vclock_gettime.o \
+ $(obj)/vdso32/vdso-fakesections.o \
$(obj)/vdso32/note.o \
$(obj)/vdso32/%.o
$(call if_changed,vdso)
diff --git a/arch/x86/vdso/vdso-fakesections.c b/arch/x86/vdso/vdso-fakesections.c
index cb8a8d7..56927a7 100644
--- a/arch/x86/vdso/vdso-fakesections.c
+++ b/arch/x86/vdso/vdso-fakesections.c
@@ -2,31 +2,23 @@
* Copyright 2014 Andy Lutomirski
* Subject to the GNU Public License, v.2
*
- * Hack to keep broken Go programs working.
- *
- * The Go runtime had a couple of bugs: it would read the section table to try
- * to figure out how many dynamic symbols there were (it shouldn't have looked
- * at the section table at all) and, if there were no SHT_SYNDYM section table
- * entry, it would use an uninitialized value for the number of symbols. As a
- * workaround, we supply a minimal section table. vdso2c will adjust the
- * in-memory image so that "vdso_fake_sections" becomes the section table.
- *
- * The bug was introduced by:
- * https://code.google.com/p/go/source/detail?r=56ea40aac72b (2012-08-31)
- * and is being addressed in the Go runtime in this issue:
- * https://code.google.com/p/go/issues/detail?id=8197
+ * String table for loadable section headers. See vdso2c.h for why
+ * this exists.
*/

-#ifndef __x86_64__
-#error This hack is specific to the 64-bit vDSO
-#endif
-
-#include <linux/elf.h>
-
-extern const __visible struct elf64_shdr vdso_fake_sections[];
-const __visible struct elf64_shdr vdso_fake_sections[] = {
- {
- .sh_type = SHT_DYNSYM,
- .sh_entsize = sizeof(Elf64_Sym),
- }
-};
+const char fake_shstrtab[] __attribute__((section(".fake_shstrtab"))) =
+ ".hash\0"
+ ".dynsym\0"
+ ".dynstr\0"
+ ".gnu.version\0"
+ ".gnu.version_d\0"
+ ".dynamic\0"
+ ".rodata\0"
+ ".fake_shstrtab\0" /* Yay, self-referential code. */
+ ".note\0"
+ ".data\0"
+ ".altinstructions\0"
+ ".altinstr_replacement\0"
+ ".eh_frame_hdr\0"
+ ".eh_frame\0"
+ ".text";
diff --git a/arch/x86/vdso/vdso-layout.lds.S b/arch/x86/vdso/vdso-layout.lds.S
index c84166c..e4cbc21 100644
--- a/arch/x86/vdso/vdso-layout.lds.S
+++ b/arch/x86/vdso/vdso-layout.lds.S
@@ -6,6 +6,16 @@
* This script controls its layout.
*/

+#if defined(BUILD_VDSO64)
+# define SHDR_SIZE 64
+#elif defined(BUILD_VDSO32) || defined(BUILD_VDSOX32)
+# define SHDR_SIZE 40
+#else
+# error unknown VDSO target
+#endif
+
+#define NUM_FAKE_SHDRS 16
+
SECTIONS
{
. = SIZEOF_HEADERS;
@@ -25,15 +35,29 @@ SECTIONS

.dynamic : { *(.dynamic) } :text :dynamic

- .rodata : { *(.rodata*) } :text
+ .rodata : {
+ *(.rodata*)
+
+ /*
+ * Ideally this would live in a C file, but that won't
+ * work cleanly for x32 until we start building the x32
+ * C code using an x32 toolchain.
+ */
+ VDSO_FAKE_SECTION_TABLE_START = .;
+ . = . + NUM_FAKE_SHDRS * SHDR_SIZE;
+ VDSO_FAKE_SECTION_TABLE_END = .;
+ } :text
+
+ .fake_shstrtab : { *(.fake_shstrtab) } :text
+
.data : {
- *(.data*)
- *(.sdata*)
- *(.got.plt) *(.got)
- *(.gnu.linkonce.d.*)
- *(.bss*)
- *(.dynbss*)
- *(.gnu.linkonce.b.*)
+ *(.data*)
+ *(.sdata*)
+ *(.got.plt) *(.got)
+ *(.gnu.linkonce.d.*)
+ *(.bss*)
+ *(.dynbss*)
+ *(.gnu.linkonce.b.*)
}

.altinstructions : { *(.altinstructions) }
diff --git a/arch/x86/vdso/vdso.lds.S b/arch/x86/vdso/vdso.lds.S
index 75e3404..6807932 100644
--- a/arch/x86/vdso/vdso.lds.S
+++ b/arch/x86/vdso/vdso.lds.S
@@ -6,6 +6,8 @@
* the DSO.
*/

+#define BUILD_VDSO64
+
#include "vdso-layout.lds.S"

/*
diff --git a/arch/x86/vdso/vdso2c.c b/arch/x86/vdso/vdso2c.c
index 7343899..238dbe82 100644
--- a/arch/x86/vdso/vdso2c.c
+++ b/arch/x86/vdso/vdso2c.c
@@ -23,6 +23,8 @@ enum {
sym_vvar_page,
sym_hpet_page,
sym_end_mapping,
+ sym_VDSO_FAKE_SECTION_TABLE_START,
+ sym_VDSO_FAKE_SECTION_TABLE_END,
};

const int special_pages[] = {
@@ -30,15 +32,26 @@ const int special_pages[] = {
sym_hpet_page,
};

-char const * const required_syms[] = {
- [sym_vvar_page] = "vvar_page",
- [sym_hpet_page] = "hpet_page",
- [sym_end_mapping] = "end_mapping",
- "VDSO32_NOTE_MASK",
- "VDSO32_SYSENTER_RETURN",
- "__kernel_vsyscall",
- "__kernel_sigreturn",
- "__kernel_rt_sigreturn",
+struct vdso_sym {
+ const char *name;
+ bool export;
+};
+
+struct vdso_sym required_syms[] = {
+ [sym_vvar_page] = {"vvar_page", true},
+ [sym_hpet_page] = {"hpet_page", true},
+ [sym_end_mapping] = {"end_mapping", true},
+ [sym_VDSO_FAKE_SECTION_TABLE_START] = {
+ "VDSO_FAKE_SECTION_TABLE_START", false
+ },
+ [sym_VDSO_FAKE_SECTION_TABLE_END] = {
+ "VDSO_FAKE_SECTION_TABLE_END", false
+ },
+ {"VDSO32_NOTE_MASK", true},
+ {"VDSO32_SYSENTER_RETURN", true},
+ {"__kernel_vsyscall", true},
+ {"__kernel_sigreturn", true},
+ {"__kernel_rt_sigreturn", true},
};

__attribute__((format(printf, 1, 2))) __attribute__((noreturn))
diff --git a/arch/x86/vdso/vdso2c.h b/arch/x86/vdso/vdso2c.h
index 8e185ce..f01ed4b 100644
--- a/arch/x86/vdso/vdso2c.h
+++ b/arch/x86/vdso/vdso2c.h
@@ -4,6 +4,116 @@
* are built for 32-bit userspace.
*/

+/*
+ * We're writing a section table for a few reasons:
+ *
+ * The Go runtime had a couple of bugs: it would read the section
+ * table to try to figure out how many dynamic symbols there were (it
+ * shouldn't have looked at the section table at all) and, if there
+ * were no SHT_SYNDYM section table entry, it would use an
+ * uninitialized value for the number of symbols. An empty DYNSYM
+ * table would work, but I see no reason not to write a valid one (and
+ * keep full performance for old Go programs). This hack is only
+ * needed on x86_64.
+ *
+ * The bug was introduced on 2012-08-31 by:
+ * https://code.google.com/p/go/source/detail?r=56ea40aac72b
+ * and was fixed on 2014-06-13 by:
+ * https://code.google.com/p/go/source/detail?r=fc1cd5e12595
+ *
+ * Binutils has issues debugging the vDSO: it reads the section table to
+ * find SHT_NOTE; it won't look at PT_NOTE for the in-memory vDSO, which
+ * would break build-id if we removed the section table. Binutils
+ * also requires that shstrndx != 0. See:
+ * https://sourceware.org/bugzilla/show_bug.cgi?id=17064
+ *
+ * elfutils might not look for PT_NOTE if there is a section table at
+ * all. I don't know whether this matters for any practical purpose.
+ *
+ * For simplicity, rather than hacking up a partial section table, we
+ * just write a mostly complete one. We omit non-dynamic symbols,
+ * though, since they're rather large.
+ *
+ * Once binutils gets fixed, we might be able to drop this for all but
+ * the 64-bit vdso, since build-id only works in kernel RPMs, and
+ * systems that update to new enough kernel RPMs will likely update
+ * binutils in sync. build-id has never worked for home-built kernel
+ * RPMs without manual symlinking, and I suspect that no one ever does
+ * that.
+ */
+struct BITSFUNC(fake_sections)
+{
+ ELF(Shdr) *table;
+ unsigned long table_offset;
+ int count, max_count;
+
+ int in_shstrndx;
+ unsigned long shstr_offset;
+ const char *shstrtab;
+ size_t shstrtab_len;
+
+ int out_shstrndx;
+};
+
+static unsigned int BITSFUNC(find_shname)(struct BITSFUNC(fake_sections) *out,
+ const char *name)
+{
+ const char *outname = out->shstrtab;
+ while (outname - out->shstrtab < out->shstrtab_len) {
+ if (!strcmp(name, outname))
+ return (outname - out->shstrtab) + out->shstr_offset;
+ outname += strlen(outname) + 1;
+ }
+
+ if (*name)
+ printf("Warning: could not find output name \"%s\"\n", name);
+ return out->shstr_offset + out->shstrtab_len - 1; /* Use a null. */
+}
+
+static void BITSFUNC(init_sections)(struct BITSFUNC(fake_sections) *out)
+{
+ if (!out->in_shstrndx)
+ fail("didn't find the fake shstrndx\n");
+
+ memset(out->table, 0, out->max_count * sizeof(ELF(Shdr)));
+
+ if (out->max_count < 1)
+ fail("we need at least two fake output sections\n");
+
+ PUT_LE(&out->table[0].sh_type, SHT_NULL);
+ PUT_LE(&out->table[0].sh_name, BITSFUNC(find_shname)(out, ""));
+
+ out->count = 1;
+}
+
+static void BITSFUNC(copy_section)(struct BITSFUNC(fake_sections) *out,
+ int in_idx, const ELF(Shdr) *in,
+ const char *name)
+{
+ uint64_t flags = GET_LE(&in->sh_flags);
+
+ bool copy = flags & SHF_ALLOC;
+
+ if (!copy)
+ return;
+
+ if (out->count >= out->max_count)
+ fail("too many copied sections (max = %d)\n", out->max_count);
+
+ if (in_idx == out->in_shstrndx)
+ out->out_shstrndx = out->count;
+
+ out->table[out->count] = *in;
+ PUT_LE(&out->table[out->count].sh_name,
+ BITSFUNC(find_shname)(out, name));
+
+ /* elfutils requires that a strtab have the correct type. */
+ if (!strcmp(name, ".fake_shstrtab"))
+ PUT_LE(&out->table[out->count].sh_type, SHT_STRTAB);
+
+ out->count++;
+}
+
static void BITSFUNC(go)(void *addr, size_t len,
FILE *outfile, const char *name)
{
@@ -19,7 +129,7 @@ static void BITSFUNC(go)(void *addr, size_t len,
const char *secstrings;
uint64_t syms[NSYMS] = {};

- uint64_t fake_sections_value = 0, fake_sections_size = 0;
+ struct BITSFUNC(fake_sections) fake_sections = {};

ELF(Phdr) *pt = (ELF(Phdr) *)(addr + GET_LE(&hdr->e_phoff));

@@ -89,23 +199,57 @@ static void BITSFUNC(go)(void *addr, size_t len,
GET_LE(&sym->st_name);

for (k = 0; k < NSYMS; k++) {
- if (!strcmp(name, required_syms[k])) {
+ if (!strcmp(name, required_syms[k].name)) {
if (syms[k]) {
fail("duplicate symbol %s\n",
- required_syms[k]);
+ required_syms[k].name);
}
syms[k] = GET_LE(&sym->st_value);
}
}

- if (!strcmp(name, "vdso_fake_sections")) {
- if (fake_sections_value)
- fail("duplicate vdso_fake_sections\n");
- fake_sections_value = GET_LE(&sym->st_value);
- fake_sections_size = GET_LE(&sym->st_size);
+ if (!strcmp(name, "fake_shstrtab")) {
+ ELF(Shdr) *sh;
+
+ fake_sections.in_shstrndx = GET_LE(&sym->st_shndx);
+ fake_sections.shstrtab = addr + GET_LE(&sym->st_value);
+ fake_sections.shstrtab_len = GET_LE(&sym->st_size);
+ sh = addr + GET_LE(&hdr->e_shoff) +
+ GET_LE(&hdr->e_shentsize) *
+ fake_sections.in_shstrndx;
+ fake_sections.shstr_offset = GET_LE(&sym->st_value) -
+ GET_LE(&sh->sh_addr);
}
}

+ /* Build the output section table. */
+ if (!syms[sym_VDSO_FAKE_SECTION_TABLE_START] ||
+ !syms[sym_VDSO_FAKE_SECTION_TABLE_END])
+ fail("couldn't find fake section table\n");
+ if ((syms[sym_VDSO_FAKE_SECTION_TABLE_END] -
+ syms[sym_VDSO_FAKE_SECTION_TABLE_START]) % sizeof(ELF(Shdr)))
+ fail("fake section table size isn't a multiple of sizeof(Shdr)\n");
+ fake_sections.table = addr + syms[sym_VDSO_FAKE_SECTION_TABLE_START];
+ fake_sections.table_offset = syms[sym_VDSO_FAKE_SECTION_TABLE_START];
+ fake_sections.max_count = (syms[sym_VDSO_FAKE_SECTION_TABLE_END] -
+ syms[sym_VDSO_FAKE_SECTION_TABLE_START]) /
+ sizeof(ELF(Shdr));
+
+ BITSFUNC(init_sections)(&fake_sections);
+ for (i = 0; i < GET_LE(&hdr->e_shnum); i++) {
+ ELF(Shdr) *sh = addr + GET_LE(&hdr->e_shoff) +
+ GET_LE(&hdr->e_shentsize) * i;
+ BITSFUNC(copy_section)(&fake_sections, i, sh,
+ secstrings + GET_LE(&sh->sh_name));
+ }
+ if (!fake_sections.out_shstrndx)
+ fail("didn't generate shstrndx?!?\n");
+
+ PUT_LE(&hdr->e_shoff, fake_sections.table_offset);
+ PUT_LE(&hdr->e_shentsize, sizeof(ELF(Shdr)));
+ PUT_LE(&hdr->e_shnum, fake_sections.count);
+ PUT_LE(&hdr->e_shstrndx, fake_sections.out_shstrndx);
+
/* Validate mapping addresses. */
for (i = 0; i < sizeof(special_pages) / sizeof(special_pages[0]); i++) {
if (!syms[i])
@@ -113,25 +257,17 @@ static void BITSFUNC(go)(void *addr, size_t len,

if (syms[i] % 4096)
fail("%s must be a multiple of 4096\n",
- required_syms[i]);
+ required_syms[i].name);
if (syms[i] < data_size)
fail("%s must be after the text mapping\n",
- required_syms[i]);
+ required_syms[i].name);
if (syms[sym_end_mapping] < syms[i] + 4096)
- fail("%s overruns end_mapping\n", required_syms[i]);
+ fail("%s overruns end_mapping\n",
+ required_syms[i].name);
}
if (syms[sym_end_mapping] % 4096)
fail("end_mapping must be a multiple of 4096\n");

- /* Remove sections or use fakes */
- if (fake_sections_size % sizeof(ELF(Shdr)))
- fail("vdso_fake_sections size is not a multiple of %ld\n",
- (long)sizeof(ELF(Shdr)));
- PUT_LE(&hdr->e_shoff, fake_sections_value);
- PUT_LE(&hdr->e_shentsize, fake_sections_value ? sizeof(ELF(Shdr)) : 0);
- PUT_LE(&hdr->e_shnum, fake_sections_size / sizeof(ELF(Shdr)));
- PUT_LE(&hdr->e_shstrndx, SHN_UNDEF);
-
if (!name) {
fwrite(addr, load_size, 1, outfile);
return;
@@ -169,9 +305,9 @@ static void BITSFUNC(go)(void *addr, size_t len,
(unsigned long)GET_LE(&alt_sec->sh_size));
}
for (i = 0; i < NSYMS; i++) {
- if (syms[i])
+ if (required_syms[i].export && syms[i])
fprintf(outfile, "\t.sym_%s = 0x%" PRIx64 ",\n",
- required_syms[i], syms[i]);
+ required_syms[i].name, syms[i]);
}
fprintf(outfile, "};\n");
}
diff --git a/arch/x86/vdso/vdso32/vdso-fakesections.c b/arch/x86/vdso/vdso32/vdso-fakesections.c
new file mode 100644
index 0000000..541468e
--- /dev/null
+++ b/arch/x86/vdso/vdso32/vdso-fakesections.c
@@ -0,0 +1 @@
+#include "../vdso-fakesections.c"
diff --git a/arch/x86/vdso/vdsox32.lds.S b/arch/x86/vdso/vdsox32.lds.S
index 46b991b..697c11e 100644
--- a/arch/x86/vdso/vdsox32.lds.S
+++ b/arch/x86/vdso/vdsox32.lds.S
@@ -6,6 +6,8 @@
* the DSO.
*/

+#define BUILD_VDSOX32
+
#include "vdso-layout.lds.S"

/*

Subject: [tip:x86/urgent] x86/vdso: Remove some redundant in-memory section headers

Commit-ID: 0e3727a8839c988a3c56170bc8da76d55a16acad
Gitweb: http://git.kernel.org/tip/0e3727a8839c988a3c56170bc8da76d55a16acad
Author: Andy Lutomirski <[email protected]>
AuthorDate: Wed, 18 Jun 2014 15:59:49 -0700
Committer: H. Peter Anvin <[email protected]>
CommitDate: Thu, 19 Jun 2014 15:45:26 -0700

x86/vdso: Remove some redundant in-memory section headers

.data doesn't need to be separate from .rodata: they're both readonly.

.altinstructions and .altinstr_replacement aren't needed by anything
except vdso2c; strip them from the final image.

While we're at it, rather than aligning the actual executable text,
just shove some unused-at-runtime data in between real data and
text.

My vdso image is still above 4k, but I'm disinclined to try to
trim it harder for 3.16. For future trimming, I suspect that these
sections could be moved to later in the file and dropped from
the in-memory image:

.gnu.version and .gnu.version_d (this may lose versions in gdb)
.eh_frame (should be harmless)
.eh_frame_hdr (I'm not really sure)
.hash (AFAIK nothing needs this section header)

Signed-off-by: Andy Lutomirski <[email protected]>
Link: http://lkml.kernel.org/r/2e96d0c49016ea6d026a614ae645e93edd325961.1403129369.git.luto@amacapital.net
Signed-off-by: H. Peter Anvin <[email protected]>
---
arch/x86/vdso/vdso-fakesections.c | 3 ---
arch/x86/vdso/vdso-layout.lds.S | 43 +++++++++++++++++++++------------------
arch/x86/vdso/vdso2c.h | 4 +++-
3 files changed, 26 insertions(+), 24 deletions(-)

diff --git a/arch/x86/vdso/vdso-fakesections.c b/arch/x86/vdso/vdso-fakesections.c
index 56927a7..aa5fbfa 100644
--- a/arch/x86/vdso/vdso-fakesections.c
+++ b/arch/x86/vdso/vdso-fakesections.c
@@ -16,9 +16,6 @@ const char fake_shstrtab[] __attribute__((section(".fake_shstrtab"))) =
".rodata\0"
".fake_shstrtab\0" /* Yay, self-referential code. */
".note\0"
- ".data\0"
- ".altinstructions\0"
- ".altinstr_replacement\0"
".eh_frame_hdr\0"
".eh_frame\0"
".text";
diff --git a/arch/x86/vdso/vdso-layout.lds.S b/arch/x86/vdso/vdso-layout.lds.S
index e4cbc21..9197544 100644
--- a/arch/x86/vdso/vdso-layout.lds.S
+++ b/arch/x86/vdso/vdso-layout.lds.S
@@ -14,7 +14,7 @@
# error unknown VDSO target
#endif

-#define NUM_FAKE_SHDRS 16
+#define NUM_FAKE_SHDRS 13

SECTIONS
{
@@ -28,15 +28,17 @@ SECTIONS
.gnu.version_d : { *(.gnu.version_d) }
.gnu.version_r : { *(.gnu.version_r) }

- .note : { *(.note.*) } :text :note
-
- .eh_frame_hdr : { *(.eh_frame_hdr) } :text :eh_frame_hdr
- .eh_frame : { KEEP (*(.eh_frame)) } :text
-
.dynamic : { *(.dynamic) } :text :dynamic

.rodata : {
*(.rodata*)
+ *(.data*)
+ *(.sdata*)
+ *(.got.plt) *(.got)
+ *(.gnu.linkonce.d.*)
+ *(.bss*)
+ *(.dynbss*)
+ *(.gnu.linkonce.b.*)

/*
* Ideally this would live in a C file, but that won't
@@ -50,28 +52,29 @@ SECTIONS

.fake_shstrtab : { *(.fake_shstrtab) } :text

- .data : {
- *(.data*)
- *(.sdata*)
- *(.got.plt) *(.got)
- *(.gnu.linkonce.d.*)
- *(.bss*)
- *(.dynbss*)
- *(.gnu.linkonce.b.*)
- }

- .altinstructions : { *(.altinstructions) }
- .altinstr_replacement : { *(.altinstr_replacement) }
+ .note : { *(.note.*) } :text :note
+
+ .eh_frame_hdr : { *(.eh_frame_hdr) } :text :eh_frame_hdr
+ .eh_frame : { KEEP (*(.eh_frame)) } :text
+

/*
- * Align the actual code well away from the non-instruction data.
- * This is the best thing for the I-cache.
+ * Text is well-separated from actual data: there's plenty of
+ * stuff that isn't used at runtime in between.
*/
- . = ALIGN(0x100);

.text : { *(.text*) } :text =0x90909090,

/*
+ * At the end so that eu-elflint stays happy when vdso2c strips
+ * these. A better implementation would avoid allocating space
+ * for these.
+ */
+ .altinstructions : { *(.altinstructions) } :text
+ .altinstr_replacement : { *(.altinstr_replacement) } :text
+
+ /*
* The remainder of the vDSO consists of special pages that are
* shared between the kernel and userspace. It needs to be at the
* end so that it doesn't overlap the mapping of the actual
diff --git a/arch/x86/vdso/vdso2c.h b/arch/x86/vdso/vdso2c.h
index f01ed4b..f42e2dd 100644
--- a/arch/x86/vdso/vdso2c.h
+++ b/arch/x86/vdso/vdso2c.h
@@ -92,7 +92,9 @@ static void BITSFUNC(copy_section)(struct BITSFUNC(fake_sections) *out,
{
uint64_t flags = GET_LE(&in->sh_flags);

- bool copy = flags & SHF_ALLOC;
+ bool copy = flags & SHF_ALLOC &&
+ strcmp(name, ".altinstructions") &&
+ strcmp(name, ".altinstr_replacement");

if (!copy)
return;

2014-06-22 08:48:12

by Ingo Molnar

[permalink] [raw]
Subject: Re: [tip:x86/urgent] x86/vdso: Discard the __bug_table section


* tip-bot for Andy Lutomirski <[email protected]> wrote:

> Commit-ID: 5f56e7167e6d438324fcba87018255d81e201383
> Gitweb: http://git.kernel.org/tip/5f56e7167e6d438324fcba87018255d81e201383
> Author: Andy Lutomirski <[email protected]>
> AuthorDate: Wed, 18 Jun 2014 15:59:46 -0700
> Committer: H. Peter Anvin <[email protected]>
> CommitDate: Thu, 19 Jun 2014 15:44:51 -0700
>
> x86/vdso: Discard the __bug_table section
>
> It serves no purpose in user code.
>
> Signed-off-by: Andy Lutomirski <[email protected]>
> Link: http://lkml.kernel.org/r/2a5bebff42defd8a5e81d96f7dc00f21143c80e8.1403129369.git.luto@amacapital.net
> Signed-off-by: H. Peter Anvin <[email protected]>
> ---
> arch/x86/vdso/vdso-layout.lds.S | 1 +
> 1 file changed, 1 insertion(+)

One of the recent x86/urgent vdso commits causes this build failure:

Error: too many copied sections (max = 13)
make[2]: *** [arch/x86/vdso/vdso-image-64.c] Error 1
make[1]: *** [arch/x86/vdso] Error 2
make: *** [arch/x86] Error 2

with the attached config.

Thanks,

Ingo


Attachments:
(No filename) (1.03 kB)
config (103.10 kB)
Download all attachments

2014-06-22 16:59:57

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [tip:x86/urgent] x86/vdso: Discard the __bug_table section

On Sun, Jun 22, 2014 at 1:47 AM, Ingo Molnar <[email protected]> wrote:
>
> * tip-bot for Andy Lutomirski <[email protected]> wrote:
>
>> Commit-ID: 5f56e7167e6d438324fcba87018255d81e201383
>> Gitweb: http://git.kernel.org/tip/5f56e7167e6d438324fcba87018255d81e201383
>> Author: Andy Lutomirski <[email protected]>
>> AuthorDate: Wed, 18 Jun 2014 15:59:46 -0700
>> Committer: H. Peter Anvin <[email protected]>
>> CommitDate: Thu, 19 Jun 2014 15:44:51 -0700
>>
>> x86/vdso: Discard the __bug_table section
>>
>> It serves no purpose in user code.
>>
>> Signed-off-by: Andy Lutomirski <[email protected]>
>> Link: http://lkml.kernel.org/r/2a5bebff42defd8a5e81d96f7dc00f21143c80e8.1403129369.git.luto@amacapital.net
>> Signed-off-by: H. Peter Anvin <[email protected]>
>> ---
>> arch/x86/vdso/vdso-layout.lds.S | 1 +
>> 1 file changed, 1 insertion(+)
>
> One of the recent x86/urgent vdso commits causes this build failure:
>
> Error: too many copied sections (max = 13)

I can't reproduce this with your config, which suggestes a binutils
issue, which is annoying. Can you tell me what version of ld you're
using and send me the output of:

for i in arch/x86/vdso/*.so.dbg; do echo $i; eu-readelf -S $i; done

To summarize, the issue is that, in 3.16, the vvar area is accessed in
a PC-relative manner from the vdso code. So we have:

vdso page 1 | vdso page 2 | vvar page 1 | vvar page 2 (where the
number of vdso pages can vary)

The difficulty comes from the fact that a decent amount of userspace
code wants the vdso to have some section headers, and linkers don't
stick section headers into allocatable sections, since section headers
were never intended to be memory mapped. So there's a risk that the
section headers dangle off the last loadable page in the vdso, at
which point they overlap the vvar area. I've seen this happen.

The "solution" in tip is to move the section headers into a real
allocatable area reserved for that purpose. The error you're seeing
is that I didn't allocate enough space for all the allocatable section
headers.

This crap almost makes me want to go back to something closer to
Stefani's implementation of sticking vvar before the vdso so we can
safely leave non-allocatable crap dangling off the end of the vdso. I
don't really want to play section header whack-a-mole. We could also
just give up on space efficiency and allocate space for several extra
section headers (and their associated names!).

--Andy

2014-06-24 18:19:46

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [tip:x86/urgent] x86/vdso: Discard the __bug_table section

On Sun, Jun 22, 2014 at 9:59 AM, Andy Lutomirski <[email protected]> wrote:
> On Sun, Jun 22, 2014 at 1:47 AM, Ingo Molnar <[email protected]> wrote:
>>
>> * tip-bot for Andy Lutomirski <[email protected]> wrote:
>>
>>> Commit-ID: 5f56e7167e6d438324fcba87018255d81e201383
>>> Gitweb: http://git.kernel.org/tip/5f56e7167e6d438324fcba87018255d81e201383
>>> Author: Andy Lutomirski <[email protected]>
>>> AuthorDate: Wed, 18 Jun 2014 15:59:46 -0700
>>> Committer: H. Peter Anvin <[email protected]>
>>> CommitDate: Thu, 19 Jun 2014 15:44:51 -0700
>>>
>>> x86/vdso: Discard the __bug_table section
>>>
>>> It serves no purpose in user code.
>>>
>>> Signed-off-by: Andy Lutomirski <[email protected]>
>>> Link: http://lkml.kernel.org/r/2a5bebff42defd8a5e81d96f7dc00f21143c80e8.1403129369.git.luto@amacapital.net
>>> Signed-off-by: H. Peter Anvin <[email protected]>
>>> ---
>>> arch/x86/vdso/vdso-layout.lds.S | 1 +
>>> 1 file changed, 1 insertion(+)
>>
>> One of the recent x86/urgent vdso commits causes this build failure:
>>
>> Error: too many copied sections (max = 13)
>
> I can't reproduce this with your config, which suggestes a binutils
> issue, which is annoying. Can you tell me what version of ld you're
> using and send me the output of:
>
> for i in arch/x86/vdso/*.so.dbg; do echo $i; eu-readelf -S $i; done

Ping!

The current state of this is obviously not so good, but I don't know
how to proceed.

--Andy

2014-06-24 18:26:38

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [tip:x86/urgent] x86/vdso: Discard the __bug_table section

On 06/24/2014 11:19 AM, Andy Lutomirski wrote:
>>>
>>> One of the recent x86/urgent vdso commits causes this build failure:
>>>
>>> Error: too many copied sections (max = 13)
>>
>> I can't reproduce this with your config, which suggestes a binutils
>> issue, which is annoying. Can you tell me what version of ld you're
>> using and send me the output of:
>>
>> for i in arch/x86/vdso/*.so.dbg; do echo $i; eu-readelf -S $i; done
>
> Ping!
>
> The current state of this is obviously not so good, but I don't know
> how to proceed.
>

We used to have this kind of problems with PHDRs, where ld would guess
how much space it would need, somehow guess wrong, and fall on its face.

I think we want to actually print the number that we are trying to copy
in addition to the maximum, and I also noticed the test looks wrong.
Thus I would like to propose the following patch as a diagnostic:

diff --git a/arch/x86/vdso/vdso2c.h b/arch/x86/vdso/vdso2c.h
index f42e2ddc663d..94158e100f26 100644
--- a/arch/x86/vdso/vdso2c.h
+++ b/arch/x86/vdso/vdso2c.h
@@ -99,8 +99,9 @@ static void BITSFUNC(copy_section)(struct
BITSFUNC(fake_sections) *out,
if (!copy)
return;

- if (out->count >= out->max_count)
- fail("too many copied sections (max = %d)\n",
out->max_count);
+ if (out->count > out->max_count)
+ fail("too many copied sections (max = %d, need = %d)\n",
+ out->max_count, out->count);

if (in_idx == out->in_shstrndx)
out->out_shstrndx = out->count;

Does anyone have any objection? Andy, I presume I am correct that =>
should be > there?

-hpa

2014-06-24 18:29:51

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [tip:x86/urgent] x86/vdso: Discard the __bug_table section

On 06/22/2014 01:47 AM, Ingo Molnar wrote:
>
> * tip-bot for Andy Lutomirski <[email protected]> wrote:
>
>> Commit-ID: 5f56e7167e6d438324fcba87018255d81e201383
>> Gitweb: http://git.kernel.org/tip/5f56e7167e6d438324fcba87018255d81e201383
>> Author: Andy Lutomirski <[email protected]>
>> AuthorDate: Wed, 18 Jun 2014 15:59:46 -0700
>> Committer: H. Peter Anvin <[email protected]>
>> CommitDate: Thu, 19 Jun 2014 15:44:51 -0700
>>
>> x86/vdso: Discard the __bug_table section
>>
>> It serves no purpose in user code.
>>
>> Signed-off-by: Andy Lutomirski <[email protected]>
>> Link: http://lkml.kernel.org/r/2a5bebff42defd8a5e81d96f7dc00f21143c80e8.1403129369.git.luto@amacapital.net
>> Signed-off-by: H. Peter Anvin <[email protected]>
>> ---
>> arch/x86/vdso/vdso-layout.lds.S | 1 +
>> 1 file changed, 1 insertion(+)
>
> One of the recent x86/urgent vdso commits causes this build failure:
>
> Error: too many copied sections (max = 13)
> make[2]: *** [arch/x86/vdso/vdso-image-64.c] Error 1
> make[1]: *** [arch/x86/vdso] Error 2
> make: *** [arch/x86] Error 2
>
> with the attached config.
>

Hi Ingo,

Could you try this with the attached patch?

-hpa



Attachments:
0001-x86-vdso-Make-vdso2c-print-the-number-of-sections-ne.patch (1.28 kB)

2014-06-24 18:37:35

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [tip:x86/urgent] x86/vdso: Discard the __bug_table section

On Tue, Jun 24, 2014 at 11:26 AM, H. Peter Anvin <[email protected]> wrote:
> On 06/24/2014 11:19 AM, Andy Lutomirski wrote:
>>>>
>>>> One of the recent x86/urgent vdso commits causes this build failure:
>>>>
>>>> Error: too many copied sections (max = 13)
>>>
>>> I can't reproduce this with your config, which suggestes a binutils
>>> issue, which is annoying. Can you tell me what version of ld you're
>>> using and send me the output of:
>>>
>>> for i in arch/x86/vdso/*.so.dbg; do echo $i; eu-readelf -S $i; done
>>
>> Ping!
>>
>> The current state of this is obviously not so good, but I don't know
>> how to proceed.
>>
>
> We used to have this kind of problems with PHDRs, where ld would guess
> how much space it would need, somehow guess wrong, and fall on its face.
>
> I think we want to actually print the number that we are trying to copy
> in addition to the maximum, and I also noticed the test looks wrong.
> Thus I would like to propose the following patch as a diagnostic:
>
> diff --git a/arch/x86/vdso/vdso2c.h b/arch/x86/vdso/vdso2c.h
> index f42e2ddc663d..94158e100f26 100644
> --- a/arch/x86/vdso/vdso2c.h
> +++ b/arch/x86/vdso/vdso2c.h
> @@ -99,8 +99,9 @@ static void BITSFUNC(copy_section)(struct
> BITSFUNC(fake_sections) *out,
> if (!copy)
> return;
>
> - if (out->count >= out->max_count)
> - fail("too many copied sections (max = %d)\n",
> out->max_count);
> + if (out->count > out->max_count)
> + fail("too many copied sections (max = %d, need = %d)\n",
> + out->max_count, out->count);
>

I think the old test was correct: we haven't incremented count yet
(it's a couple lines below), so count is the zero-based index to which
we're writing.

I thought of doing the need = %d thing, but I think that the output is
a foregone conclusion: count == max_count + 1 when this fails. A list
of all the section names would be more interesting, but eu-readelf -S
will tell is that.

--Andy

2014-06-24 18:44:09

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [tip:x86/urgent] x86/vdso: Discard the __bug_table section

On 06/24/2014 11:37 AM, Andy Lutomirski wrote:
>>
>> diff --git a/arch/x86/vdso/vdso2c.h b/arch/x86/vdso/vdso2c.h
>> index f42e2ddc663d..94158e100f26 100644
>> --- a/arch/x86/vdso/vdso2c.h
>> +++ b/arch/x86/vdso/vdso2c.h
>> @@ -99,8 +99,9 @@ static void BITSFUNC(copy_section)(struct
>> BITSFUNC(fake_sections) *out,
>> if (!copy)
>> return;
>>
>> - if (out->count >= out->max_count)
>> - fail("too many copied sections (max = %d)\n",
>> out->max_count);
>> + if (out->count > out->max_count)
>> + fail("too many copied sections (max = %d, need = %d)\n",
>> + out->max_count, out->count);
>>
>
> I think the old test was correct: we haven't incremented count yet
> (it's a couple lines below), so count is the zero-based index to which
> we're writing.
>
> I thought of doing the need = %d thing, but I think that the output is
> a foregone conclusion: count == max_count + 1 when this fails. A list
> of all the section names would be more interesting, but eu-readelf -S
> will tell is that.
>

Well, I have reproduced this failure. eu-readelf output included.

-hpa


Attachments:
err (9.74 kB)

2014-06-24 18:47:33

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [tip:x86/urgent] x86/vdso: Discard the __bug_table section

On 06/24/2014 11:29 AM, H. Peter Anvin wrote:
>
> Hi Ingo,
>
> Could you try this with the attached patch?
>

Nevermind, not useful...

-hpa

2014-06-24 19:41:14

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [tip:x86/urgent] x86/vdso: Discard the __bug_table section

On Tue, Jun 24, 2014 at 11:43 AM, H. Peter Anvin <[email protected]> wrote:
> On 06/24/2014 11:37 AM, Andy Lutomirski wrote:
>>>
>>> diff --git a/arch/x86/vdso/vdso2c.h b/arch/x86/vdso/vdso2c.h
>>> index f42e2ddc663d..94158e100f26 100644
>>> --- a/arch/x86/vdso/vdso2c.h
>>> +++ b/arch/x86/vdso/vdso2c.h
>>> @@ -99,8 +99,9 @@ static void BITSFUNC(copy_section)(struct
>>> BITSFUNC(fake_sections) *out,
>>> if (!copy)
>>> return;
>>>
>>> - if (out->count >= out->max_count)
>>> - fail("too many copied sections (max = %d)\n",
>>> out->max_count);
>>> + if (out->count > out->max_count)
>>> + fail("too many copied sections (max = %d, need = %d)\n",
>>> + out->max_count, out->count);
>>>
>>
>> I think the old test was correct: we haven't incremented count yet
>> (it's a couple lines below), so count is the zero-based index to which
>> we're writing.
>>
>> I thought of doing the need = %d thing, but I think that the output is
>> a foregone conclusion: count == max_count + 1 when this fails. A list
>> of all the section names would be more interesting, but eu-readelf -S
>> will tell is that.
>>
>
> Well, I have reproduced this failure. eu-readelf output included.

It's branch profiling. Patches coming.

>
> -hpa
>



--
Andy Lutomirski
AMA Capital Management, LLC

2014-06-24 19:50:28

by Andy Lutomirski

[permalink] [raw]
Subject: [PATCH 1/2] x86,vdso: Move DISABLE_BRANCH_PROFILING into the vdso makefile

It should really apply to everything in the vdso, and putting it in
the C files seems unreliable.

Signed-off-by: Andy Lutomirski <[email protected]>
---
arch/x86/vdso/Makefile | 4 +++-
arch/x86/vdso/vclock_gettime.c | 3 ---
2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/x86/vdso/Makefile b/arch/x86/vdso/Makefile
index 68a15c4..61b04fe 100644
--- a/arch/x86/vdso/Makefile
+++ b/arch/x86/vdso/Makefile
@@ -66,7 +66,8 @@ $(obj)/vdso-image-%.c: $(obj)/vdso%.so.dbg $(obj)/vdso2c FORCE
#
CFL := $(PROFILING) -mcmodel=small -fPIC -O2 -fasynchronous-unwind-tables -m64 \
$(filter -g%,$(KBUILD_CFLAGS)) $(call cc-option, -fno-stack-protector) \
- -fno-omit-frame-pointer -foptimize-sibling-calls
+ -fno-omit-frame-pointer -foptimize-sibling-calls \
+ -DDISABLE_BRANCH_PROFILING

$(vobjs): KBUILD_CFLAGS += $(CFL)

@@ -149,6 +150,7 @@ KBUILD_CFLAGS_32 += -m32 -msoft-float -mregparm=0 -fpic
KBUILD_CFLAGS_32 += $(call cc-option, -fno-stack-protector)
KBUILD_CFLAGS_32 += $(call cc-option, -foptimize-sibling-calls)
KBUILD_CFLAGS_32 += -fno-omit-frame-pointer
+KBUILD_CFLAGS_32 += -DDISABLE_BRANCH_PROFILING
$(vdso32-images:%=$(obj)/%.dbg): KBUILD_CFLAGS = $(KBUILD_CFLAGS_32)

$(vdso32-images:%=$(obj)/%.dbg): $(obj)/vdso32-%.so.dbg: FORCE \
diff --git a/arch/x86/vdso/vclock_gettime.c b/arch/x86/vdso/vclock_gettime.c
index b2e4f49..9793322 100644
--- a/arch/x86/vdso/vclock_gettime.c
+++ b/arch/x86/vdso/vclock_gettime.c
@@ -11,9 +11,6 @@
* Check with readelf after changing.
*/

-/* Disable profiling for userspace code: */
-#define DISABLE_BRANCH_PROFILING
-
#include <uapi/linux/time.h>
#include <asm/vgtod.h>
#include <asm/hpet.h>
--
1.9.3

2014-06-24 19:50:48

by Andy Lutomirski

[permalink] [raw]
Subject: [PATCH 2/2] x86,vdso2c: Error out if DT_RELA is present

It was missing from the list of inappropriate dynamic entries.

Signed-off-by: Andy Lutomirski <[email protected]>
---
arch/x86/vdso/vdso2c.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/vdso/vdso2c.h b/arch/x86/vdso/vdso2c.h
index f42e2dd..df95a2f 100644
--- a/arch/x86/vdso/vdso2c.h
+++ b/arch/x86/vdso/vdso2c.h
@@ -164,7 +164,7 @@ static void BITSFUNC(go)(void *addr, size_t len,
for (i = 0; dyn + i < dyn_end &&
GET_LE(&dyn[i].d_tag) != DT_NULL; i++) {
typeof(dyn[i].d_tag) tag = GET_LE(&dyn[i].d_tag);
- if (tag == DT_REL || tag == DT_RELSZ ||
+ if (tag == DT_REL || tag == DT_RELSZ || tag == DT_RELA ||
tag == DT_RELENT || tag == DT_TEXTREL)
fail("vdso image contains dynamic relocations\n");
}
--
1.9.3

2014-06-24 19:51:30

by Andy Lutomirski

[permalink] [raw]
Subject: [PATCH 0/2] x86: Build fixes for tip/x86/urgent

For reasons that I don't quite understand, the current code to
disable branch profiling in the vdso wasn't working right. Redo it
better.

Patch 2 is a sanity check: if that error happens, then something
is wrong with the vdso.

Andy Lutomirski (2):
x86,vdso: Move DISABLE_BRANCH_PROFILING into the vdso makefile
x86,vdso2c: Error out if DT_RELA is present

arch/x86/vdso/Makefile | 4 +++-
arch/x86/vdso/vclock_gettime.c | 3 ---
arch/x86/vdso/vdso2c.h | 2 +-
3 files changed, 4 insertions(+), 5 deletions(-)

--
1.9.3

2014-06-24 20:10:10

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH 1/2] x86,vdso: Move DISABLE_BRANCH_PROFILING into the vdso makefile

On 06/24/2014 12:50 PM, Andy Lutomirski wrote:
> It should really apply to everything in the vdso, and putting it in
> the C files seems unreliable.

Please write a more detailed patch description. In this case, it was
causing build failures, and that should be noted in the description.

> It was missing from the list of inappropriate dynamic entries.

Same here -- what kind of problems does this cause or indicate?

-hpa

2014-06-24 20:47:01

by Andy Lutomirski

[permalink] [raw]
Subject: [PATCH v2 0/2] x86: Build fixes for tip/x86/urgent

For reasons that I don't quite understand, the current code to
disable branch profiling in the vdso wasn't working right. Redo it
better.

Patch 2 is a sanity check: if that error happens, then something
is wrong with the vdso.

Changes from v1: Improved patch descriptions

Andy Lutomirski (2):
x86,vdso: Move DISABLE_BRANCH_PROFILING into the vdso makefile
x86,vdso2c: Error out if DT_RELA is present

arch/x86/vdso/Makefile | 4 +++-
arch/x86/vdso/vclock_gettime.c | 3 ---
arch/x86/vdso/vdso2c.h | 2 +-
3 files changed, 4 insertions(+), 5 deletions(-)

--
1.9.3

2014-06-24 20:47:08

by Andy Lutomirski

[permalink] [raw]
Subject: [PATCH v2 1/2] x86,vdso: Move DISABLE_BRANCH_PROFILING into the vdso makefile

DISABLE_BRANCH_PROFILING turns off branch profiling (i.e. a
redefinition of 'if'). Branch profiling depends on a bunch of
kernel-internal symbols and generates extra output sections, none of
which are useful or functional in the vDSO.

It's currently turned off for vclock_gettime.c, but vgetcpu.c also
triggers branch profiling, so just turn it off in the makefile.

This fixes the build on some configurations: the vdso could contain
undefined symbols, and the fake section table overflowed due to
ftrace's added sections.

Signed-off-by: Andy Lutomirski <[email protected]>
---
arch/x86/vdso/Makefile | 4 +++-
arch/x86/vdso/vclock_gettime.c | 3 ---
2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/x86/vdso/Makefile b/arch/x86/vdso/Makefile
index 68a15c4..61b04fe 100644
--- a/arch/x86/vdso/Makefile
+++ b/arch/x86/vdso/Makefile
@@ -66,7 +66,8 @@ $(obj)/vdso-image-%.c: $(obj)/vdso%.so.dbg $(obj)/vdso2c FORCE
#
CFL := $(PROFILING) -mcmodel=small -fPIC -O2 -fasynchronous-unwind-tables -m64 \
$(filter -g%,$(KBUILD_CFLAGS)) $(call cc-option, -fno-stack-protector) \
- -fno-omit-frame-pointer -foptimize-sibling-calls
+ -fno-omit-frame-pointer -foptimize-sibling-calls \
+ -DDISABLE_BRANCH_PROFILING

$(vobjs): KBUILD_CFLAGS += $(CFL)

@@ -149,6 +150,7 @@ KBUILD_CFLAGS_32 += -m32 -msoft-float -mregparm=0 -fpic
KBUILD_CFLAGS_32 += $(call cc-option, -fno-stack-protector)
KBUILD_CFLAGS_32 += $(call cc-option, -foptimize-sibling-calls)
KBUILD_CFLAGS_32 += -fno-omit-frame-pointer
+KBUILD_CFLAGS_32 += -DDISABLE_BRANCH_PROFILING
$(vdso32-images:%=$(obj)/%.dbg): KBUILD_CFLAGS = $(KBUILD_CFLAGS_32)

$(vdso32-images:%=$(obj)/%.dbg): $(obj)/vdso32-%.so.dbg: FORCE \
diff --git a/arch/x86/vdso/vclock_gettime.c b/arch/x86/vdso/vclock_gettime.c
index b2e4f49..9793322 100644
--- a/arch/x86/vdso/vclock_gettime.c
+++ b/arch/x86/vdso/vclock_gettime.c
@@ -11,9 +11,6 @@
* Check with readelf after changing.
*/

-/* Disable profiling for userspace code: */
-#define DISABLE_BRANCH_PROFILING
-
#include <uapi/linux/time.h>
#include <asm/vgtod.h>
#include <asm/hpet.h>
--
1.9.3

2014-06-24 20:47:25

by Andy Lutomirski

[permalink] [raw]
Subject: [PATCH v2 2/2] x86,vdso2c: Error out if DT_RELA is present

vdso2c was checking for various types of relocations to detect when
the vdso had undefined symbols or was otherwise dependent on
relocation at load time. Undefined symbols in the vdso would fail if
accessed at runtime, and certain implementation errors (e.g. branch
profiling or incorrect symbol visibilities) could result in data
access through the GOT that requires relocations. This could be
as simple as:

extern char foo;
return foo;

Without some kind of visibility control, the compiler would assume
that foo could be interposed at load time and would generate a
relocation.

My toolchain emits explicit-addent (RELA) instead of implicit-addent
(REL) relocations for data access, and vdso2c forgot to detect those.

Whether these bad relocations would actually fail at runtime depends
on what the linker sticks in the unrelocated references. Nonetheless,
these relocations have no business existing in the vDSO and should be
fixed rather than silently ignored.

This error could trigger on some configurations due to branch
profiling. The previous patch fixed that.

Signed-off-by: Andy Lutomirski <[email protected]>
---
arch/x86/vdso/vdso2c.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/vdso/vdso2c.h b/arch/x86/vdso/vdso2c.h
index f42e2dd..df95a2f 100644
--- a/arch/x86/vdso/vdso2c.h
+++ b/arch/x86/vdso/vdso2c.h
@@ -164,7 +164,7 @@ static void BITSFUNC(go)(void *addr, size_t len,
for (i = 0; dyn + i < dyn_end &&
GET_LE(&dyn[i].d_tag) != DT_NULL; i++) {
typeof(dyn[i].d_tag) tag = GET_LE(&dyn[i].d_tag);
- if (tag == DT_REL || tag == DT_RELSZ ||
+ if (tag == DT_REL || tag == DT_RELSZ || tag == DT_RELA ||
tag == DT_RELENT || tag == DT_TEXTREL)
fail("vdso image contains dynamic relocations\n");
}
--
1.9.3

Subject: [tip:x86/urgent] x86/vdso: Error out in vdso2c if DT_RELA is present

Commit-ID: 6a89d71078dad9b1c49ccdf1ffa656fbe36ccd1e
Gitweb: http://git.kernel.org/tip/6a89d71078dad9b1c49ccdf1ffa656fbe36ccd1e
Author: Andy Lutomirski <[email protected]>
AuthorDate: Tue, 24 Jun 2014 13:46:53 -0700
Committer: H. Peter Anvin <[email protected]>
CommitDate: Tue, 24 Jun 2014 13:53:57 -0700

x86/vdso: Error out in vdso2c if DT_RELA is present

vdso2c was checking for various types of relocations to detect when
the vdso had undefined symbols or was otherwise dependent on
relocation at load time. Undefined symbols in the vdso would fail if
accessed at runtime, and certain implementation errors (e.g. branch
profiling or incorrect symbol visibilities) could result in data
access through the GOT that requires relocations. This could be
as simple as:

extern char foo;
return foo;

Without some kind of visibility control, the compiler would assume
that foo could be interposed at load time and would generate a
relocation.

x86-64 and x32 (as opposed to i386) use explicit-addent (RELA) instead
of implicit-addent (REL) relocations for data access, and vdso2c
forgot to detect those.

Whether these bad relocations would actually fail at runtime depends
on what the linker sticks in the unrelocated references. Nonetheless,
these relocations have no business existing in the vDSO and should be
fixed rather than silently ignored.

This error could trigger on some configurations due to branch
profiling. The previous patch fixed that.

Signed-off-by: Andy Lutomirski <[email protected]>
Link: http://lkml.kernel.org/r/74ef0c00b4d2a3b573e00a4113874e62f772e348.1403642755.git.luto@amacapital.net
Signed-off-by: H. Peter Anvin <[email protected]>
---
arch/x86/vdso/vdso2c.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/vdso/vdso2c.h b/arch/x86/vdso/vdso2c.h
index f42e2dd..df95a2f 100644
--- a/arch/x86/vdso/vdso2c.h
+++ b/arch/x86/vdso/vdso2c.h
@@ -164,7 +164,7 @@ static void BITSFUNC(go)(void *addr, size_t len,
for (i = 0; dyn + i < dyn_end &&
GET_LE(&dyn[i].d_tag) != DT_NULL; i++) {
typeof(dyn[i].d_tag) tag = GET_LE(&dyn[i].d_tag);
- if (tag == DT_REL || tag == DT_RELSZ ||
+ if (tag == DT_REL || tag == DT_RELSZ || tag == DT_RELA ||
tag == DT_RELENT || tag == DT_TEXTREL)
fail("vdso image contains dynamic relocations\n");
}

Subject: [tip:x86/urgent] x86/vdso: Move DISABLE_BRANCH_PROFILING into the vdso makefile

Commit-ID: 46b57a76930f01ebf31230ed35af5beeccb5ad95
Gitweb: http://git.kernel.org/tip/46b57a76930f01ebf31230ed35af5beeccb5ad95
Author: Andy Lutomirski <[email protected]>
AuthorDate: Tue, 24 Jun 2014 13:46:52 -0700
Committer: H. Peter Anvin <[email protected]>
CommitDate: Tue, 24 Jun 2014 13:53:00 -0700

x86/vdso: Move DISABLE_BRANCH_PROFILING into the vdso makefile

DISABLE_BRANCH_PROFILING turns off branch profiling (i.e. a
redefinition of 'if'). Branch profiling depends on a bunch of
kernel-internal symbols and generates extra output sections, none of
which are useful or functional in the vDSO.

It's currently turned off for vclock_gettime.c, but vgetcpu.c also
triggers branch profiling, so just turn it off in the makefile.

This fixes the build on some configurations: the vdso could contain
undefined symbols, and the fake section table overflowed due to
ftrace's added sections.

Signed-off-by: Andy Lutomirski <[email protected]>
Link: http://lkml.kernel.org/r/bf1ec29e03b2bbc081f6dcaefa64db1c3a83fb21.1403642755.git.luto@amacapital.net
Signed-off-by: H. Peter Anvin <[email protected]>
---
arch/x86/vdso/Makefile | 4 +++-
arch/x86/vdso/vclock_gettime.c | 3 ---
2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/x86/vdso/Makefile b/arch/x86/vdso/Makefile
index 68a15c4..61b04fe 100644
--- a/arch/x86/vdso/Makefile
+++ b/arch/x86/vdso/Makefile
@@ -66,7 +66,8 @@ $(obj)/vdso-image-%.c: $(obj)/vdso%.so.dbg $(obj)/vdso2c FORCE
#
CFL := $(PROFILING) -mcmodel=small -fPIC -O2 -fasynchronous-unwind-tables -m64 \
$(filter -g%,$(KBUILD_CFLAGS)) $(call cc-option, -fno-stack-protector) \
- -fno-omit-frame-pointer -foptimize-sibling-calls
+ -fno-omit-frame-pointer -foptimize-sibling-calls \
+ -DDISABLE_BRANCH_PROFILING

$(vobjs): KBUILD_CFLAGS += $(CFL)

@@ -149,6 +150,7 @@ KBUILD_CFLAGS_32 += -m32 -msoft-float -mregparm=0 -fpic
KBUILD_CFLAGS_32 += $(call cc-option, -fno-stack-protector)
KBUILD_CFLAGS_32 += $(call cc-option, -foptimize-sibling-calls)
KBUILD_CFLAGS_32 += -fno-omit-frame-pointer
+KBUILD_CFLAGS_32 += -DDISABLE_BRANCH_PROFILING
$(vdso32-images:%=$(obj)/%.dbg): KBUILD_CFLAGS = $(KBUILD_CFLAGS_32)

$(vdso32-images:%=$(obj)/%.dbg): $(obj)/vdso32-%.so.dbg: FORCE \
diff --git a/arch/x86/vdso/vclock_gettime.c b/arch/x86/vdso/vclock_gettime.c
index b2e4f49..9793322 100644
--- a/arch/x86/vdso/vclock_gettime.c
+++ b/arch/x86/vdso/vclock_gettime.c
@@ -11,9 +11,6 @@
* Check with readelf after changing.
*/

-/* Disable profiling for userspace code: */
-#define DISABLE_BRANCH_PROFILING
-
#include <uapi/linux/time.h>
#include <asm/vgtod.h>
#include <asm/hpet.h>