From ef0ee4e06824057f96e82a29bdd96f06d075ffb1 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Tue, 17 Nov 2020 12:30:19 +0100 Subject: [PATCH] 4.9-stable patches added patches: perf-core-fix-a-memory-leak-in-perf_event_parse_addr_filter.patch perf-core-fix-bad-use-of-igrab.patch perf-core-fix-crash-when-using-hw-tracing-kernel-filters.patch --- ...leak-in-perf_event_parse_addr_filter.patch | 80 +++++++++ .../perf-core-fix-bad-use-of-igrab.patch | 161 ++++++++++++++++++ ...when-using-hw-tracing-kernel-filters.patch | 71 ++++++++ queue-4.9/series | 3 + 4 files changed, 315 insertions(+) create mode 100644 queue-4.9/perf-core-fix-a-memory-leak-in-perf_event_parse_addr_filter.patch create mode 100644 queue-4.9/perf-core-fix-bad-use-of-igrab.patch create mode 100644 queue-4.9/perf-core-fix-crash-when-using-hw-tracing-kernel-filters.patch diff --git a/queue-4.9/perf-core-fix-a-memory-leak-in-perf_event_parse_addr_filter.patch b/queue-4.9/perf-core-fix-a-memory-leak-in-perf_event_parse_addr_filter.patch new file mode 100644 index 00000000000..a5126decb23 --- /dev/null +++ b/queue-4.9/perf-core-fix-a-memory-leak-in-perf_event_parse_addr_filter.patch @@ -0,0 +1,80 @@ +From foo@baz Tue Nov 17 12:22:26 PM CET 2020 +From: "kiyin(尹亮)" +Date: Wed, 4 Nov 2020 08:23:22 +0300 +Subject: perf/core: Fix a memory leak in perf_event_parse_addr_filter() + +From: "kiyin(尹亮)" + +commit 7bdb157cdebbf95a1cd94ed2e01b338714075d00 upstream + +As shown through runtime testing, the "filename" allocation is not +always freed in perf_event_parse_addr_filter(). + +There are three possible ways that this could happen: + + - It could be allocated twice on subsequent iterations through the loop, + - or leaked on the success path, + - or on the failure path. + +Clean up the code flow to make it obvious that 'filename' is always +freed in the reallocation path and in the two return paths as well. + +We rely on the fact that kfree(NULL) is NOP and filename is initialized +with NULL. + +This fixes the leak. No other side effects expected. + +[ Dan Carpenter: cleaned up the code flow & added a changelog. ] +[ Ingo Molnar: updated the changelog some more. ] + +Fixes: 375637bc5249 ("perf/core: Introduce address range filtering") +Signed-off-by: "kiyin(尹亮)" +Signed-off-by: Dan Carpenter +Signed-off-by: Ingo Molnar +Cc: "Srivatsa S. Bhat" +Cc: Anthony Liguori +[sudip: Backported to 4.9: adjust context] +Signed-off-by: Sudip Mukherjee +Signed-off-by: Greg Kroah-Hartman +--- + kernel/events/core.c | 10 ++++------ + 1 file changed, 4 insertions(+), 6 deletions(-) + +--- a/kernel/events/core.c ++++ b/kernel/events/core.c +@@ -8261,6 +8261,7 @@ perf_event_parse_addr_filter(struct perf + if (token == IF_SRC_FILE || token == IF_SRC_FILEADDR) { + int fpos = filter->range ? 2 : 1; + ++ kfree(filename); + filename = match_strdup(&args[fpos]); + if (!filename) { + ret = -ENOMEM; +@@ -8292,10 +8293,7 @@ perf_event_parse_addr_filter(struct perf + ret = kern_path(filename, LOOKUP_FOLLOW, + &filter->path); + if (ret) +- goto fail_free_name; +- +- kfree(filename); +- filename = NULL; ++ goto fail; + + ret = -EINVAL; + if (!filter->path.dentry || +@@ -8313,13 +8311,13 @@ perf_event_parse_addr_filter(struct perf + if (state != IF_STATE_ACTION) + goto fail; + ++ kfree(filename); + kfree(orig); + + return 0; + +-fail_free_name: +- kfree(filename); + fail: ++ kfree(filename); + free_filters_list(filters); + kfree(orig); + diff --git a/queue-4.9/perf-core-fix-bad-use-of-igrab.patch b/queue-4.9/perf-core-fix-bad-use-of-igrab.patch new file mode 100644 index 00000000000..626b249600d --- /dev/null +++ b/queue-4.9/perf-core-fix-bad-use-of-igrab.patch @@ -0,0 +1,161 @@ +From foo@baz Tue Nov 17 12:22:26 PM CET 2020 +From: Song Liu +Date: Tue, 17 Apr 2018 23:29:07 -0700 +Subject: perf/core: Fix bad use of igrab() + +From: Song Liu + +commit 9511bce9fe8e5e6c0f923c09243a713eba560141 upstream + +As Miklos reported and suggested: + + "This pattern repeats two times in trace_uprobe.c and in + kernel/events/core.c as well: + + ret = kern_path(filename, LOOKUP_FOLLOW, &path); + if (ret) + goto fail_address_parse; + + inode = igrab(d_inode(path.dentry)); + path_put(&path); + + And it's wrong. You can only hold a reference to the inode if you + have an active ref to the superblock as well (which is normally + through path.mnt) or holding s_umount. + + This way unmounting the containing filesystem while the tracepoint is + active will give you the "VFS: Busy inodes after unmount..." message + and a crash when the inode is finally put. + + Solution: store path instead of inode." + +This patch fixes the issue in kernel/event/core.c. + +Reviewed-and-tested-by: Alexander Shishkin +Reported-by: Miklos Szeredi +Signed-off-by: Song Liu +Signed-off-by: Peter Zijlstra (Intel) +Cc: +Cc: Alexander Shishkin +Cc: Arnaldo Carvalho de Melo +Cc: Jiri Olsa +Cc: Linus Torvalds +Cc: Peter Zijlstra +Cc: Stephane Eranian +Cc: Thomas Gleixner +Cc: Vince Weaver +Fixes: 375637bc5249 ("perf/core: Introduce address range filtering") +Link: http://lkml.kernel.org/r/20180418062907.3210386-2-songliubraving@fb.com +Signed-off-by: Ingo Molnar +[sudip: Backported to 4.9: use file_inode()] +Signed-off-by: Sudip Mukherjee +Signed-off-by: Greg Kroah-Hartman +--- + arch/x86/events/intel/pt.c | 4 ++-- + include/linux/perf_event.h | 2 +- + kernel/events/core.c | 21 +++++++++------------ + 3 files changed, 12 insertions(+), 15 deletions(-) + +--- a/arch/x86/events/intel/pt.c ++++ b/arch/x86/events/intel/pt.c +@@ -1117,7 +1117,7 @@ static int pt_event_addr_filters_validat + if (!filter->range || !filter->size) + return -EOPNOTSUPP; + +- if (!filter->inode) { ++ if (!filter->path.dentry) { + if (!valid_kernel_ip(filter->offset)) + return -EINVAL; + +@@ -1144,7 +1144,7 @@ static void pt_event_addr_filters_sync(s + return; + + list_for_each_entry(filter, &head->list, entry) { +- if (filter->inode && !offs[range]) { ++ if (filter->path.dentry && !offs[range]) { + msr_a = msr_b = 0; + } else { + /* apply the offset */ +--- a/include/linux/perf_event.h ++++ b/include/linux/perf_event.h +@@ -475,7 +475,7 @@ struct pmu { + */ + struct perf_addr_filter { + struct list_head entry; +- struct inode *inode; ++ struct path path; + unsigned long offset; + unsigned long size; + unsigned int range : 1, +--- a/kernel/events/core.c ++++ b/kernel/events/core.c +@@ -6271,7 +6271,7 @@ static void perf_event_addr_filters_exec + + raw_spin_lock_irqsave(&ifh->lock, flags); + list_for_each_entry(filter, &ifh->list, entry) { +- if (filter->inode) { ++ if (filter->path.dentry) { + event->addr_filters_offs[count] = 0; + restart++; + } +@@ -6814,7 +6814,7 @@ static bool perf_addr_filter_match(struc + struct file *file, unsigned long offset, + unsigned long size) + { +- if (filter->inode != file->f_inode) ++ if (d_inode(filter->path.dentry) != file_inode(file)) + return false; + + if (filter->offset > offset + size) +@@ -8028,8 +8028,7 @@ static void free_filters_list(struct lis + struct perf_addr_filter *filter, *iter; + + list_for_each_entry_safe(filter, iter, filters, entry) { +- if (filter->inode) +- iput(filter->inode); ++ path_put(&filter->path); + list_del(&filter->entry); + kfree(filter); + } +@@ -8123,7 +8122,7 @@ static void perf_event_addr_filters_appl + * Adjust base offset if the filter is associated to a binary + * that needs to be mapped: + */ +- if (filter->inode) ++ if (filter->path.dentry) + event->addr_filters_offs[count] = + perf_addr_filter_apply(filter, mm); + +@@ -8196,7 +8195,6 @@ perf_event_parse_addr_filter(struct perf + { + struct perf_addr_filter *filter = NULL; + char *start, *orig, *filename = NULL; +- struct path path; + substring_t args[MAX_OPT_ARGS]; + int state = IF_STATE_ACTION, token; + unsigned int kernel = 0; +@@ -8287,19 +8285,18 @@ perf_event_parse_addr_filter(struct perf + goto fail; + + /* look up the path and grab its inode */ +- ret = kern_path(filename, LOOKUP_FOLLOW, &path); ++ ret = kern_path(filename, LOOKUP_FOLLOW, ++ &filter->path); + if (ret) + goto fail_free_name; + +- filter->inode = igrab(d_inode(path.dentry)); +- path_put(&path); + kfree(filename); + filename = NULL; + + ret = -EINVAL; +- if (!filter->inode || +- !S_ISREG(filter->inode->i_mode)) +- /* free_filters_list() will iput() */ ++ if (!filter->path.dentry || ++ !S_ISREG(d_inode(filter->path.dentry) ++ ->i_mode)) + goto fail; + } + diff --git a/queue-4.9/perf-core-fix-crash-when-using-hw-tracing-kernel-filters.patch b/queue-4.9/perf-core-fix-crash-when-using-hw-tracing-kernel-filters.patch new file mode 100644 index 00000000000..60731906445 --- /dev/null +++ b/queue-4.9/perf-core-fix-crash-when-using-hw-tracing-kernel-filters.patch @@ -0,0 +1,71 @@ +From foo@baz Tue Nov 17 12:22:26 PM CET 2020 +From: Mathieu Poirier +Date: Mon, 16 Jul 2018 17:13:51 -0600 +Subject: perf/core: Fix crash when using HW tracing kernel filters + +From: Mathieu Poirier + +commit 7f635ff187ab6be0b350b3ec06791e376af238ab upstream + +In function perf_event_parse_addr_filter(), the path::dentry of each struct +perf_addr_filter is left unassigned (as it should be) when the pattern +being parsed is related to kernel space. But in function +perf_addr_filter_match() the same dentries are given to d_inode() where +the value is not expected to be NULL, resulting in the following splat: + + Unable to handle kernel NULL pointer dereference at virtual address 0000000000000058 + pc : perf_event_mmap+0x2fc/0x5a0 + lr : perf_event_mmap+0x2c8/0x5a0 + Process uname (pid: 2860, stack limit = 0x000000001cbcca37) + Call trace: + perf_event_mmap+0x2fc/0x5a0 + mmap_region+0x124/0x570 + do_mmap+0x344/0x4f8 + vm_mmap_pgoff+0xe4/0x110 + vm_mmap+0x2c/0x40 + elf_map+0x60/0x108 + load_elf_binary+0x450/0x12c4 + search_binary_handler+0x90/0x290 + __do_execve_file.isra.13+0x6e4/0x858 + sys_execve+0x3c/0x50 + el0_svc_naked+0x30/0x34 + +This patch is fixing the problem by introducing a new check in function +perf_addr_filter_match() to see if the filter's dentry is NULL. + +Signed-off-by: Mathieu Poirier +Signed-off-by: Peter Zijlstra (Intel) +Acked-by: Alexander Shishkin +Cc: Arnaldo Carvalho de Melo +Cc: Jiri Olsa +Cc: Linus Torvalds +Cc: Peter Zijlstra +Cc: Stephane Eranian +Cc: Thomas Gleixner +Cc: Vince Weaver +Cc: acme@kernel.org +Cc: miklos@szeredi.hu +Cc: namhyung@kernel.org +Cc: songliubraving@fb.com +Fixes: 9511bce9fe8e ("perf/core: Fix bad use of igrab()") +Link: http://lkml.kernel.org/r/1531782831-1186-1-git-send-email-mathieu.poirier@linaro.org +Signed-off-by: Ingo Molnar +Signed-off-by: Sudip Mukherjee +Signed-off-by: Greg Kroah-Hartman +--- + kernel/events/core.c | 4 ++++ + 1 file changed, 4 insertions(+) + +--- a/kernel/events/core.c ++++ b/kernel/events/core.c +@@ -6814,6 +6814,10 @@ static bool perf_addr_filter_match(struc + struct file *file, unsigned long offset, + unsigned long size) + { ++ /* d_inode(NULL) won't be equal to any mapped user-space file */ ++ if (!filter->path.dentry) ++ return false; ++ + if (d_inode(filter->path.dentry) != file_inode(file)) + return false; + diff --git a/queue-4.9/series b/queue-4.9/series index 945d27ffe0a..24020ff3dfc 100644 --- a/queue-4.9/series +++ b/queue-4.9/series @@ -56,3 +56,6 @@ net-x25-fix-null-ptr-deref-in-x25_connect.patch net-update-window_clamp-if-sock_rcvbuf-is-set.patch random32-make-prandom_u32-output-unpredictable.patch x86-speculation-allow-ibpb-to-be-conditionally-enabled-on-cpus-with-always-on-stibp.patch +perf-core-fix-bad-use-of-igrab.patch +perf-core-fix-crash-when-using-hw-tracing-kernel-filters.patch +perf-core-fix-a-memory-leak-in-perf_event_parse_addr_filter.patch -- 2.47.3