History

objtool/klp: Fix symbol resolution for duplicate data symbols

find_sympos() calculates a sympos used by livepatch to disambiguate duplicately-named symbols. For function symbols, there's a hack which counts .text.unlikely symbols before other .text symbols, matching the linker script's section ordering. Not only is the hack fragile, data symbols can have the same problem. So for example, adding a reference to pwq_cache in ep_unregister_pollwait() can trigger a corrupt sympos and a relocation to the wrong pwq_cache symbol in the livepatch module, resulting in a crash or undefined behavior. Remove the existing hack in favor of a fully deterministic solution, using the new .klp.symid table to derive the symbol-to-id mapping from the original vmlinux.o and the id-to-address mapping from the corresponding vmlinux, which can then be used to determine the exact sympos associated with the original vmlinux. Modules don't need any special treatment: the .ko has the same section/symbol ordering as the original whole-archive symbol table.

Josh Poimboeuf committed 3 months ago

Commit 15fa2031 parent 029223d5 files · +427 −62Browse files

Unified · Split

5 files
  1. scripts/livepatch/klp-build +4 −0
  2. tools/objtool/Build +2 −1
  3. tools/objtool/include/objtool/klp.h +5 −0
  4. tools/objtool/klp-diff.c +5 −61
  5. tools/objtool/klp-sympos.c +411 −0
