2023-08-17 09:16:36

by Junhao He

[permalink] [raw]
Subject: [PATCH 0/2] Fix memory leak in coresight drivers

When build kernel with CONFIG_KASAN=y
there are reports of memory leaks, like:
...
unreferenced object 0xffff2020510fe200 (size 64):
comm "insmod", pid 4642, jiffies 4295983961 (age 46049.752s)
hex dump (first 32 bytes):
10 20 40 06 28 20 ff ff 10 40 7f 06 20 20 ff ff . @.( ...@.. ..
10 20 bb 8a 20 00 ff ff 10 e0 c7 8a 20 00 ff ff . .. ....... ...
backtrace:
[<0000000034ec4724>] __kmem_cache_alloc_node+0x2f8/0x348
[<0000000057fbc15d>] __kmalloc_node_track_caller+0x5c/0x110
[<0000000055d5e34b>] krealloc+0x8c/0x178
[<00000000a4635beb>] coresight_alloc_device_name+0x128/0x188 [coresight]
[<000000000ce9d17b>] smb_probe+0x268/0x478 [ultrasoc_smb]
...
unreferenced object 0xffff00213c141000 (size 1024):
comm "systemd-udevd", pid 2123, jiffies 4294909467 (age 6062.160s)
hex dump (first 32 bytes):
04 00 00 00 02 00 00 00 18 10 14 3c 21 00 ff ff ...........<!...
00 00 00 00 00 00 00 00 03 00 00 00 10 00 00 00 ................
backtrace:
[<000000004b7c9001>] __kmem_cache_alloc_node+0x2f8/0x348
[<00000000b0fc7ceb>] __kmalloc+0x58/0x108
[<0000000064ff4695>] acpi_os_allocate+0x2c/0x68
[<000000007d57d116>] acpi_ut_initialize_buffer+0x54/0xe0
[<0000000024583908>] acpi_evaluate_object+0x388/0x438
[<0000000017b2e72b>] acpi_evaluate_object_typed+0xe8/0x240
[<000000005df0eac2>] coresight_get_platform_data+0x1b4/0x988 [coresight]
...

The patchset based on "coresight: platform: acpi: Ignore the absence of graph"
https://git.kernel.org/pub/scm/linux/kernel/git/coresight/linux.git/commit/?h=next&id=3a2888aa1f962c55ca36119aebe67355c7bf54e4

Junhao He (2):
coresight: Fix memory leak in acpi_buffer->pointer
coresight: core: fix memory leak in dict->fwnode_list

drivers/hwtracing/coresight/coresight-core.c | 20 +++++++++-
.../hwtracing/coresight/coresight-platform.c | 40 ++++++++++++-------
2 files changed, 45 insertions(+), 15 deletions(-)

--
2.33.0



2023-08-17 10:43:00

by Junhao He

[permalink] [raw]
Subject: [PATCH 1/2] coresight: Fix memory leak in acpi_buffer->pointer

There are memory leaks reported by kmemleak:
...
unreferenced object 0xffff00213c141000 (size 1024):
comm "systemd-udevd", pid 2123, jiffies 4294909467 (age 6062.160s)
hex dump (first 32 bytes):
04 00 00 00 02 00 00 00 18 10 14 3c 21 00 ff ff ...........<!...
00 00 00 00 00 00 00 00 03 00 00 00 10 00 00 00 ................
backtrace:
[<000000004b7c9001>] __kmem_cache_alloc_node+0x2f8/0x348
[<00000000b0fc7ceb>] __kmalloc+0x58/0x108
[<0000000064ff4695>] acpi_os_allocate+0x2c/0x68
[<000000007d57d116>] acpi_ut_initialize_buffer+0x54/0xe0
[<0000000024583908>] acpi_evaluate_object+0x388/0x438
[<0000000017b2e72b>] acpi_evaluate_object_typed+0xe8/0x240
[<000000005df0eac2>] coresight_get_platform_data+0x1b4/0x988 [coresight]
...

The ACPI buffer memory (buf.pointer) should be freed. But the buffer
is also used after returning from acpi_get_dsd_graph().
Move the temporary variables buf to acpi_coresight_parse_graph(),
and free it before the function return to prevent memory leak.

Fixes: 76ffa5ab5b79 ("coresight: Support for ACPI bindings")
Signed-off-by: Junhao He <[email protected]>
---
.../hwtracing/coresight/coresight-platform.c | 40 ++++++++++++-------
1 file changed, 26 insertions(+), 14 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c
index 7d7b641c0a71..9d550f5697fa 100644
--- a/drivers/hwtracing/coresight/coresight-platform.c
+++ b/drivers/hwtracing/coresight/coresight-platform.c
@@ -492,19 +492,18 @@ static inline bool acpi_validate_dsd_graph(const union acpi_object *graph)

/* acpi_get_dsd_graph - Find the _DSD Graph property for the given device. */
static const union acpi_object *
-acpi_get_dsd_graph(struct acpi_device *adev)
+acpi_get_dsd_graph(struct acpi_device *adev, struct acpi_buffer *buf)
{
int i;
- struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER };
acpi_status status;
const union acpi_object *dsd;

status = acpi_evaluate_object_typed(adev->handle, "_DSD", NULL,
- &buf, ACPI_TYPE_PACKAGE);
+ buf, ACPI_TYPE_PACKAGE);
if (ACPI_FAILURE(status))
return NULL;

- dsd = buf.pointer;
+ dsd = buf->pointer;

