Skip to content

Commit

Permalink
test_firmware: fix the memory leak of the allocated firmware buffer
Browse files Browse the repository at this point in the history 
The following kernel memory leak was noticed after running
tools/testing/selftests/firmware/fw_run_tests.sh:

[root@pc-mtodorov firmware]# cat /sys/kernel/debug/kmemleak
.
.
.
unreferenced object 0xffff955389bc3400 (size 1024):
  comm "test_firmware-0", pid 5451, jiffies 4294944822 (age 65.652s)
  hex dump (first 32 bytes):
    47 48 34 35 36 37 0a 00 00 00 00 00 00 00 00 00  GH4567..........
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<ffffffff962f5dec>] slab_post_alloc_hook+0x8c/0x3c0
    [<ffffffff962fcca4>] __kmem_cache_alloc_node+0x184/0x240
    [<ffffffff962704de>] kmalloc_trace+0x2e/0xc0
    [<ffffffff9665b42d>] test_fw_run_batch_request+0x9d/0x180
    [<ffffffff95fd813b>] kthread+0x10b/0x140
    [<ffffffff95e033e9>] ret_from_fork+0x29/0x50
unreferenced object 0xffff9553c334b400 (size 1024):
  comm "test_firmware-1", pid 5452, jiffies 4294944822 (age 65.652s)
  hex dump (first 32 bytes):
    47 48 34 35 36 37 0a 00 00 00 00 00 00 00 00 00  GH4567..........
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<ffffffff962f5dec>] slab_post_alloc_hook+0x8c/0x3c0
    [<ffffffff962fcca4>] __kmem_cache_alloc_node+0x184/0x240
    [<ffffffff962704de>] kmalloc_trace+0x2e/0xc0
    [<ffffffff9665b42d>] test_fw_run_batch_request+0x9d/0x180
    [<ffffffff95fd813b>] kthread+0x10b/0x140
    [<ffffffff95e033e9>] ret_from_fork+0x29/0x50
unreferenced object 0xffff9553c334f000 (size 1024):
  comm "test_firmware-2", pid 5453, jiffies 4294944822 (age 65.652s)
  hex dump (first 32 bytes):
    47 48 34 35 36 37 0a 00 00 00 00 00 00 00 00 00  GH4567..........
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<ffffffff962f5dec>] slab_post_alloc_hook+0x8c/0x3c0
    [<ffffffff962fcca4>] __kmem_cache_alloc_node+0x184/0x240
    [<ffffffff962704de>] kmalloc_trace+0x2e/0xc0
    [<ffffffff9665b42d>] test_fw_run_batch_request+0x9d/0x180
    [<ffffffff95fd813b>] kthread+0x10b/0x140
    [<ffffffff95e033e9>] ret_from_fork+0x29/0x50
unreferenced object 0xffff9553c3348400 (size 1024):
  comm "test_firmware-3", pid 5454, jiffies 4294944822 (age 65.652s)
  hex dump (first 32 bytes):
    47 48 34 35 36 37 0a 00 00 00 00 00 00 00 00 00  GH4567..........
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<ffffffff962f5dec>] slab_post_alloc_hook+0x8c/0x3c0
    [<ffffffff962fcca4>] __kmem_cache_alloc_node+0x184/0x240
    [<ffffffff962704de>] kmalloc_trace+0x2e/0xc0
    [<ffffffff9665b42d>] test_fw_run_batch_request+0x9d/0x180
    [<ffffffff95fd813b>] kthread+0x10b/0x140
    [<ffffffff95e033e9>] ret_from_fork+0x29/0x50
[root@pc-mtodorov firmware]#

Note that the size 1024 corresponds to the size of the test firmware
buffer. The actual number of the buffers leaked is around 70-110,
depending on the test run.

The cause of the leak is the following:

request_partial_firmware_into_buf() and request_firmware_into_buf()
provided firmware buffer isn't released on release_firmware(), we
have allocated it and we are responsible for deallocating it manually.
This is introduced in a number of context where previously only
release_firmware() was called, which was insufficient.

