Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [Medium] Permanent memory leak of ftrace page group when all tracepoints in a module are rejected. Pre-existing issues: - [High] Lockless readers can observe uninitialized ftrace pages due to a missing write memory barrier. -- commit 47d688aa11de52022b8f09bf86665d475e816eb6 Author: Jose Fernandez (Anthropic) <[email protected]> Subject: ftrace: Let ftrace_call_adjust() reject a patch site This commit addresses an issue where ftrace_process_locs() does not skip a patch site when ftrace_call_adjust() returns 0. It tests the adjusted address and skips the site if it is NULL, preventing a bug_on() during ftrace_init_nop on arm64. > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > index f9d80c7bd9f16..394f82230e0c3 100644 > --- a/kernel/trace/ftrace.c > +++ b/kernel/trace/ftrace.c > @@ -7676,6 +7676,10 @@ static int ftrace_process_locs(struct module *mod, > } > > addr = ftrace_call_adjust(addr); > + if (!addr) { > + skipped++; > + continue; > + } [Severity: Medium] If all tracepoints in a module are rejected and skipped here, the allocated start_pg retains an index of 0. When the module is later unloaded, does this leave the empty page permanently in the global ftrace_pages list? During module unload, ftrace_release_mod() checks if the first record's ip is within the module: kernel/trace/ftrace.c:ftrace_release_mod() { ... for (pg = ftrace_pages_start; pg; pg = pg->next) { rec = &pg->records[0]; if (within_module(rec->ip, mod)) { /* * pg->index could be zero if we only allocated * a page but did not store any records */ ... } Since the record ip is 0, within_module(0, mod) fails, and the page group would not be freed. Can this regression cause a memory leak of the ftrace page group for each incompatible module load? [Severity: High] This isn't a bug introduced by this patch, but while reviewing this function, I noticed that the new ftrace page is published to the lockless list without a write memory barrier earlier in ftrace_process_locs(): kernel/trace/ftrace.c:ftrace_process_locs() { ... if (WARN_ON(ftrace_pages->next)) { /* Hmm, we have free pages? */ while (ftrace_pages->next) ftrace_pages = ftrace_pages->next; } ftrace_pages->next = start_pg; ... } Because the cpu can reorder the kzalloc initialization of start_pg after its publication to the ftrace_pages list, lockless readers like ftrace_location_range() iterating the list under rcu_read_lock() might read uninitialized data: kernel/trace/ftrace.c:lookup_rec() { ... for (pg = ftrace_pages_start; pg; pg = pg->next) { if (pg->index == 0 || rec->ip < pg->records[0].ip || rec->ip > pg->records[pg->index - 1].ip) continue; ... } Could this lead to a panic or memory corruption due to an out-of-bounds access or a garbage pointer dereference in bsearch() if the reader observes an uninitialized pg->index? > > end_offset = (pg->index+1) * sizeof(pg->records[0]); > if (end_offset > PAGE_SIZE << pg->order) { -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