/*
* _DSD property consists tuples { Prop_UUID, Package() }
@@ -555,12 +554,12 @@ acpi_validate_coresight_graph(const union acpi_object *cs_graph)
* returns NULL.
*/
static const union acpi_object *
-acpi_get_coresight_graph(struct acpi_device *adev)
+acpi_get_coresight_graph(struct acpi_device *adev, struct acpi_buffer *buf)
{
const union acpi_object *graph_list, *graph;
int i, nr_graphs;

- graph_list = acpi_get_dsd_graph(adev);
+ graph_list = acpi_get_dsd_graph(adev, buf);
if (!graph_list)
return graph_list;

@@ -661,22 +660,24 @@ static int acpi_coresight_parse_graph(struct device *dev,
struct acpi_device *adev,
struct coresight_platform_data *pdata)
{
+ int ret = 0;
int i, nlinks;
const union acpi_object *graph;
struct coresight_connection conn, zero_conn = {};
struct coresight_connection *new_conn;
+ struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };

- graph = acpi_get_coresight_graph(adev);
+ graph = acpi_get_coresight_graph(adev, &buf);
/*
* There are no graph connections, which is fine for some components.
* e.g., ETE
*/
if (!graph)
- return 0;
+ goto free;

nlinks = graph->package.elements[2].integer.value;
if (!nlinks)
- return 0;
+ goto free;

for (i = 0; i < nlinks; i++) {
const union acpi_object *link = &graph->package.elements[3 + i];
@@ -684,17 +685,28 @@ static int acpi_coresight_parse_graph(struct device *dev,

conn = zero_conn;
dir = acpi_coresight_parse_link(adev, link, &conn);
- if (dir < 0)
- return dir;
+ if (dir < 0) {
+ ret = dir;
+ goto free;
+ }

if (dir == ACPI_CORESIGHT_LINK_MASTER) {
new_conn = coresight_add_out_conn(dev, pdata, &conn);
- if (IS_ERR(new_conn))
- return PTR_ERR(new_conn);
+ if (IS_ERR(new_conn)) {
+ ret = PTR_ERR(new_conn);
+ goto free;
+ }
}
}

- return 0;
+free:
+ /*
+ * When ACPI fails to alloc a buffer, it will free the buffer
+ * created via ACPI_ALLOCATE_BUFFER and set to NULL.
+ * ACPI_FREE can handle NULL pointers, so free it directly.
+ */
+ ACPI_FREE(buf.pointer);
+ return ret;
}

/*
--
2.33.0


2023-08-19 13:13:34

by Junhao He

[permalink] [raw]
Subject: [PATCH 2/2] coresight: core: fix memory leak in dict->fwnode_list

There are memory leaks reported by kmemleak:
...
unreferenced object 0xffff2020103c3200 (size 256):
comm "insmod", pid 4476, jiffies 4294978252 (age 50072.536s)
hex dump (first 32 bytes):
10 60 40 06 28 20 ff ff 10 c0 59 06 20 20 ff ff .`@.( ....Y. ..
10 e0 47 06 28 20 ff ff 10 00 49 06 28 20 ff ff ..G.( ....I.( ..
backtrace:
[<0000000034ec4724>] __kmem_cache_alloc_node+0x2f8/0x348
[<0000000057fbc15d>] __kmalloc_node_track_caller+0x5c/0x110
[<00000055d5e34b>] krealloc+0x8c/0x178
[<00000000a4635beb>] coresight_alloc_device_name+0x128/0x188 [coresight]
[<00000000a92ddfee>] funnel_cs_ops+0x10/0xfffffffffffedaa0 [coresight_funnel]
[<00000000449e20f8>] dynamic_funnel_ids+0x80/0xfffffffffffed840 [coresight_funnel]
...

when remove driver, the golab variables defined by the macro
DEFINE_CORESIGHT_DEVLIST will be released, dict->nr_idx and
dict->fwnode_list are cleared to 0. The lifetime of the golab
variable has ended. So the buffer pointer is lost.

Use the callback of devm_add_action_or_reset() to free memory.

Fixes: 0f5f9b6ba9e1 ("coresight: Use platform agnostic names")
Signed-off-by: Junhao He <[email protected]>
---
drivers/hwtracing/coresight/coresight-core.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 9fabe00a40d6..6849faad697d 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1756,6 +1756,20 @@ bool coresight_loses_context_with_cpu(struct device *dev)
}
EXPORT_SYMBOL_GPL(coresight_loses_context_with_cpu);

+void coresight_release_dev_list(void *data)
+{
+ struct coresight_dev_list *dict = data;
+
+ mutex_lock(&coresight_mutex);
+
+ if (dict->nr_idx) {
+ kfree(dict->fwnode_list);
+ dict->nr_idx = 0;
+ }
+
+ mutex_unlock(&coresight_mutex);
+}
+
/*
* coresight_alloc_device_name - Get an index for a given device in the
* device index list specific to a driver. An index is allocated for a
@@ -1766,12 +1780,16 @@ EXPORT_SYMBOL_GPL(coresight_loses_context_with_cpu);
char *coresight_alloc_device_name(struct coresight_dev_list *dict,
struct device *dev)
{
- int idx;
+ int idx, ret;
char *name = NULL;
struct fwnode_handle **list;

mutex_lock(&coresight_mutex);

+ ret = devm_add_action_or_reset(dev, coresight_release_dev_list, dict);
+ if (ret)
+ goto done;
+
idx = coresight_search_device_idx(dict, dev_fwnode(dev));
if (idx < 0) {
/* Make space for the new entry */
--
2.33.0