Reported-by: Mirsad Goran Todorovac <[email protected]>
Fixes: 7feebfa ("test_firmware: add support for request_firmware_into_buf")
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Dan Carpenter <[email protected]>
Cc: Takashi Iwai <[email protected]>
Cc: Luis Chamberlain <[email protected]>
Cc: Russ Weight <[email protected]>
Cc: Tianfei zhang <[email protected]>
Cc: Christophe JAILLET <[email protected]>
Cc: Zhengchao Shao <[email protected]>
Cc: Colin Ian King <[email protected]>
Cc: [email protected]
Cc: Kees Cook <[email protected]>
Cc: Scott Branden <[email protected]>
Cc: Luis R. Rodriguez <[email protected]>
Cc: [email protected]
Cc: [email protected] # v5.4
Signed-off-by: Mirsad Goran Todorovac <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
  • Loading branch information
@gregkh
Mirsad Goran Todorovac authored and gregkh committed May 31, 2023
1 parent be37bed commit 48e1560
Showing 1 changed file with 18 additions and 1 deletion.
19 changes: 18 additions & 1 deletion lib/test_firmware.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ struct test_batched_req {
bool sent;
const struct firmware *fw;
const char *name;
const char *fw_buf;
struct completion completion;
struct task_struct *task;
struct device *dev;
Expand Down Expand Up @@ -175,8 +176,14 @@ static void __test_release_all_firmware(void)

for (i = 0; i < test_fw_config->num_requests; i++) {
req = &test_fw_config->reqs[i];
if (req->fw)
if (req->fw) {
if (req->fw_buf) {
kfree_const(req->fw_buf);
req->fw_buf = NULL;
}
release_firmware(req->fw);
req->fw = NULL;
}
}

vfree(test_fw_config->reqs);
Expand Down Expand Up @@ -670,6 +677,8 @@ static ssize_t trigger_request_store(struct device *dev,

mutex_lock(&test_fw_mutex);
release_firmware(test_firmware);
if (test_fw_config->reqs)
__test_release_all_firmware();
test_firmware = NULL;
rc = request_firmware(&test_firmware, name, dev);
if (rc) {
Expand Down Expand Up @@ -770,6 +779,8 @@ static ssize_t trigger_async_request_store(struct device *dev,
mutex_lock(&test_fw_mutex);
release_firmware(test_firmware);
test_firmware = NULL;
if (test_fw_config->reqs)
__test_release_all_firmware();
rc = request_firmware_nowait(THIS_MODULE, 1, name, dev, GFP_KERNEL,
NULL, trigger_async_request_cb);
if (rc) {
Expand Down Expand Up @@ -812,6 +823,8 @@ static ssize_t trigger_custom_fallback_store(struct device *dev,

mutex_lock(&test_fw_mutex);
release_firmware(test_firmware);
if (test_fw_config->reqs)
__test_release_all_firmware();
test_firmware = NULL;
rc = request_firmware_nowait(THIS_MODULE, FW_ACTION_NOUEVENT, name,
dev, GFP_KERNEL, NULL,
Expand Down Expand Up @@ -874,6 +887,8 @@ static int test_fw_run_batch_request(void *data)
test_fw_config->buf_size);
if (!req->fw)
kfree(test_buf);
else
req->fw_buf = test_buf;
} else {
req->rc = test_fw_config->req_firmware(&req->fw,
req->name,
Expand Down Expand Up @@ -934,6 +949,7 @@ static ssize_t trigger_batched_requests_store(struct device *dev,
req->fw = NULL;
req->idx = i;
req->name = test_fw_config->name;
req->fw_buf = NULL;
req->dev = dev;
init_completion(&req->completion);
req->task = kthread_run(test_fw_run_batch_request, req,
Expand Down Expand Up @@ -1038,6 +1054,7 @@ ssize_t trigger_batched_requests_async_store(struct device *dev,
for (i = 0; i < test_fw_config->num_requests; i++) {
req = &test_fw_config->reqs[i];
req->name = test_fw_config->name;
req->fw_buf = NULL;
req->fw = NULL;
req->idx = i;
init_completion(&req->completion);
Expand Down

0 comments on commit 48e1560

Please sign in to comment.