lkml.org 
[lkml]   [2023]   [Jun]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.15 142/159] test_firmware: fix the memory leak of the allocated firmware buffer
    Date
    From: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>

    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: 7feebfa487b92 ("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>
    ---
    lib/test_firmware.c | 19 ++++++++++++++++++-
    1 file changed, 18 insertions(+), 1 deletion(-)

    --- a/lib/test_firmware.c
    +++ b/lib/test_firmware.c
    @@ -41,6 +41,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;
    @@ -143,8 +144,14 @@ static void __test_release_all_firmware(

    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);
    @@ -586,6 +593,8 @@ static ssize_t trigger_request_store(str

    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) {
    @@ -686,6 +695,8 @@ static ssize_t trigger_async_request_sto
    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) {
    @@ -728,6 +739,8 @@ static ssize_t trigger_custom_fallback_s

    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,
    @@ -790,6 +803,8 @@ static int test_fw_run_batch_request(voi
    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,
    @@ -845,6 +860,7 @@ static ssize_t trigger_batched_requests_
    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,
    @@ -944,6 +960,7 @@ ssize_t trigger_batched_requests_async_s
    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);

    \
     
     \ /
      Last update: 2023-06-07 23:04    [W:3.282 / U:0.088 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site