--- /dev/null
+From foo@baz Tue Nov 17 12:22:26 PM CET 2020
+From: "kiyin(尹亮)" <kiyin@tencent.com>
+Date: Wed, 4 Nov 2020 08:23:22 +0300
+Subject: perf/core: Fix a memory leak in perf_event_parse_addr_filter()
+
+From: "kiyin(尹亮)" <kiyin@tencent.com>
+
+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(尹亮)" <kiyin@tencent.com>
+Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
+Signed-off-by: Ingo Molnar <mingo@kernel.org>
+Cc: "Srivatsa S. Bhat" <srivatsa@csail.mit.edu>
+Cc: Anthony Liguori <aliguori@amazon.com>
+[sudip: Backported to 4.9: adjust context]
+Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+---
+ 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);
+
--- /dev/null
+From foo@baz Tue Nov 17 12:22:26 PM CET 2020
+From: Song Liu <songliubraving@fb.com>
+Date: Tue, 17 Apr 2018 23:29:07 -0700
+Subject: perf/core: Fix bad use of igrab()
+
+From: Song Liu <songliubraving@fb.com>
+
+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 <alexander.shishkin@linux.intel.com>
+Reported-by: Miklos Szeredi <miklos@szeredi.hu>
+Signed-off-by: Song Liu <songliubraving@fb.com>
+Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
+Cc: <kernel-team@fb.com>
+Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
+Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
+Cc: Jiri Olsa <jolsa@redhat.com>
+Cc: Linus Torvalds <torvalds@linux-foundation.org>
+Cc: Peter Zijlstra <peterz@infradead.org>
+Cc: Stephane Eranian <eranian@google.com>
+Cc: Thomas Gleixner <tglx@linutronix.de>
+Cc: Vince Weaver <vincent.weaver@maine.edu>
+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 <mingo@kernel.org>
+[sudip: Backported to 4.9: use file_inode()]
+Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+---
+ 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;
+ }
+
--- /dev/null
+From foo@baz Tue Nov 17 12:22:26 PM CET 2020
+From: Mathieu Poirier <mathieu.poirier@linaro.org>
+Date: Mon, 16 Jul 2018 17:13:51 -0600
+Subject: perf/core: Fix crash when using HW tracing kernel filters
+
+From: Mathieu Poirier <mathieu.poirier@linaro.org>
+
+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 <mathieu.poirier@linaro.org>
+Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
+Acked-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
+Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
+Cc: Jiri Olsa <jolsa@redhat.com>
+Cc: Linus Torvalds <torvalds@linux-foundation.org>
+Cc: Peter Zijlstra <peterz@infradead.org>
+Cc: Stephane Eranian <eranian@google.com>
+Cc: Thomas Gleixner <tglx@linutronix.de>
+Cc: Vince Weaver <vincent.weaver@maine.edu>
+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 <mingo@kernel.org>
+Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+---
+ 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;
+
projects / thirdparty / kernel / stable-queue.git / commitdiff
