test_firmware: fix the memory leak of the allocated firmware buffer
commit 48e156023059e57a8fc68b498439832f7600ffff upstream.
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 <mirsad.todorovac@alu.unizg.hr>
Fixes: 7feebfa487
("test_firmware: add support for request_firmware_into_buf")
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Dan Carpenter <error27@gmail.com>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Russ Weight <russell.h.weight@intel.com>
Cc: Tianfei zhang <tianfei.zhang@intel.com>
Cc: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Cc: Zhengchao Shao <shaozhengchao@huawei.com>
Cc: Colin Ian King <colin.i.king@gmail.com>
Cc: linux-kernel@vger.kernel.org
Cc: Kees Cook <keescook@chromium.org>
Cc: Scott Branden <sbranden@broadcom.com>
Cc: Luis R. Rodriguez <mcgrof@kernel.org>
Cc: linux-kselftest@vger.kernel.org
Cc: stable@vger.kernel.org # v5.4
Signed-off-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
Link: https://lore.kernel.org/r/20230509084746.48259-3-mirsad.todorovac@alu.unizg.hr
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
parent
fb7dce686f
commit
94f3bc7e84
@ -38,6 +38,7 @@ struct test_batched_req {
|
|||||||
bool sent;
|
bool sent;
|
||||||
const struct firmware *fw;
|
const struct firmware *fw;
|
||||||
const char *name;
|
const char *name;
|
||||||
|
const char *fw_buf;
|
||||||
struct completion completion;
|
struct completion completion;
|
||||||
struct task_struct *task;
|
struct task_struct *task;
|
||||||
struct device *dev;
|
struct device *dev;
|
||||||
@ -134,8 +135,14 @@ static void __test_release_all_firmware(void)
|
|||||||
|
|
||||||
for (i = 0; i < test_fw_config->num_requests; i++) {
|
for (i = 0; i < test_fw_config->num_requests; i++) {
|
||||||
req = &test_fw_config->reqs[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);
|
release_firmware(req->fw);
|
||||||
|
req->fw = NULL;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
vfree(test_fw_config->reqs);
|
vfree(test_fw_config->reqs);
|
||||||
@ -489,6 +496,8 @@ static ssize_t trigger_request_store(struct device *dev,
|
|||||||
|
|
||||||
mutex_lock(&test_fw_mutex);
|
mutex_lock(&test_fw_mutex);
|
||||||
release_firmware(test_firmware);
|
release_firmware(test_firmware);
|
||||||
|
if (test_fw_config->reqs)
|
||||||
|
__test_release_all_firmware();
|
||||||
test_firmware = NULL;
|
test_firmware = NULL;
|
||||||
rc = request_firmware(&test_firmware, name, dev);
|
rc = request_firmware(&test_firmware, name, dev);
|
||||||
if (rc) {
|
if (rc) {
|
||||||
@ -531,6 +540,8 @@ static ssize_t trigger_async_request_store(struct device *dev,
|
|||||||
mutex_lock(&test_fw_mutex);
|
mutex_lock(&test_fw_mutex);
|
||||||
release_firmware(test_firmware);
|
release_firmware(test_firmware);
|
||||||
test_firmware = NULL;
|
test_firmware = NULL;
|
||||||
|
if (test_fw_config->reqs)
|
||||||
|
__test_release_all_firmware();
|
||||||
rc = request_firmware_nowait(THIS_MODULE, 1, name, dev, GFP_KERNEL,
|
rc = request_firmware_nowait(THIS_MODULE, 1, name, dev, GFP_KERNEL,
|
||||||
NULL, trigger_async_request_cb);
|
NULL, trigger_async_request_cb);
|
||||||
if (rc) {
|
if (rc) {
|
||||||
@ -573,6 +584,8 @@ static ssize_t trigger_custom_fallback_store(struct device *dev,
|
|||||||
|
|
||||||
mutex_lock(&test_fw_mutex);
|
mutex_lock(&test_fw_mutex);
|
||||||
release_firmware(test_firmware);
|
release_firmware(test_firmware);
|
||||||
|
if (test_fw_config->reqs)
|
||||||
|
__test_release_all_firmware();
|
||||||
test_firmware = NULL;
|
test_firmware = NULL;
|
||||||
rc = request_firmware_nowait(THIS_MODULE, FW_ACTION_NOHOTPLUG, name,
|
rc = request_firmware_nowait(THIS_MODULE, FW_ACTION_NOHOTPLUG, name,
|
||||||
dev, GFP_KERNEL, NULL,
|
dev, GFP_KERNEL, NULL,
|
||||||
@ -625,6 +638,8 @@ static int test_fw_run_batch_request(void *data)
|
|||||||
TEST_FIRMWARE_BUF_SIZE);
|
TEST_FIRMWARE_BUF_SIZE);
|
||||||
if (!req->fw)
|
if (!req->fw)
|
||||||
kfree(test_buf);
|
kfree(test_buf);
|
||||||
|
else
|
||||||
|
req->fw_buf = test_buf;
|
||||||
} else {
|
} else {
|
||||||
req->rc = test_fw_config->req_firmware(&req->fw,
|
req->rc = test_fw_config->req_firmware(&req->fw,
|
||||||
req->name,
|
req->name,
|
||||||
@ -680,6 +695,7 @@ static ssize_t trigger_batched_requests_store(struct device *dev,
|
|||||||
req->fw = NULL;
|
req->fw = NULL;
|
||||||
req->idx = i;
|
req->idx = i;
|
||||||
req->name = test_fw_config->name;
|
req->name = test_fw_config->name;
|
||||||
|
req->fw_buf = NULL;
|
||||||
req->dev = dev;
|
req->dev = dev;
|
||||||
init_completion(&req->completion);
|
init_completion(&req->completion);
|
||||||
req->task = kthread_run(test_fw_run_batch_request, req,
|
req->task = kthread_run(test_fw_run_batch_request, req,
|
||||||
@ -779,6 +795,7 @@ ssize_t trigger_batched_requests_async_store(struct device *dev,
|
|||||||
for (i = 0; i < test_fw_config->num_requests; i++) {
|
for (i = 0; i < test_fw_config->num_requests; i++) {
|
||||||
req = &test_fw_config->reqs[i];
|
req = &test_fw_config->reqs[i];
|
||||||
req->name = test_fw_config->name;
|
req->name = test_fw_config->name;
|
||||||
|
req->fw_buf = NULL;
|
||||||
req->fw = NULL;
|
req->fw = NULL;
|
||||||
req->idx = i;
|
req->idx = i;
|
||||||
init_completion(&req->completion);
|
init_completion(&req->completion);
|
||||||
|
Loading…
Reference in New Issue
Block a user