scripts/livepatch/klp-build+4−0
@@ -611,6 +611,8 @@ copy_orig_objects() { · 610 unmodified lines
611611 done
612612 xtrace_restore
613613
614+ cp -f "$PWD/vmlinux" "$ORIG_DIR" || die "missing vmlinux"
615+
614616 mv -f "$TMP_DIR/build.log" "$ORIG_DIR"
615617 touch "$TIMESTAMP"
616618 touch "$ORIG_DIR/.complete"
@@ -681,6 +683,8 @@ generate_checksums() { · 64 unmodified lines
681683 "$OBJTOOL" klp checksum "$dest"
682684 done
683685
686+ [[ -f "$src_dir/vmlinux" ]] && cp -f "$src_dir/vmlinux" "$dest_dir"
687+
684688 touch "$dest_dir/.complete"
685689 }
686690
tools/objtool/Build+2−1
@@ -13,7 +13,8 @@ objtool-$(BUILD_DISAS) += disas.o · 12 unmodified lines
1313 objtool-$(BUILD_DISAS) += trace.o
1414
1515 objtool-$(BUILD_ORC) += orc_gen.o orc_dump.o
16−objtool-$(BUILD_KLP) += builtin-klp.o klp-checksum.o klp-diff.o klp-post-link.o
16+objtool-$(BUILD_KLP) += builtin-klp.o klp-checksum.o klp-diff.o \
17+ klp-post-link.o klp-sympos.o
1718
1819 objtool-y += libstring.o
1920 objtool-y += libctype.o
tools/objtool/include/objtool/klp.h+5−0
@@ -43,9 +43,14 @@ struct klp_symid { · 42 unmodified lines
4343 };
4444
4545 struct objtool_file;
46+struct elf;
47+struct symbol;
4648
4749 int klp_create_symid_sections(struct objtool_file *file);
4850
51+int klp_sympos_init(struct elf *orig);
52+unsigned long klp_find_sympos(struct elf *elf, struct symbol *sym);
53+
4954 int cmd_klp_checksum(int argc, const char **argv);
5055 int cmd_klp_diff(int argc, const char **argv);
5156 int cmd_klp_post_link(int argc, const char **argv);
tools/objtool/klp-diff.c+5−61
@@ -898,65 +898,6 @@ static int correlate_symbols(struct elfs *e) · 897 unmodified lines
898898 return 0;
899899 }
900900
901−/* "sympos" is used by livepatch to disambiguate duplicate symbol names */
902−static unsigned long find_sympos(struct elf *elf, struct symbol *sym)
903−{
904− bool vmlinux = str_ends_with(objname, "vmlinux.o");
905− unsigned long sympos = 0, nr_matches = 0;
906− bool has_dup = false;
907− struct symbol *s;
908−
909− if (sym->bind != STB_LOCAL)
910− return 0;
911−
912− if (vmlinux && is_func_sym(sym)) {
913− /*
914− * HACK: Unfortunately, symbol ordering can differ between
915− * vmlinux.o and vmlinux due to the linker script emitting
916− * .text.unlikely* before .text*. Count .text.unlikely* first.
917− *
918− * TODO: Disambiguate symbols more reliably (checksums?)
919− */
920− for_each_sym(elf, s) {
921− if (strstarts(s->sec->name, ".text.unlikely") &&
922− !strcmp(s->name, sym->name)) {
923− nr_matches++;
924− if (s == sym)
925− sympos = nr_matches;
926− else
927− has_dup = true;
928− }
929− }
930− for_each_sym(elf, s) {
931− if (!strstarts(s->sec->name, ".text.unlikely") &&
932− !strcmp(s->name, sym->name)) {
933− nr_matches++;
934− if (s == sym)
935− sympos = nr_matches;
936− else
937− has_dup = true;
938− }
939− }
940− } else {
941− for_each_sym(elf, s) {
942− if (!strcmp(s->name, sym->name)) {
943− nr_matches++;
944− if (s == sym)
945− sympos = nr_matches;
946− else
947− has_dup = true;
948− }
949− }
950− }
951−
952− if (!sympos) {
953− ERROR("can't find sympos for %s", sym->name);
954− return ULONG_MAX;
955− }
956−
957− return has_dup ? sympos : 0;
958−}
959−
960901 static int clone_sym_relocs(struct elfs *e, struct symbol *patched_sym);
961902
962903 static struct symbol *__clone_symbol(struct elf *elf, struct symbol *patched_sym,
@@ -1418,7 +1359,7 @@ static int clone_reloc_klp(struct elfs *e, struct reloc *patched_reloc, · 455 unmodified lines
14181359 return -1;
14191360
14201361 sym_orig_name = patched_sym->twin->name;
1421− sympos = find_sympos(e->orig, patched_sym->twin);
1362+ sympos = klp_find_sympos(e->orig, patched_sym->twin);
14221363 if (sympos == ULONG_MAX)
14231364 return -1;
14241365 }
@@ -2036,7 +1977,7 @@ static int create_klp_sections(struct elfs *e) · 611 unmodified lines
20361977
20371978 /* klp_func_ext.sympos */
20381979 BUILD_BUG_ON(sizeof(sympos) != sizeof_field(struct klp_func_ext, sympos));
2039− sympos = find_sympos(e->orig, sym->clone->twin);
1980+ sympos = klp_find_sympos(e->orig, sym->clone->twin);
20401981 if (sympos == ULONG_MAX)
20411982 return -1;
20421983 memcpy(func_data + offsetof(struct klp_func_ext, sympos), &sympos,
@@ -2190,6 +2131,9 @@ int cmd_klp_diff(int argc, const char **argv) · 147 unmodified lines
21902131 if (!e.orig || !e.patched)
21912132 return -1;
21922133
2134+ if (klp_sympos_init(e.orig))
2135+ return -1;
2136+
21932137 if (read_exports())
21942138 return -1;
21952139
tools/objtool/klp-sympos.c+411−0
@@ -0,0 +1,411 @@
1+// SPDX-License-Identifier: GPL-2.0-or-later
2+/*
3+ * Compute "sympos", the position used by livepatch to disambiguate
4+ * duplicate symbol names in the patched object.
5+ */
6+#include <stdlib.h>
7+#include <string.h>
8+#include <fcntl.h>
9+
10+#include <objtool/objtool.h>
11+#include <objtool/warn.h>
12+#include <objtool/endianness.h>
13+#include <objtool/klp.h>
14+
15+#include <linux/string.h>
16+
17+struct vmlinux_sym {
18+ struct hlist_node hash;
19+ const char *name;
20+ u64 addr;
21+};
22+
23+struct vmlinux_symid {
24+ struct hlist_node hash;
25+ u64 id;
26+ u64 addr;
27+};
28+
29+struct vmlinux_o_symid {
30+ struct hlist_node hash;
31+ u64 id;
32+ unsigned int sym_idx;
33+};
34+
35+static DEFINE_HASHTABLE(vmlinux_o_symids, 16);
36+
37+/*
38+ * The original linked kernel, found next to the orig vmlinux.o. Read with raw
39+ * libelf rather than elf_open_read(): only the symbol table and the resolved
40+ * .klp.symid table are needed, not the (huge) instruction/reloc machinery.
41+ *
42+ * Both tables are built once by read_orig_vmlinux(). The Elf handle stays
43+ * open because the hashed names point into its mmapped string table.
44+ */
45+static struct {
46+ Elf *elf;
47+ DECLARE_HASHTABLE(syms, 16); /* name -> address */
48+ DECLARE_HASHTABLE(symids, 16); /* .klp.symid id -> address */
49+} vmlinux;
50+
51+/*
52+ * Would the symbol be visible to the runtime's kallsyms-based symbol lookup?
53+ */
54+static bool vmlinux_sym_in_kallsyms(Elf *elf, GElf_Sym *sym)
55+{
56+ unsigned int type = GELF_ST_TYPE(sym->st_info);
57+ GElf_Shdr shdr;
58+ Elf_Scn *scn;
59+
60+ if (sym->st_shndx == SHN_UNDEF || sym->st_shndx >= SHN_LORESERVE)
61+ return false;
62+
63+ if (type == STT_SECTION || type == STT_FILE)
64+ return false;
65+
66+ scn = elf_getscn(elf, sym->st_shndx);
67+ if (!scn || !gelf_getshdr(scn, &shdr))
68+ return false;
69+
70+ return shdr.sh_flags & SHF_ALLOC;
71+}
72+
73+static int read_orig_vmlinux(const char *filename)
74+{
75+ size_t shstrndx, nr_syms = 0, nr_symids = 0, strtab_idx = 0;
76+ Elf_Data *symtab_data = NULL, *symid_data = NULL;
77+ struct klp_symid *symids;
78+ Elf_Scn *scn = NULL;
79+ GElf_Ehdr ehdr;
80+ int fd;
81+
82+ fd = open(filename, O_RDONLY);
83+ if (fd == -1) {
84+ ERROR_GLIBC("can't open '%s'", filename);
85+ return -1;
86+ }
87+
88+ if (elf_version(EV_CURRENT) == EV_NONE) {
89+ ERROR_ELF("elf_version");
90+ return -1;
91+ }
92+
93+ vmlinux.elf = elf_begin(fd, ELF_C_READ_MMAP, NULL);
94+ if (!vmlinux.elf) {
95+ ERROR_ELF("elf_begin");
96+ return -1;
97+ }
98+
99+ if (!gelf_getehdr(vmlinux.elf, &ehdr)) {
100+ ERROR_ELF("gelf_getehdr");
101+ return -1;
102+ }
103+
104+ if (elf_getshdrstrndx(vmlinux.elf, &shstrndx)) {
105+ ERROR_ELF("elf_getshdrstrndx");
106+ return -1;
107+ }
108+
109+ while ((scn = elf_nextscn(vmlinux.elf, scn))) {
110+ const char *name;
111+ GElf_Shdr shdr;
112+
113+ if (!gelf_getshdr(scn, &shdr)) {
114+ ERROR_ELF("gelf_getshdr");
115+ return -1;
116+ }
117+
118+ if (shdr.sh_type == SHT_SYMTAB) {
119+ symtab_data = elf_getdata(scn, NULL);
120+ if (!symtab_data) {
121+ ERROR_ELF("elf_getdata");
122+ return -1;
123+ }
124+ nr_syms = shdr.sh_size / shdr.sh_entsize;
125+ strtab_idx = shdr.sh_link;
126+ continue;
127+ }
128+
129+ name = elf_strptr(vmlinux.elf, shstrndx, shdr.sh_name);
130+ if (name && !strcmp(name, KLP_SYMID_SEC)) {
131+ if (shdr.sh_size % sizeof(struct klp_symid)) {
132+ ERROR("%s: %s: struct klp_symid size mismatch",
133+ filename, KLP_SYMID_SEC);
134+ return -1;
135+ }
136+ symid_data = elf_getdata(scn, NULL);
137+ if (!symid_data) {
138+ ERROR_ELF("elf_getdata");
139+ return -1;
140+ }
141+ nr_symids = shdr.sh_size / sizeof(struct klp_symid);
142+ }
143+ }
144+
145+ if (!symtab_data) {
146+ ERROR("%s: missing symbol table", filename);
147+ return -1;
148+ }
149+
150+ if (!symid_data) {
151+ ERROR("%s: missing %s section, kernel not built with CONFIG_KLP_BUILD?",
152+ filename, KLP_SYMID_SEC);
153+ return -1;
154+ }
155+
156+ for (size_t i = 0; i < nr_syms; i++) {
157+ struct vmlinux_sym *vsym;
158+ const char *name;
159+ GElf_Sym s;
160+
161+ if (!gelf_getsym(symtab_data, i, &s)) {
162+ ERROR_ELF("gelf_getsym");
163+ return -1;
164+ }
165+
166+ if (!vmlinux_sym_in_kallsyms(vmlinux.elf, &s))
167+ continue;
168+
169+ name = elf_strptr(vmlinux.elf, strtab_idx, s.st_name);
170+ if (!name)
171+ continue;
172+
173+ vsym = calloc(1, sizeof(*vsym));
174+ if (!vsym) {
175+ ERROR_GLIBC("calloc");
176+ return -1;
177+ }
178+
179+ vsym->name = name;
180+ vsym->addr = s.st_value;
181+ hash_add(vmlinux.syms, &vsym->hash, str_hash(name));
182+ }
183+
184+ symids = symid_data->d_buf;
185+
186+ for (size_t i = 0; i < nr_symids; i++) {
187+ struct vmlinux_symid *vsymid;
188+
189+ vsymid = calloc(1, sizeof(*vsymid));
190+ if (!vsymid) {
191+ ERROR_GLIBC("calloc");
192+ return -1;
193+ }
194+
195+ vsymid->id = __bswap_if_needed(&ehdr, symids[i].id);
196+ vsymid->addr = __bswap_if_needed(&ehdr, symids[i].addr);
197+ hash_add(vmlinux.symids, &vsymid->hash, vsymid->id);
198+ }
199+
200+ /* the fd and Elf handle stay open, the hashed names live in the mmap */
201+ return 0;
202+}
203+
204+/*
205+ * Read the orig vmlinux.o's .klp.symid table, an array of entries whose 'addr'
206+ * fields have relocs to the symbols they describe.
207+ */
208+static int read_vmlinux_o_symids(struct elf *vmlinux_o)
209+{
210+ struct section *sec;
211+
212+ for_each_sec(vmlinux_o, sec) {
213+ unsigned long nr;
214+
215+ if (strcmp(sec->name, KLP_SYMID_SEC))
216+ continue;
217+
218+ if (sec_size(sec) % sizeof(struct klp_symid)) {
219+ ERROR("%s: %s: struct klp_symid size mismatch",
220+ vmlinux_o->name, KLP_SYMID_SEC);
221+ return -1;
222+ }
223+
224+ nr = sec_size(sec) / sizeof(struct klp_symid);
225+
226+ for (unsigned long i = 0; i < nr; i++) {
227+ unsigned long offset = i * sizeof(struct klp_symid);
228+ struct vmlinux_o_symid *entry;
229+ struct klp_symid *symid;
230+ struct reloc *reloc;
231+
232+ entry = calloc(1, sizeof(*entry));
233+ if (!entry) {
234+ ERROR_GLIBC("calloc");
235+ return -1;
236+ }
237+
238+ symid = sec->data->d_buf + offset;
239+ entry->id = bswap_if_needed(vmlinux_o, symid->id);
240+
241+ reloc = find_reloc_by_dest(vmlinux_o, sec,
242+ offset + offsetof(struct klp_symid, addr));
243+ if (!reloc) {
244+ ERROR("%s: missing reloc for %s entry",
245+ vmlinux_o->name, KLP_SYMID_SEC);
246+ return -1;
247+ }
248+ entry->sym_idx = reloc->sym->idx;
249+
250+ hash_add(vmlinux_o_symids, &entry->hash, entry->sym_idx);
251+ }
252+ }
253+
254+ return 0;
255+}
256+
257+int klp_sympos_init(struct elf *orig)
258+{
259+ char *filename;
260+ int ret;
261+
262+ if (!str_ends_with(objname, "vmlinux.o"))
263+ return 0;
264+
265+ if (read_vmlinux_o_symids(orig))
266+ return -1;
267+
268+ filename = strndup(objname, strlen(objname) - 2);
269+ if (!filename) {
270+ ERROR_GLIBC("strndup");
271+ return -1;
272+ }
273+
274+ ret = read_orig_vmlinux(filename);
275+ free(filename);
276+
277+ return ret;
278+}
279+
280+/* Find the symbol's id in the orig vmlinux.o's .klp.symid table */
281+static int find_vmlinux_o_symid(struct symbol *sym, u64 *id)
282+{
283+ struct vmlinux_o_symid *entry;
284+
285+ hash_for_each_possible(vmlinux_o_symids, entry, hash, sym->idx) {
286+ if (entry->sym_idx == sym->idx) {
287+ *id = entry->id;
288+ return 0;
289+ }
290+ }
291+
292+ ERROR("no %s entry for symbol %s in orig vmlinux.o", KLP_SYMID_SEC,
293+ sym->name);
294+ return -1;
295+}
296+
297+/* Find the symbol's final address in the orig vmlinux's .klp.symid table */
298+static int find_vmlinux_symid_addr(u64 id, u64 *addr)
299+{
300+ struct vmlinux_symid *symid;
301+
302+ hash_for_each_possible(vmlinux.symids, symid, hash, id) {
303+ if (symid->id == id) {
304+ *addr = symid->addr;
305+ return 0;
306+ }
307+ }
308+
309+ return -1;
310+}
311+
312+/*
313+ * Find the sympos of a vmlinux-local symbol by ranking its final address
314+ * among the duplicately named symbols in the linked orig vmlinux, replicating
315+ * the order in which kallsyms_on_each_match_symbol() counts them.
316+ */
317+static unsigned long find_vmlinux_sympos(struct symbol *sym)
318+{
319+ unsigned long nr_matches = 0, sympos = 1;
320+ u32 key = str_hash(sym->name);
321+ struct vmlinux_sym *vsym;
322+ bool found = false;
323+ u64 id, addr;
324+
325+ hash_for_each_possible(vmlinux.syms, vsym, hash, key)
326+ if (!strcmp(vsym->name, sym->name))
327+ nr_matches++;
328+
329+ if (!nr_matches) {
330+ ERROR("can't find symbol %s in orig vmlinux", sym->name);
331+ return ULONG_MAX;
332+ }
333+
334+ /*
335+ * Unique symbols don't need disambiguating. They also have no
336+ * .klp.symid entry, which is only emitted for names duplicated in
337+ * vmlinux.o, so the lookups below would fail.
338+ */
339+ if (nr_matches == 1)
340+ return 0;
341+
342+ if (find_vmlinux_o_symid(sym, &id))
343+ return ULONG_MAX;
344+
345+ if (find_vmlinux_symid_addr(id, &addr)) {
346+ ERROR("no %s entry for symbol %s in orig vmlinux", KLP_SYMID_SEC,
347+ sym->name);
348+ return ULONG_MAX;
349+ }
350+
351+ hash_for_each_possible(vmlinux.syms, vsym, hash, key) {
352+ if (strcmp(vsym->name, sym->name))
353+ continue;
354+
355+ if (vsym->addr < addr)
356+ sympos++;
357+ else if (vsym->addr == addr)
358+ found = true;
359+ }
360+
361+ if (!found) {
362+ ERROR("%s address mismatch for symbol %s, stale orig vmlinux?",
363+ KLP_SYMID_SEC, sym->name);
364+ return ULONG_MAX;
365+ }
366+
367+ return sympos;
368+}
369+
370+/*
371+ * "sympos" is used by livepatch to disambiguate duplicate symbol names.
372+ */
373+unsigned long klp_find_sympos(struct elf *elf, struct symbol *sym)
374+{
375+ unsigned long sympos = 0, nr_matches = 0;
376+ bool has_dup = false;
377+ struct symbol *s;
378+
379+ if (sym->bind != STB_LOCAL)
380+ return 0;
381+
382+ /*
383+ * vmlinux: the final link reorders symbols relative to vmlinux.o,
384+ * so the position needs to be derived from the linked orig vmlinux via
385+ * the .klp.symid table.
386+ */
387+ if (vmlinux.elf)
388+ return find_vmlinux_sympos(sym);
389+
390+ /*
391+ * modules: the final .ko preserves symbol table order, so a
392+ * symtab-order count here matches the runtime count done by
393+ * module_kallsyms_on_each_symbol().
394+ */
395+ for_each_sym(elf, s) {
396+ if (!strcmp(s->name, sym->name)) {
397+ nr_matches++;
398+ if (s == sym)
399+ sympos = nr_matches;
400+ else
401+ has_dup = true;
402+ }
403+ }
404+
405+ if (!sympos) {
406+ ERROR("can't find sympos for %s", sym->name);
407+ return ULONG_MAX;
408+ }
409+
410+ return has_dup ? sympos : 0;
411+}