Skip to content

Commit ef378c3

Browse files
committed
scripts/sorttable: Zero out weak functions in mcount_loc table
When a function is annotated as "weak" and is overridden, the code is not removed. If it is traced, the fentry/mcount location in the weak function will be referenced by the "__mcount_loc" section. This will then be added to the available_filter_functions list. Since only the address of the functions are listed, to find the name to show, a search of kallsyms is used. Since kallsyms will return the function by simply finding the function that the address is after but before the next function, an address of a weak function will show up as the function before it. This is because kallsyms does not save names of weak functions. This has caused issues in the past, as now the traced weak function will be listed in available_filter_functions with the name of the function before it. At best, this will cause the previous function's name to be listed twice. At worse, if the previous function was marked notrace, it will now show up as a function that can be traced. Note that it only shows up that it can be traced but will not be if enabled, which causes confusion. https://lore.kernel.org/all/[email protected]/ The commit b39181f ("ftrace: Add FTRACE_MCOUNT_MAX_OFFSET to avoid adding weak function") was a workaround to this by checking the function address before printing its name. If the address was too far from the function given by the name then instead of printing the name it would print: __ftrace_invalid_address___<invalid-offset> The real issue is that these invalid addresses are listed in the ftrace table look up which available_filter_functions is derived from. A place holder must be listed in that file because set_ftrace_filter may take a series of indexes into that file instead of names to be able to do O(1) lookups to enable filtering (many tools use this method). Even if kallsyms saved the size of the function, it does not remove the need of having these place holders. The real solution is to not add a weak function into the ftrace table in the first place. To solve this, the sorttable.c code that sorts the mcount regions during the build is modified to take a "nm -S vmlinux" input, sort it, and any function listed in the mcount_loc section that is not within a boundary of the function list given by nm is considered a weak function and is zeroed out. Note, this does not mean they will remain zero when booting as KASLR will still shift those addresses. To handle this, the entries in the mcount_loc section will be ignored if they are zero or match the kaslr_offset() value. Before: ~# grep __ftrace_invalid_address___ /sys/kernel/tracing/available_filter_functions | wc -l 551 After: ~# grep __ftrace_invalid_address___ /sys/kernel/tracing/available_filter_functions | wc -l 0 Cc: bpf <[email protected]> Cc: Masami Hiramatsu <[email protected]> Cc: Mark Rutland <[email protected]> Cc: Mathieu Desnoyers <[email protected]> Cc: Andrew Morton <[email protected]> Cc: Peter Zijlstra <[email protected]> Cc: Linus Torvalds <[email protected]> Cc: Masahiro Yamada <[email protected]> Cc: Nathan Chancellor <[email protected]> Cc: Nicolas Schier <[email protected]> Cc: Zheng Yejian <[email protected]> Cc: Martin Kelly <[email protected]> Cc: Christophe Leroy <[email protected]> Cc: Josh Poimboeuf <[email protected]> Cc: Heiko Carstens <[email protected]> Cc: Catalin Marinas <[email protected]> Cc: Will Deacon <[email protected]> Cc: Vasily Gorbik <[email protected]> Cc: Alexander Gordeev <[email protected]> Link: https://lore.kernel.org/[email protected] Signed-off-by: Steven Rostedt (Google) <[email protected]>
1 parent 5fb964f commit ef378c3

File tree

3 files changed

+134
-4
lines changed

3 files changed

+134
-4
lines changed

