Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Yunfeng Ye <yeyunfeng@huawei.com>
Subject: [PATCH] perf kmem: Fix memory leak in __cmd_record()
To:     <peterz@infradead.org>, <mingo@redhat.com>, <acme@kernel.org>,
        <mark.rutland@arm.com>, <alexander.shishkin@linux.intel.com>,
        <jolsa@redhat.com>, <namhyung@kernel.org>
CC:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "hushiyuan@huawei.com" <hushiyuan@huawei.com>,
        "linfeilong@huawei.com" <linfeilong@huawei.com>
Message-ID: <81e3d338-dbf3-341b-431d-27bb51996e46@huawei.com>
Date:   Mon, 28 Oct 2019 14:46:35 +0800
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:60.0) Gecko/20100101
 Thunderbird/60.6.1
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

There are memory leaks in __cmd_record() found by visual inspection.
calloc() and strdup() are used to allocate memory for rec_argv pointer
array and argv, which should be freed before the function returns.

In addition, checking whether strdup() is success or not, if failure,
then go to the failure path to free memory and return the function.

Fixes: ba77c9e11111 ("perf: Add 'perf kmem' tool")
Signed-off-by: Yunfeng Ye <yeyunfeng@huawei.com>
---
 tools/perf/builtin-kmem.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index 9661671cc26e..6a62acfc9470 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -1847,6 +1847,7 @@ static int __cmd_record(int argc, const char **argv)
 	};
 	unsigned int rec_argc, i, j;
 	const char **rec_argv;
+	int ret = -ENOMEM;

 	rec_argc = ARRAY_SIZE(record_args) + argc - 1;
 	if (kmem_slab)
@@ -1873,10 +1874,20 @@ static int __cmd_record(int argc, const char **argv)
 			rec_argv[i] = strdup(page_events[j]);
 	}

-	for (j = 1; j < (unsigned int)argc; j++, i++)
-		rec_argv[i] = argv[j];
+	for (j = 0; j < i; j++)
+		if (!rec_argv[j]) /* check strdup success or not */
+			goto out;
+
+	rec_argc = i;
+	for (j = 1; j < (unsigned int)argc; j++, rec_argc++)
+		rec_argv[rec_argc] = argv[j];

-	return cmd_record(i, rec_argv);
+	ret = cmd_record(rec_argc, rec_argv);
+out:
+	for (i--; (int)i >= 0; i--)
+		free((void *)rec_argv[i]);
+	free(rec_argv);
+	return ret;
 }

 static int kmem_config(const char *var, const char *value, void *cb __maybe_unused)
-- 
2.7.4