linux-kernel - [tip: perf/urgent] perf/core: Fix a memory leak in perf_event_parse_addr

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <160476203763.11244.1944858021967804200.tip-bot2@tip-bot2>
Date:   Sat, 07 Nov 2020 15:13:57 -0000
From:   tip-bot2 for kiyin(尹亮) 
        <tip-bot2@...utronix.de>
To:     linux-tip-commits@...r.kernel.org
Cc:     kiyin@...cent.com, Dan Carpenter <dan.carpenter@...cle.com>,
        Ingo Molnar <mingo@...nel.org>,
        "Srivatsa S. Bhat" <srivatsa@...il.mit.edu>,
        Anthony Liguori <aliguori@...zon.com>, x86@...nel.org,
        linux-kernel@...r.kernel.org
Subject: [tip: perf/urgent] perf/core: Fix a memory leak in
 perf_event_parse_addr_filter()

The following commit has been merged into the perf/urgent branch of tip:

Commit-ID:     7bdb157cdebbf95a1cd94ed2e01b338714075d00
Gitweb:        https://git.kernel.org/tip/7bdb157cdebbf95a1cd94ed2e01b338714075d00
Author:        kiyin(尹亮) <kiyin@...cent.com>
AuthorDate:    Wed, 04 Nov 2020 08:23:22 +03:00
Committer:     Ingo Molnar <mingo@...nel.org>
CommitterDate: Sat, 07 Nov 2020 13:07:26 +01:00

perf/core: Fix a memory leak in perf_event_parse_addr_filter()

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@...cent.com>
Signed-off-by: Dan Carpenter <dan.carpenter@...cle.com>
Signed-off-by: Ingo Molnar <mingo@...nel.org>
Cc: "Srivatsa S. Bhat" <srivatsa@...il.mit.edu>
Cc: Anthony Liguori <aliguori@...zon.com>
--
 kernel/events/core.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)
---
 kernel/events/core.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index da467e1..5a29ab0 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -10085,6 +10085,7 @@ perf_event_parse_addr_filter(struct perf_event *event, char *fstr,
 			if (token == IF_SRC_FILE || token == IF_SRC_FILEADDR) {
 				int fpos = token == IF_SRC_FILE ? 2 : 1;
 
+				kfree(filename);
 				filename = match_strdup(&args[fpos]);
 				if (!filename) {
 					ret = -ENOMEM;
@@ -10131,16 +10132,13 @@ perf_event_parse_addr_filter(struct perf_event *event, char *fstr,
 				 */
 				ret = -EOPNOTSUPP;
 				if (!event->ctx->task)
-					goto fail_free_name;
+					goto fail;
 
 				/* look up the path and grab its inode */
 				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 ||
@@ -10160,13 +10158,13 @@ perf_event_parse_addr_filter(struct perf_event *event, char *fstr,
 	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);