kernel/trace/ftrace.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7004,6 +7004,7 @@ static int ftrace_process_locs(struct module *mod,
70047004
unsigned long count;
70057005
unsigned long *p;
70067006
unsigned long addr;
7007+
unsigned long kaslr;
70077008
unsigned long flags = 0; /* Shut up gcc */
70087009
int ret = -ENOMEM;
70097010

@@ -7052,6 +7053,9 @@ static int ftrace_process_locs(struct module *mod,
70527053
ftrace_pages->next = start_pg;
70537054
}
70547055

7056+
/* For zeroed locations that were shifted for core kernel */
7057+
kaslr = !mod ? kaslr_offset() : 0;
7058+
70557059
p = start;
70567060
pg = start_pg;
70577061
while (p < end) {
@@ -7063,7 +7067,7 @@ static int ftrace_process_locs(struct module *mod,
70637067
* object files to satisfy alignments.
70647068
* Skip any NULL pointers.
70657069
*/
7066-
if (!addr) {
7070+
if (!addr || addr == kaslr) {
70677071
skipped++;
70687072
continue;
70697073
}

scripts/link-vmlinux.sh

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,12 +177,14 @@ mksysmap()
177177

178178
sorttable()
179179
{
180-
${objtree}/scripts/sorttable ${1}
180+
${NM} -S ${1} > .tmp_vmlinux.nm-sort
181+
${objtree}/scripts/sorttable -s .tmp_vmlinux.nm-sort ${1}
181182
}
182183

183184
cleanup()
184185
{
185186
rm -f .btf.*
187+
rm -f .tmp_vmlinux.nm-sort
186188
rm -f System.map
187189
rm -f vmlinux
188190
rm -f vmlinux.map

scripts/sorttable.c

Lines changed: 126 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -580,6 +580,98 @@ static void rela_write_addend(Elf_Rela *rela, uint64_t val)
580580
e.rela_write_addend(rela, val);
581581
}
582582

583+
struct func_info {
584+
uint64_t addr;
585+
uint64_t size;
586+
};
587+
588+
/* List of functions created by: nm -S vmlinux */
589+
static struct func_info *function_list;
590+
static int function_list_size;
591+
592+
/* Allocate functions in 1k blocks */
593+
#define FUNC_BLK_SIZE 1024
594+
#define FUNC_BLK_MASK (FUNC_BLK_SIZE - 1)
595+
596+
static int add_field(uint64_t addr, uint64_t size)
597+
{
598+
struct func_info *fi;
599+
int fsize = function_list_size;
600+
601+
if (!(fsize & FUNC_BLK_MASK)) {
602+
fsize += FUNC_BLK_SIZE;
603+
fi = realloc(function_list, fsize * sizeof(struct func_info));
604+
if (!fi)
605+
return -1;
606+
function_list = fi;
607+
}
608+
fi = &function_list[function_list_size++];
609+
fi->addr = addr;
610+
fi->size = size;
611+
return 0;
612+
}
613+
614+
/* Only return match if the address lies inside the function size */
615+
static int cmp_func_addr(const void *K, const void *A)
616+
{
617+
uint64_t key = *(const uint64_t *)K;
618+
const struct func_info *a = A;
619+
620+
if (key < a->addr)
621+
return -1;
622+
return key >= a->addr + a->size;
623+
}
624+
625+
/* Find the function in function list that is bounded by the function size */
626+
static int find_func(uint64_t key)
627+
{
628+
return bsearch(&key, function_list, function_list_size,
629+
sizeof(struct func_info), cmp_func_addr) != NULL;
630+
}
631+
632+
static int cmp_funcs(const void *A, const void *B)
633+
{
634+
const struct func_info *a = A;
635+
const struct func_info *b = B;
636+
637+
if (a->addr < b->addr)
638+
return -1;
639+
return a->addr > b->addr;
640+
}
641+
642+
static int parse_symbols(const char *fname)
643+
{
644+
FILE *fp;
645+
char addr_str[20]; /* Only need 17, but round up to next int size */
646+
char size_str[20];
647+
char type;
648+
649+
fp = fopen(fname, "r");
650+
if (!fp) {
651+
perror(fname);
652+
return -1;
653+
}
654+
655+
while (fscanf(fp, "%16s %16s %c %*s\n", addr_str, size_str, &type) == 3) {
656+
uint64_t addr;
657+
uint64_t size;
658+
659+
/* Only care about functions */
660+
if (type != 't' && type != 'T' && type != 'W')
661+
continue;
662+
663+
addr = strtoull(addr_str, NULL, 16);
664+
size = strtoull(size_str, NULL, 16);
665+
if (add_field(addr, size) < 0)
666+
return -1;
667+
}
668+
fclose(fp);
669+
670+
qsort(function_list, function_list_size, sizeof(struct func_info), cmp_funcs);
671+
672+
return 0;
673+
}
674+
583675
static pthread_t mcount_sort_thread;
584676
static bool sort_reloc;
585677

@@ -752,6 +844,21 @@ static void *sort_mcount_loc(void *arg)
752844
goto out;
753845
}
754846

847+
/* zero out any locations not found by function list */
848+
if (function_list_size) {
849+
for (void *ptr = vals; ptr < vals + size; ptr += long_size) {
850+
uint64_t key;
851+
852+
key = long_size == 4 ? r((uint32_t *)ptr) : r8((uint64_t *)ptr);
853+
if (!find_func(key)) {
854+
if (long_size == 4)
855+
*(uint32_t *)ptr = 0;
856+
else
857+
*(uint64_t *)ptr = 0;
858+
}
859+
}
860+
}
861+
755862
compare_values = long_size == 4 ? compare_values_32 : compare_values_64;
756863

757864
qsort(vals, count, long_size, compare_values);
@@ -801,6 +908,8 @@ static void get_mcount_loc(struct elf_mcount_loc *emloc, Elf_Shdr *symtab_sec,
801908
return;
802909
}
803910
}
911+
#else /* MCOUNT_SORT_ENABLED */
912+
static inline int parse_symbols(const char *fname) { return 0; }
804913
#endif
805914

806915
static int do_sort(Elf_Ehdr *ehdr,
@@ -1256,14 +1365,29 @@ int main(int argc, char *argv[])
12561365
int i, n_error = 0; /* gcc-4.3.0 false positive complaint */
12571366
size_t size = 0;
12581367
void *addr = NULL;
1368+
int c;
1369+
1370+
while ((c = getopt(argc, argv, "s:")) >= 0) {
1371+
switch (c) {
1372+
case 's':
1373+
if (parse_symbols(optarg) < 0) {
1374+
fprintf(stderr, "Could not parse %s\n", optarg);
1375+
return -1;
1376+
}
1377+
break;
1378+
default:
1379+
fprintf(stderr, "usage: sorttable [-s nm-file] vmlinux...\n");
1380+
return 0;
1381+
}
1382+
}
12591383

1260-
if (argc < 2) {
1384+
if ((argc - optind) < 1) {
12611385
fprintf(stderr, "usage: sorttable vmlinux...\n");
12621386
return 0;
12631387
}
12641388

12651389
/* Process each file in turn, allowing deep failure. */
1266-
for (i = 1; i < argc; i++) {
1390+
for (i = optind; i < argc; i++) {
12671391
addr = mmap_file(argv[i], &size);
12681392
if (!addr) {
12691393
++n_error;

0 commit comments

Comments
 (0)