[PATCH] ftrace: Add ftrace_page to list after the index is calculated

From: Chen Zhongjin
Date: Wed Mar 08 2023 - 02:10:52 EST


KASAN reported follow problem:

BUG: KASAN: use-after-free in lookup_rec
Read of size 8 at addr ffff000199270ff0 by task modprobe

CPU: 2 Comm: modprobe
Hardware name: QEMU KVM Virtual Machine
Call trace:
kasan_report
__asan_load8
lookup_rec
ftrace_location
arch_check_ftrace_location
check_kprobe_address_safe
register_kprobe.part.0
register_kprobe

This happened when lookup_rec accessing pg->records[pg->index - 1].
The accessed position is not a valid records address, it has -16 offset
to the last allocated records page.

In ftrace_process_locs, ftrace_page will be added to ftrace_pages_start
list fist, then its pg->index will be calculated. Before pg->index++,
pg->index == 0.

lookup_rec iterates the whole list if it doesn't find a record. When
there is a page with pg->index == 0, getting pg->records[-1].ip causes
this problem.

Add ftrace_page to the ftrace_pages_start list after pg->index is
calculated, to fix this.

Fixes: 3208230983a0 ("ftrace: Remove usage of "freed" records")
Signed-off-by: Chen Zhongjin <chenzhongjin@xxxxxxxxxx>
---
kernel/trace/ftrace.c | 35 ++++++++++++++++++-----------------
1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 29baa97d0d53..a258c48ad91e 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6804,28 +6804,14 @@ static int ftrace_process_locs(struct module *mod,

mutex_lock(&ftrace_lock);

+ if (WARN_ON(mod && !ftrace_pages))
+ goto out;
+
/*
* Core and each module needs their own pages, as
* modules will free them when they are removed.
* Force a new page to be allocated for modules.
*/
- if (!mod) {
- WARN_ON(ftrace_pages || ftrace_pages_start);
- /* First initialization */
- ftrace_pages = ftrace_pages_start = start_pg;
- } else {
- if (!ftrace_pages)
- goto out;
-
- 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;
- }
-
p = start;
pg = start_pg;
while (p < end) {
@@ -6855,6 +6841,21 @@ static int ftrace_process_locs(struct module *mod,
/* We should have used all pages */
WARN_ON(pg->next);

+ /* Add pages to ftrace_pages list */
+ if (!mod) {
+ WARN_ON(ftrace_pages || ftrace_pages_start);
+ /* First initialization */
+ ftrace_pages_start = start_pg;
+ } else {
+ 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;
+ }
+
/* Assign the last page to ftrace_pages */
ftrace_pages = pg;

--
2.17.1