2019-05-19 20:08:20 +08:00
|
|
|
// SPDX-License-Identifier: GPL-2.0-only
|
2014-07-15 05:38:12 +08:00
|
|
|
/*
|
|
|
|
* This module provides an interface to trigger and test firmware loading.
|
|
|
|
*
|
|
|
|
* It is designed to be used for basic evaluation of the firmware loading
|
|
|
|
* subsystem (for example when validating firmware verification). It lacks
|
|
|
|
* any extra dependencies, and will not normally be loaded by the system
|
|
|
|
* unless explicitly requested by name.
|
|
|
|
*/
|
|
|
|
|
|
|
|
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
|
|
|
|
|
|
|
|
#include <linux/init.h>
|
|
|
|
#include <linux/module.h>
|
|
|
|
#include <linux/printk.h>
|
2015-12-10 06:50:27 +08:00
|
|
|
#include <linux/completion.h>
|
2014-07-15 05:38:12 +08:00
|
|
|
#include <linux/firmware.h>
|
|
|
|
#include <linux/device.h>
|
|
|
|
#include <linux/fs.h>
|
|
|
|
#include <linux/miscdevice.h>
|
2019-08-23 02:40:04 +08:00
|
|
|
#include <linux/sizes.h>
|
2014-07-15 05:38:12 +08:00
|
|
|
#include <linux/slab.h>
|
|
|
|
#include <linux/uaccess.h>
|
2017-07-21 04:13:42 +08:00
|
|
|
#include <linux/delay.h>
|
2023-01-14 17:22:03 +08:00
|
|
|
#include <linux/kstrtox.h>
|
2017-07-21 04:13:42 +08:00
|
|
|
#include <linux/kthread.h>
|
2018-04-06 07:25:34 +08:00
|
|
|
#include <linux/vmalloc.h>
|
2020-01-16 00:35:49 +08:00
|
|
|
#include <linux/efi_embedded_fw.h>
|
2017-07-21 04:13:42 +08:00
|
|
|
|
2020-09-10 06:53:54 +08:00
|
|
|
MODULE_IMPORT_NS(TEST_FIRMWARE);
|
|
|
|
|
2017-07-21 04:13:42 +08:00
|
|
|
#define TEST_FIRMWARE_NAME "test-firmware.bin"
|
|
|
|
#define TEST_FIRMWARE_NUM_REQS 4
|
2019-08-23 02:40:04 +08:00
|
|
|
#define TEST_FIRMWARE_BUF_SIZE SZ_1K
|
2014-07-15 05:38:12 +08:00
|
|
|
|
|
|
|
static DEFINE_MUTEX(test_fw_mutex);
|
|
|
|
static const struct firmware *test_firmware;
|
|
|
|
|
2017-07-21 04:13:42 +08:00
|
|
|
struct test_batched_req {
|
|
|
|
u8 idx;
|
|
|
|
int rc;
|
|
|
|
bool sent;
|
|
|
|
const struct firmware *fw;
|
|
|
|
const char *name;
|
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: 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>
2023-05-09 16:47:49 +08:00
|
|
|
const char *fw_buf;
|
2017-07-21 04:13:42 +08:00
|
|
|
struct completion completion;
|
|
|
|
struct task_struct *task;
|
|
|
|
struct device *dev;
|
|
|
|
};
|
|
|
|
|
|
|
|
/**
|
|
|
|
* test_config - represents configuration for the test for different triggers
|
|
|
|
*
|
|
|
|
* @name: the name of the firmware file to look for
|
2019-08-23 02:40:04 +08:00
|
|
|
* @into_buf: when the into_buf is used if this is true
|
|
|
|
* request_firmware_into_buf() will be used instead.
|
2020-10-03 01:38:28 +08:00
|
|
|
* @buf_size: size of buf to allocate when into_buf is true
|
|
|
|
* @file_offset: file offset to request when calling request_firmware_into_buf
|
|
|
|
* @partial: partial read opt when calling request_firmware_into_buf
|
2017-07-21 04:13:42 +08:00
|
|
|
* @sync_direct: when the sync trigger is used if this is true
|
|
|
|
* request_firmware_direct() will be used instead.
|
|
|
|
* @send_uevent: whether or not to send a uevent for async requests
|
|
|
|
* @num_requests: number of requests to try per test case. This is trigger
|
|
|
|
* specific.
|
|
|
|
* @reqs: stores all requests information
|
|
|
|
* @read_fw_idx: index of thread from which we want to read firmware results
|
|
|
|
* from through the read_fw trigger.
|
|
|
|
* @test_result: a test may use this to collect the result from the call
|
|
|
|
* of the request_firmware*() calls used in their tests. In order of
|
|
|
|
* priority we always keep first any setup error. If no setup errors were
|
|
|
|
* found then we move on to the first error encountered while running the
|
|
|
|
* API. Note that for async calls this typically will be a successful
|
|
|
|
* result (0) unless of course you've used bogus parameters, or the system
|
|
|
|
* is out of memory. In the async case the callback is expected to do a
|
|
|
|
* bit more homework to figure out what happened, unfortunately the only
|
|
|
|
* information passed today on error is the fact that no firmware was
|
|
|
|
* found so we can only assume -ENOENT on async calls if the firmware is
|
|
|
|
* NULL.
|
|
|
|
*
|
|
|
|
* Errors you can expect:
|
|
|
|
*
|
|
|
|
* API specific:
|
|
|
|
*
|
|
|
|
* 0: success for sync, for async it means request was sent
|
|
|
|
* -EINVAL: invalid parameters or request
|
|
|
|
* -ENOENT: files not found
|
|
|
|
*
|
|
|
|
* System environment:
|
|
|
|
*
|
|
|
|
* -ENOMEM: memory pressure on system
|
|
|
|
* -ENODEV: out of number of devices to test
|
|
|
|
* -EINVAL: an unexpected error has occurred
|
|
|
|
* @req_firmware: if @sync_direct is true this is set to
|
|
|
|
* request_firmware_direct(), otherwise request_firmware()
|
|
|
|
*/
|
|
|
|
struct test_config {
|
|
|
|
char *name;
|
2019-08-23 02:40:04 +08:00
|
|
|
bool into_buf;
|
2020-10-03 01:38:28 +08:00
|
|
|
size_t buf_size;
|
|
|
|
size_t file_offset;
|
|
|
|
bool partial;
|
2017-07-21 04:13:42 +08:00
|
|
|
bool sync_direct;
|
|
|
|
bool send_uevent;
|
|
|
|
u8 num_requests;
|
|
|
|
u8 read_fw_idx;
|
|
|
|
|
|
|
|
/*
|
|
|
|
* These below don't belong her but we'll move them once we create
|
|
|
|
* a struct fw_test_device and stuff the misc_dev under there later.
|
|
|
|
*/
|
|
|
|
struct test_batched_req *reqs;
|
|
|
|
int test_result;
|
|
|
|
int (*req_firmware)(const struct firmware **fw, const char *name,
|
|
|
|
struct device *device);
|
|
|
|
};
|
|
|
|
|
2018-01-11 19:13:45 +08:00
|
|
|
static struct test_config *test_fw_config;
|
2017-07-21 04:13:42 +08:00
|
|
|
|
2014-07-15 05:38:12 +08:00
|
|
|
static ssize_t test_fw_misc_read(struct file *f, char __user *buf,
|
|
|
|
size_t size, loff_t *offset)
|
|
|
|
{
|
|
|
|
ssize_t rc = 0;
|
|
|
|
|
|
|
|
mutex_lock(&test_fw_mutex);
|
|
|
|
if (test_firmware)
|
|
|
|
rc = simple_read_from_buffer(buf, size, offset,
|
|
|
|
test_firmware->data,
|
|
|
|
test_firmware->size);
|
|
|
|
mutex_unlock(&test_fw_mutex);
|
|
|
|
return rc;
|
|
|
|
}
|
|
|
|
|
|
|
|
static const struct file_operations test_fw_fops = {
|
|
|
|
.owner = THIS_MODULE,
|
|
|
|
.read = test_fw_misc_read,
|
|
|
|
};
|
|
|
|
|
2017-07-21 04:13:42 +08:00
|
|
|
static void __test_release_all_firmware(void)
|
|
|
|
{
|
|
|
|
struct test_batched_req *req;
|
|
|
|
u8 i;
|
|
|
|
|
|
|
|
if (!test_fw_config->reqs)
|
|
|
|
return;
|
|
|
|
|
|
|
|
for (i = 0; i < test_fw_config->num_requests; i++) {
|
|
|
|
req = &test_fw_config->reqs[i];
|
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: 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>
2023-05-09 16:47:49 +08:00
|
|
|
if (req->fw) {
|
|
|
|
if (req->fw_buf) {
|
|
|
|
kfree_const(req->fw_buf);
|
|
|
|
req->fw_buf = NULL;
|
|
|
|
}
|
2017-07-21 04:13:42 +08:00
|
|
|
release_firmware(req->fw);
|
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: 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>
2023-05-09 16:47:49 +08:00
|
|
|
req->fw = NULL;
|
|
|
|
}
|
2017-07-21 04:13:42 +08:00
|
|
|
}
|
|
|
|
|
|
|
|
vfree(test_fw_config->reqs);
|
|
|
|
test_fw_config->reqs = NULL;
|
|
|
|
}
|
|
|
|
|
|
|
|
static void test_release_all_firmware(void)
|
|
|
|
{
|
|
|
|
mutex_lock(&test_fw_mutex);
|
|
|
|
__test_release_all_firmware();
|
|
|
|
mutex_unlock(&test_fw_mutex);
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
static void __test_firmware_config_free(void)
|
|
|
|
{
|
|
|
|
__test_release_all_firmware();
|
|
|
|
kfree_const(test_fw_config->name);
|
|
|
|
test_fw_config->name = NULL;
|
|
|
|
}
|
|
|
|
|
|
|
|
/*
|
|
|
|
* XXX: move to kstrncpy() once merged.
|
|
|
|
*
|
|
|
|
* Users should use kfree_const() when freeing these.
|
|
|
|
*/
|
|
|
|
static int __kstrncpy(char **dst, const char *name, size_t count, gfp_t gfp)
|
|
|
|
{
|
|
|
|
*dst = kstrndup(name, count, gfp);
|
|
|
|
if (!*dst)
|
|
|
|
return -ENOSPC;
|
|
|
|
return count;
|
|
|
|
}
|
|
|
|
|
|
|
|
static int __test_firmware_config_init(void)
|
|
|
|
{
|
|
|
|
int ret;
|
|
|
|
|
|
|
|
ret = __kstrncpy(&test_fw_config->name, TEST_FIRMWARE_NAME,
|
|
|
|
strlen(TEST_FIRMWARE_NAME), GFP_KERNEL);
|
|
|
|
if (ret < 0)
|
|
|
|
goto out;
|
|
|
|
|
|
|
|
test_fw_config->num_requests = TEST_FIRMWARE_NUM_REQS;
|
|
|
|
test_fw_config->send_uevent = true;
|
2019-08-23 02:40:04 +08:00
|
|
|
test_fw_config->into_buf = false;
|
2020-10-03 01:38:28 +08:00
|
|
|
test_fw_config->buf_size = TEST_FIRMWARE_BUF_SIZE;
|
|
|
|
test_fw_config->file_offset = 0;
|
|
|
|
test_fw_config->partial = false;
|
2017-07-21 04:13:42 +08:00
|
|
|
test_fw_config->sync_direct = false;
|
|
|
|
test_fw_config->req_firmware = request_firmware;
|
|
|
|
test_fw_config->test_result = 0;
|
|
|
|
test_fw_config->reqs = NULL;
|
|
|
|
|
|
|
|
return 0;
|
|
|
|
|
|
|
|
out:
|
|
|
|
__test_firmware_config_free();
|
|
|
|
return ret;
|
|
|
|
}
|
|
|
|
|
|
|
|
static ssize_t reset_store(struct device *dev,
|
|
|
|
struct device_attribute *attr,
|
|
|
|
const char *buf, size_t count)
|
|
|
|
{
|
|
|
|
int ret;
|
|
|
|
|
|
|
|
mutex_lock(&test_fw_mutex);
|
|
|
|
|
|
|
|
__test_firmware_config_free();
|
|
|
|
|
|
|
|
ret = __test_firmware_config_init();
|
|
|
|
if (ret < 0) {
|
|
|
|
ret = -ENOMEM;
|
|
|
|
pr_err("could not alloc settings for config trigger: %d\n",
|
|
|
|
ret);
|
|
|
|
goto out;
|
|
|
|
}
|
|
|
|
|
|
|
|
pr_info("reset\n");
|
|
|
|
ret = count;
|
|
|
|
|
|
|
|
out:
|
|
|
|
mutex_unlock(&test_fw_mutex);
|
|
|
|
|
|
|
|
return ret;
|
|
|
|
}
|
|
|
|
static DEVICE_ATTR_WO(reset);
|
|
|
|
|
|
|
|
static ssize_t config_show(struct device *dev,
|
|
|
|
struct device_attribute *attr,
|
|
|
|
char *buf)
|
|
|
|
{
|
|
|
|
int len = 0;
|
|
|
|
|
|
|
|
mutex_lock(&test_fw_mutex);
|
|
|
|
|
2019-05-15 17:33:22 +08:00
|
|
|
len += scnprintf(buf, PAGE_SIZE - len,
|
2017-07-21 04:13:42 +08:00
|
|
|
"Custom trigger configuration for: %s\n",
|
|
|
|
dev_name(dev));
|
|
|
|
|
|
|
|
if (test_fw_config->name)
|
2020-10-03 01:38:28 +08:00
|
|
|
len += scnprintf(buf + len, PAGE_SIZE - len,
|
2017-07-21 04:13:42 +08:00
|
|
|
"name:\t%s\n",
|
|
|
|
test_fw_config->name);
|
|
|
|
else
|
2020-10-03 01:38:28 +08:00
|
|
|
len += scnprintf(buf + len, PAGE_SIZE - len,
|
2017-07-21 04:13:42 +08:00
|
|
|
"name:\tEMTPY\n");
|
|
|
|
|
2020-10-03 01:38:28 +08:00
|
|
|
len += scnprintf(buf + len, PAGE_SIZE - len,
|
2017-07-21 04:13:42 +08:00
|
|
|
"num_requests:\t%u\n", test_fw_config->num_requests);
|
|
|
|
|
2020-10-03 01:38:28 +08:00
|
|
|
len += scnprintf(buf + len, PAGE_SIZE - len,
|
2017-07-21 04:13:42 +08:00
|
|
|
"send_uevent:\t\t%s\n",
|
|
|
|
test_fw_config->send_uevent ?
|
2021-04-25 10:00:24 +08:00
|
|
|
"FW_ACTION_UEVENT" :
|
|
|
|
"FW_ACTION_NOUEVENT");
|
2020-10-03 01:38:28 +08:00
|
|
|
len += scnprintf(buf + len, PAGE_SIZE - len,
|
2019-08-23 02:40:04 +08:00
|
|
|
"into_buf:\t\t%s\n",
|
|
|
|
test_fw_config->into_buf ? "true" : "false");
|
2020-10-03 01:38:28 +08:00
|
|
|
len += scnprintf(buf + len, PAGE_SIZE - len,
|
|
|
|
"buf_size:\t%zu\n", test_fw_config->buf_size);
|
|
|
|
len += scnprintf(buf + len, PAGE_SIZE - len,
|
|
|
|
"file_offset:\t%zu\n", test_fw_config->file_offset);
|
|
|
|
len += scnprintf(buf + len, PAGE_SIZE - len,
|
|
|
|
"partial:\t\t%s\n",
|
|
|
|
test_fw_config->partial ? "true" : "false");
|
|
|
|
len += scnprintf(buf + len, PAGE_SIZE - len,
|
2017-07-21 04:13:42 +08:00
|
|
|
"sync_direct:\t\t%s\n",
|
|
|
|
test_fw_config->sync_direct ? "true" : "false");
|
2020-10-03 01:38:28 +08:00
|
|
|
len += scnprintf(buf + len, PAGE_SIZE - len,
|
2017-07-21 04:13:42 +08:00
|
|
|
"read_fw_idx:\t%u\n", test_fw_config->read_fw_idx);
|
|
|
|
|
|
|
|
mutex_unlock(&test_fw_mutex);
|
|
|
|
|
|
|
|
return len;
|
|
|
|
}
|
|
|
|
static DEVICE_ATTR_RO(config);
|
|
|
|
|
|
|
|
static ssize_t config_name_store(struct device *dev,
|
|
|
|
struct device_attribute *attr,
|
|
|
|
const char *buf, size_t count)
|
|
|
|
{
|
|
|
|
int ret;
|
|
|
|
|
|
|
|
mutex_lock(&test_fw_mutex);
|
|
|
|
kfree_const(test_fw_config->name);
|
|
|
|
ret = __kstrncpy(&test_fw_config->name, buf, count, GFP_KERNEL);
|
|
|
|
mutex_unlock(&test_fw_mutex);
|
|
|
|
|
|
|
|
return ret;
|
|
|
|
}
|
|
|
|
|
|
|
|
/*
|
|
|
|
* As per sysfs_kf_seq_show() the buf is max PAGE_SIZE.
|
|
|
|
*/
|
|
|
|
static ssize_t config_test_show_str(char *dst,
|
|
|
|
char *src)
|
|
|
|
{
|
|
|
|
int len;
|
|
|
|
|
|
|
|
mutex_lock(&test_fw_mutex);
|
|
|
|
len = snprintf(dst, PAGE_SIZE, "%s\n", src);
|
|
|
|
mutex_unlock(&test_fw_mutex);
|
|
|
|
|
|
|
|
return len;
|
|
|
|
}
|
|
|
|
|
test_firmware: prevent race conditions by a correct implementation of locking
[ Upstream commit 4acfe3dfde685a5a9eaec5555351918e2d7266a1 ]
Dan Carpenter spotted a race condition in a couple of situations like
these in the test_firmware driver:
static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg)
{
u8 val;
int ret;
ret = kstrtou8(buf, 10, &val);
if (ret)
return ret;
mutex_lock(&test_fw_mutex);
*(u8 *)cfg = val;
mutex_unlock(&test_fw_mutex);
/* Always return full write size even if we didn't consume all */
return size;
}
static ssize_t config_num_requests_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
int rc;
mutex_lock(&test_fw_mutex);
if (test_fw_config->reqs) {
pr_err("Must call release_all_firmware prior to changing config\n");
rc = -EINVAL;
mutex_unlock(&test_fw_mutex);
goto out;
}
mutex_unlock(&test_fw_mutex);
rc = test_dev_config_update_u8(buf, count,
&test_fw_config->num_requests);
out:
return rc;
}
static ssize_t config_read_fw_idx_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
return test_dev_config_update_u8(buf, count,
&test_fw_config->read_fw_idx);
}
The function test_dev_config_update_u8() is called from both the locked
and the unlocked context, function config_num_requests_store() and
config_read_fw_idx_store() which can both be called asynchronously as
they are driver's methods, while test_dev_config_update_u8() and siblings
change their argument pointed to by u8 *cfg or similar pointer.
To avoid deadlock on test_fw_mutex, the lock is dropped before calling
test_dev_config_update_u8() and re-acquired within test_dev_config_update_u8()
itself, but alas this creates a race condition.
Having two locks wouldn't assure a race-proof mutual exclusion.
This situation is best avoided by the introduction of a new, unlocked
function __test_dev_config_update_u8() which can be called from the locked
context and reducing test_dev_config_update_u8() to:
static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg)
{
int ret;
mutex_lock(&test_fw_mutex);
ret = __test_dev_config_update_u8(buf, size, cfg);
mutex_unlock(&test_fw_mutex);
return ret;
}
doing the locking and calling the unlocked primitive, which enables both
locked and unlocked versions without duplication of code.
The similar approach was applied to all functions called from the locked
and the unlocked context, which safely mitigates both deadlocks and race
conditions in the driver.
__test_dev_config_update_bool(), __test_dev_config_update_u8() and
__test_dev_config_update_size_t() unlocked versions of the functions
were introduced to be called from the locked contexts as a workaround
without releasing the main driver's lock and thereof causing a race
condition.
The test_dev_config_update_bool(), test_dev_config_update_u8() and
test_dev_config_update_size_t() locked versions of the functions
are being called from driver methods without the unnecessary multiplying
of the locking and unlocking code for each method, and complicating
the code with saving of the return value across lock.
Fixes: 7feebfa487b92 ("test_firmware: add support for request_firmware_into_buf")
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Russ Weight <russell.h.weight@intel.com>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Tianfei Zhang <tianfei.zhang@intel.com>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Colin Ian King <colin.i.king@gmail.com>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: linux-kselftest@vger.kernel.org
Cc: stable@vger.kernel.org # v5.4
Suggested-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
Link: https://lore.kernel.org/r/20230509084746.48259-1-mirsad.todorovac@alu.unizg.hr
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
2023-05-09 16:47:45 +08:00
|
|
|
static inline int __test_dev_config_update_bool(const char *buf, size_t size,
|
2017-07-21 04:13:42 +08:00
|
|
|
bool *cfg)
|
|
|
|
{
|
|
|
|
int ret;
|
|
|
|
|
2023-01-14 17:22:03 +08:00
|
|
|
if (kstrtobool(buf, cfg) < 0)
|
2017-07-21 04:13:42 +08:00
|
|
|
ret = -EINVAL;
|
|
|
|
else
|
|
|
|
ret = size;
|
test_firmware: prevent race conditions by a correct implementation of locking
[ Upstream commit 4acfe3dfde685a5a9eaec5555351918e2d7266a1 ]
Dan Carpenter spotted a race condition in a couple of situations like
these in the test_firmware driver:
static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg)
{
u8 val;
int ret;
ret = kstrtou8(buf, 10, &val);
if (ret)
return ret;
mutex_lock(&test_fw_mutex);
*(u8 *)cfg = val;
mutex_unlock(&test_fw_mutex);
/* Always return full write size even if we didn't consume all */
return size;
}
static ssize_t config_num_requests_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
int rc;
mutex_lock(&test_fw_mutex);
if (test_fw_config->reqs) {
pr_err("Must call release_all_firmware prior to changing config\n");
rc = -EINVAL;
mutex_unlock(&test_fw_mutex);
goto out;
}
mutex_unlock(&test_fw_mutex);
rc = test_dev_config_update_u8(buf, count,
&test_fw_config->num_requests);
out:
return rc;
}
static ssize_t config_read_fw_idx_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
return test_dev_config_update_u8(buf, count,
&test_fw_config->read_fw_idx);
}
The function test_dev_config_update_u8() is called from both the locked
and the unlocked context, function config_num_requests_store() and
config_read_fw_idx_store() which can both be called asynchronously as
they are driver's methods, while test_dev_config_update_u8() and siblings
change their argument pointed to by u8 *cfg or similar pointer.
To avoid deadlock on test_fw_mutex, the lock is dropped before calling
test_dev_config_update_u8() and re-acquired within test_dev_config_update_u8()
itself, but alas this creates a race condition.
Having two locks wouldn't assure a race-proof mutual exclusion.
This situation is best avoided by the introduction of a new, unlocked
function __test_dev_config_update_u8() which can be called from the locked
context and reducing test_dev_config_update_u8() to:
static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg)
{
int ret;
mutex_lock(&test_fw_mutex);
ret = __test_dev_config_update_u8(buf, size, cfg);
mutex_unlock(&test_fw_mutex);
return ret;
}
doing the locking and calling the unlocked primitive, which enables both
locked and unlocked versions without duplication of code.
The similar approach was applied to all functions called from the locked
and the unlocked context, which safely mitigates both deadlocks and race
conditions in the driver.
__test_dev_config_update_bool(), __test_dev_config_update_u8() and
__test_dev_config_update_size_t() unlocked versions of the functions
were introduced to be called from the locked contexts as a workaround
without releasing the main driver's lock and thereof causing a race
condition.
The test_dev_config_update_bool(), test_dev_config_update_u8() and
test_dev_config_update_size_t() locked versions of the functions
are being called from driver methods without the unnecessary multiplying
of the locking and unlocking code for each method, and complicating
the code with saving of the return value across lock.
Fixes: 7feebfa487b92 ("test_firmware: add support for request_firmware_into_buf")
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Russ Weight <russell.h.weight@intel.com>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Tianfei Zhang <tianfei.zhang@intel.com>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Colin Ian King <colin.i.king@gmail.com>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: linux-kselftest@vger.kernel.org
Cc: stable@vger.kernel.org # v5.4
Suggested-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
Link: https://lore.kernel.org/r/20230509084746.48259-1-mirsad.todorovac@alu.unizg.hr
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
2023-05-09 16:47:45 +08:00
|
|
|
|
|
|
|
return ret;
|
|
|
|
}
|
|
|
|
|
|
|
|
static int test_dev_config_update_bool(const char *buf, size_t size,
|
|
|
|
bool *cfg)
|
|
|
|
{
|
|
|
|
int ret;
|
|
|
|
|
|
|
|
mutex_lock(&test_fw_mutex);
|
|
|
|
ret = __test_dev_config_update_bool(buf, size, cfg);
|
2017-07-21 04:13:42 +08:00
|
|
|
mutex_unlock(&test_fw_mutex);
|
|
|
|
|
|
|
|
return ret;
|
|
|
|
}
|
|
|
|
|
2020-04-15 08:25:17 +08:00
|
|
|
static ssize_t test_dev_config_show_bool(char *buf, bool val)
|
2017-07-21 04:13:42 +08:00
|
|
|
{
|
|
|
|
return snprintf(buf, PAGE_SIZE, "%d\n", val);
|
|
|
|
}
|
|
|
|
|
test_firmware: prevent race conditions by a correct implementation of locking
[ Upstream commit 4acfe3dfde685a5a9eaec5555351918e2d7266a1 ]
Dan Carpenter spotted a race condition in a couple of situations like
these in the test_firmware driver:
static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg)
{
u8 val;
int ret;
ret = kstrtou8(buf, 10, &val);
if (ret)
return ret;
mutex_lock(&test_fw_mutex);
*(u8 *)cfg = val;
mutex_unlock(&test_fw_mutex);
/* Always return full write size even if we didn't consume all */
return size;
}
static ssize_t config_num_requests_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
int rc;
mutex_lock(&test_fw_mutex);
if (test_fw_config->reqs) {
pr_err("Must call release_all_firmware prior to changing config\n");
rc = -EINVAL;
mutex_unlock(&test_fw_mutex);
goto out;
}
mutex_unlock(&test_fw_mutex);
rc = test_dev_config_update_u8(buf, count,
&test_fw_config->num_requests);
out:
return rc;
}
static ssize_t config_read_fw_idx_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
return test_dev_config_update_u8(buf, count,
&test_fw_config->read_fw_idx);
}
The function test_dev_config_update_u8() is called from both the locked
and the unlocked context, function config_num_requests_store() and
config_read_fw_idx_store() which can both be called asynchronously as
they are driver's methods, while test_dev_config_update_u8() and siblings
change their argument pointed to by u8 *cfg or similar pointer.
To avoid deadlock on test_fw_mutex, the lock is dropped before calling
test_dev_config_update_u8() and re-acquired within test_dev_config_update_u8()
itself, but alas this creates a race condition.
Having two locks wouldn't assure a race-proof mutual exclusion.
This situation is best avoided by the introduction of a new, unlocked
function __test_dev_config_update_u8() which can be called from the locked
context and reducing test_dev_config_update_u8() to:
static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg)
{
int ret;
mutex_lock(&test_fw_mutex);
ret = __test_dev_config_update_u8(buf, size, cfg);
mutex_unlock(&test_fw_mutex);
return ret;
}
doing the locking and calling the unlocked primitive, which enables both
locked and unlocked versions without duplication of code.
The similar approach was applied to all functions called from the locked
and the unlocked context, which safely mitigates both deadlocks and race
conditions in the driver.
__test_dev_config_update_bool(), __test_dev_config_update_u8() and
__test_dev_config_update_size_t() unlocked versions of the functions
were introduced to be called from the locked contexts as a workaround
without releasing the main driver's lock and thereof causing a race
condition.
The test_dev_config_update_bool(), test_dev_config_update_u8() and
test_dev_config_update_size_t() locked versions of the functions
are being called from driver methods without the unnecessary multiplying
of the locking and unlocking code for each method, and complicating
the code with saving of the return value across lock.
Fixes: 7feebfa487b92 ("test_firmware: add support for request_firmware_into_buf")
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Russ Weight <russell.h.weight@intel.com>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Tianfei Zhang <tianfei.zhang@intel.com>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Colin Ian King <colin.i.king@gmail.com>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: linux-kselftest@vger.kernel.org
Cc: stable@vger.kernel.org # v5.4
Suggested-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
Link: https://lore.kernel.org/r/20230509084746.48259-1-mirsad.todorovac@alu.unizg.hr
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
2023-05-09 16:47:45 +08:00
|
|
|
static int __test_dev_config_update_size_t(
|
|
|
|
const char *buf,
|
2020-10-03 01:38:28 +08:00
|
|
|
size_t size,
|
|
|
|
size_t *cfg)
|
|
|
|
{
|
|
|
|
int ret;
|
|
|
|
long new;
|
|
|
|
|
|
|
|
ret = kstrtol(buf, 10, &new);
|
|
|
|
if (ret)
|
|
|
|
return ret;
|
|
|
|
|
|
|
|
*(size_t *)cfg = new;
|
|
|
|
|
|
|
|
/* Always return full write size even if we didn't consume all */
|
|
|
|
return size;
|
|
|
|
}
|
|
|
|
|
|
|
|
static ssize_t test_dev_config_show_size_t(char *buf, size_t val)
|
|
|
|
{
|
|
|
|
return snprintf(buf, PAGE_SIZE, "%zu\n", val);
|
|
|
|
}
|
|
|
|
|
2020-04-15 08:25:17 +08:00
|
|
|
static ssize_t test_dev_config_show_int(char *buf, int val)
|
2017-07-21 04:13:42 +08:00
|
|
|
{
|
|
|
|
return snprintf(buf, PAGE_SIZE, "%d\n", val);
|
|
|
|
}
|
|
|
|
|
test_firmware: prevent race conditions by a correct implementation of locking
[ Upstream commit 4acfe3dfde685a5a9eaec5555351918e2d7266a1 ]
Dan Carpenter spotted a race condition in a couple of situations like
these in the test_firmware driver:
static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg)
{
u8 val;
int ret;
ret = kstrtou8(buf, 10, &val);
if (ret)
return ret;
mutex_lock(&test_fw_mutex);
*(u8 *)cfg = val;
mutex_unlock(&test_fw_mutex);
/* Always return full write size even if we didn't consume all */
return size;
}
static ssize_t config_num_requests_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
int rc;
mutex_lock(&test_fw_mutex);
if (test_fw_config->reqs) {
pr_err("Must call release_all_firmware prior to changing config\n");
rc = -EINVAL;
mutex_unlock(&test_fw_mutex);
goto out;
}
mutex_unlock(&test_fw_mutex);
rc = test_dev_config_update_u8(buf, count,
&test_fw_config->num_requests);
out:
return rc;
}
static ssize_t config_read_fw_idx_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
return test_dev_config_update_u8(buf, count,
&test_fw_config->read_fw_idx);
}
The function test_dev_config_update_u8() is called from both the locked
and the unlocked context, function config_num_requests_store() and
config_read_fw_idx_store() which can both be called asynchronously as
they are driver's methods, while test_dev_config_update_u8() and siblings
change their argument pointed to by u8 *cfg or similar pointer.
To avoid deadlock on test_fw_mutex, the lock is dropped before calling
test_dev_config_update_u8() and re-acquired within test_dev_config_update_u8()
itself, but alas this creates a race condition.
Having two locks wouldn't assure a race-proof mutual exclusion.
This situation is best avoided by the introduction of a new, unlocked
function __test_dev_config_update_u8() which can be called from the locked
context and reducing test_dev_config_update_u8() to:
static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg)
{
int ret;
mutex_lock(&test_fw_mutex);
ret = __test_dev_config_update_u8(buf, size, cfg);
mutex_unlock(&test_fw_mutex);
return ret;
}
doing the locking and calling the unlocked primitive, which enables both
locked and unlocked versions without duplication of code.
The similar approach was applied to all functions called from the locked
and the unlocked context, which safely mitigates both deadlocks and race
conditions in the driver.
__test_dev_config_update_bool(), __test_dev_config_update_u8() and
__test_dev_config_update_size_t() unlocked versions of the functions
were introduced to be called from the locked contexts as a workaround
without releasing the main driver's lock and thereof causing a race
condition.
The test_dev_config_update_bool(), test_dev_config_update_u8() and
test_dev_config_update_size_t() locked versions of the functions
are being called from driver methods without the unnecessary multiplying
of the locking and unlocking code for each method, and complicating
the code with saving of the return value across lock.
Fixes: 7feebfa487b92 ("test_firmware: add support for request_firmware_into_buf")
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Russ Weight <russell.h.weight@intel.com>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Tianfei Zhang <tianfei.zhang@intel.com>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Colin Ian King <colin.i.king@gmail.com>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: linux-kselftest@vger.kernel.org
Cc: stable@vger.kernel.org # v5.4
Suggested-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
Link: https://lore.kernel.org/r/20230509084746.48259-1-mirsad.todorovac@alu.unizg.hr
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
2023-05-09 16:47:45 +08:00
|
|
|
static int __test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg)
|
2017-07-21 04:13:42 +08:00
|
|
|
{
|
2020-12-16 12:44:00 +08:00
|
|
|
u8 val;
|
2017-07-21 04:13:42 +08:00
|
|
|
int ret;
|
|
|
|
|
2020-12-16 12:44:00 +08:00
|
|
|
ret = kstrtou8(buf, 10, &val);
|
2017-07-21 04:13:42 +08:00
|
|
|
if (ret)
|
|
|
|
return ret;
|
|
|
|
|
2020-12-16 12:44:00 +08:00
|
|
|
*(u8 *)cfg = val;
|
2017-07-21 04:13:42 +08:00
|
|
|
|
|
|
|
/* Always return full write size even if we didn't consume all */
|
|
|
|
return size;
|
|
|
|
}
|
|
|
|
|
test_firmware: prevent race conditions by a correct implementation of locking
[ Upstream commit 4acfe3dfde685a5a9eaec5555351918e2d7266a1 ]
Dan Carpenter spotted a race condition in a couple of situations like
these in the test_firmware driver:
static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg)
{
u8 val;
int ret;
ret = kstrtou8(buf, 10, &val);
if (ret)
return ret;
mutex_lock(&test_fw_mutex);
*(u8 *)cfg = val;
mutex_unlock(&test_fw_mutex);
/* Always return full write size even if we didn't consume all */
return size;
}
static ssize_t config_num_requests_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
int rc;
mutex_lock(&test_fw_mutex);
if (test_fw_config->reqs) {
pr_err("Must call release_all_firmware prior to changing config\n");
rc = -EINVAL;
mutex_unlock(&test_fw_mutex);
goto out;
}
mutex_unlock(&test_fw_mutex);
rc = test_dev_config_update_u8(buf, count,
&test_fw_config->num_requests);
out:
return rc;
}
static ssize_t config_read_fw_idx_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
return test_dev_config_update_u8(buf, count,
&test_fw_config->read_fw_idx);
}
The function test_dev_config_update_u8() is called from both the locked
and the unlocked context, function config_num_requests_store() and
config_read_fw_idx_store() which can both be called asynchronously as
they are driver's methods, while test_dev_config_update_u8() and siblings
change their argument pointed to by u8 *cfg or similar pointer.
To avoid deadlock on test_fw_mutex, the lock is dropped before calling
test_dev_config_update_u8() and re-acquired within test_dev_config_update_u8()
itself, but alas this creates a race condition.
Having two locks wouldn't assure a race-proof mutual exclusion.
This situation is best avoided by the introduction of a new, unlocked
function __test_dev_config_update_u8() which can be called from the locked
context and reducing test_dev_config_update_u8() to:
static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg)
{
int ret;
mutex_lock(&test_fw_mutex);
ret = __test_dev_config_update_u8(buf, size, cfg);
mutex_unlock(&test_fw_mutex);
return ret;
}
doing the locking and calling the unlocked primitive, which enables both
locked and unlocked versions without duplication of code.
The similar approach was applied to all functions called from the locked
and the unlocked context, which safely mitigates both deadlocks and race
conditions in the driver.
__test_dev_config_update_bool(), __test_dev_config_update_u8() and
__test_dev_config_update_size_t() unlocked versions of the functions
were introduced to be called from the locked contexts as a workaround
without releasing the main driver's lock and thereof causing a race
condition.
The test_dev_config_update_bool(), test_dev_config_update_u8() and
test_dev_config_update_size_t() locked versions of the functions
are being called from driver methods without the unnecessary multiplying
of the locking and unlocking code for each method, and complicating
the code with saving of the return value across lock.
Fixes: 7feebfa487b92 ("test_firmware: add support for request_firmware_into_buf")
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Russ Weight <russell.h.weight@intel.com>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Tianfei Zhang <tianfei.zhang@intel.com>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Colin Ian King <colin.i.king@gmail.com>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: linux-kselftest@vger.kernel.org
Cc: stable@vger.kernel.org # v5.4
Suggested-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
Link: https://lore.kernel.org/r/20230509084746.48259-1-mirsad.todorovac@alu.unizg.hr
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
2023-05-09 16:47:45 +08:00
|
|
|
static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg)
|
|
|
|
{
|
|
|
|
int ret;
|
|
|
|
|
|
|
|
mutex_lock(&test_fw_mutex);
|
|
|
|
ret = __test_dev_config_update_u8(buf, size, cfg);
|
|
|
|
mutex_unlock(&test_fw_mutex);
|
|
|
|
|
|
|
|
return ret;
|
|
|
|
}
|
|
|
|
|
2020-04-15 08:25:17 +08:00
|
|
|
static ssize_t test_dev_config_show_u8(char *buf, u8 val)
|
2017-07-21 04:13:42 +08:00
|
|
|
{
|
|
|
|
return snprintf(buf, PAGE_SIZE, "%u\n", val);
|
|
|
|
}
|
|
|
|
|
|
|
|
static ssize_t config_name_show(struct device *dev,
|
|
|
|
struct device_attribute *attr,
|
|
|
|
char *buf)
|
|
|
|
{
|
|
|
|
return config_test_show_str(buf, test_fw_config->name);
|
|
|
|
}
|
2017-12-20 02:15:07 +08:00
|
|
|
static DEVICE_ATTR_RW(config_name);
|
2017-07-21 04:13:42 +08:00
|
|
|
|
|
|
|
static ssize_t config_num_requests_store(struct device *dev,
|
|
|
|
struct device_attribute *attr,
|
|
|
|
const char *buf, size_t count)
|
|
|
|
{
|
|
|
|
int rc;
|
|
|
|
|
|
|
|
mutex_lock(&test_fw_mutex);
|
|
|
|
if (test_fw_config->reqs) {
|
|
|
|
pr_err("Must call release_all_firmware prior to changing config\n");
|
|
|
|
rc = -EINVAL;
|
2018-01-11 19:12:55 +08:00
|
|
|
mutex_unlock(&test_fw_mutex);
|
2017-07-21 04:13:42 +08:00
|
|
|
goto out;
|
|
|
|
}
|
|
|
|
|
test_firmware: prevent race conditions by a correct implementation of locking
[ Upstream commit 4acfe3dfde685a5a9eaec5555351918e2d7266a1 ]
Dan Carpenter spotted a race condition in a couple of situations like
these in the test_firmware driver:
static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg)
{
u8 val;
int ret;
ret = kstrtou8(buf, 10, &val);
if (ret)
return ret;
mutex_lock(&test_fw_mutex);
*(u8 *)cfg = val;
mutex_unlock(&test_fw_mutex);
/* Always return full write size even if we didn't consume all */
return size;
}
static ssize_t config_num_requests_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
int rc;
mutex_lock(&test_fw_mutex);
if (test_fw_config->reqs) {
pr_err("Must call release_all_firmware prior to changing config\n");
rc = -EINVAL;
mutex_unlock(&test_fw_mutex);
goto out;
}
mutex_unlock(&test_fw_mutex);
rc = test_dev_config_update_u8(buf, count,
&test_fw_config->num_requests);
out:
return rc;
}
static ssize_t config_read_fw_idx_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
return test_dev_config_update_u8(buf, count,
&test_fw_config->read_fw_idx);
}
The function test_dev_config_update_u8() is called from both the locked
and the unlocked context, function config_num_requests_store() and
config_read_fw_idx_store() which can both be called asynchronously as
they are driver's methods, while test_dev_config_update_u8() and siblings
change their argument pointed to by u8 *cfg or similar pointer.
To avoid deadlock on test_fw_mutex, the lock is dropped before calling
test_dev_config_update_u8() and re-acquired within test_dev_config_update_u8()
itself, but alas this creates a race condition.
Having two locks wouldn't assure a race-proof mutual exclusion.
This situation is best avoided by the introduction of a new, unlocked
function __test_dev_config_update_u8() which can be called from the locked
context and reducing test_dev_config_update_u8() to:
static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg)
{
int ret;
mutex_lock(&test_fw_mutex);
ret = __test_dev_config_update_u8(buf, size, cfg);
mutex_unlock(&test_fw_mutex);
return ret;
}
doing the locking and calling the unlocked primitive, which enables both
locked and unlocked versions without duplication of code.
The similar approach was applied to all functions called from the locked
and the unlocked context, which safely mitigates both deadlocks and race
conditions in the driver.
__test_dev_config_update_bool(), __test_dev_config_update_u8() and
__test_dev_config_update_size_t() unlocked versions of the functions
were introduced to be called from the locked contexts as a workaround
without releasing the main driver's lock and thereof causing a race
condition.
The test_dev_config_update_bool(), test_dev_config_update_u8() and
test_dev_config_update_size_t() locked versions of the functions
are being called from driver methods without the unnecessary multiplying
of the locking and unlocking code for each method, and complicating
the code with saving of the return value across lock.
Fixes: 7feebfa487b92 ("test_firmware: add support for request_firmware_into_buf")
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Russ Weight <russell.h.weight@intel.com>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Tianfei Zhang <tianfei.zhang@intel.com>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Colin Ian King <colin.i.king@gmail.com>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: linux-kselftest@vger.kernel.org
Cc: stable@vger.kernel.org # v5.4
Suggested-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
Link: https://lore.kernel.org/r/20230509084746.48259-1-mirsad.todorovac@alu.unizg.hr
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
2023-05-09 16:47:45 +08:00
|
|
|
rc = __test_dev_config_update_u8(buf, count,
|
|
|
|
&test_fw_config->num_requests);
|
|
|
|
mutex_unlock(&test_fw_mutex);
|
2017-07-21 04:13:42 +08:00
|
|
|
|
|
|
|
out:
|
|
|
|
return rc;
|
|
|
|
}
|
|
|
|
|
|
|
|
static ssize_t config_num_requests_show(struct device *dev,
|
|
|
|
struct device_attribute *attr,
|
|
|
|
char *buf)
|
|
|
|
{
|
|
|
|
return test_dev_config_show_u8(buf, test_fw_config->num_requests);
|
|
|
|
}
|
2017-12-20 02:15:07 +08:00
|
|
|
static DEVICE_ATTR_RW(config_num_requests);
|
2017-07-21 04:13:42 +08:00
|
|
|
|
2019-08-23 02:40:04 +08:00
|
|
|
static ssize_t config_into_buf_store(struct device *dev,
|
|
|
|
struct device_attribute *attr,
|
|
|
|
const char *buf, size_t count)
|
|
|
|
{
|
|
|
|
return test_dev_config_update_bool(buf,
|
|
|
|
count,
|
|
|
|
&test_fw_config->into_buf);
|
|
|
|
}
|
|
|
|
|
|
|
|
static ssize_t config_into_buf_show(struct device *dev,
|
|
|
|
struct device_attribute *attr,
|
|
|
|
char *buf)
|
|
|
|
{
|
|
|
|
return test_dev_config_show_bool(buf, test_fw_config->into_buf);
|
|
|
|
}
|
|
|
|
static DEVICE_ATTR_RW(config_into_buf);
|
|
|
|
|
2020-10-03 01:38:28 +08:00
|
|
|
static ssize_t config_buf_size_store(struct device *dev,
|
|
|
|
struct device_attribute *attr,
|
|
|
|
const char *buf, size_t count)
|
|
|
|
{
|
|
|
|
int rc;
|
|
|
|
|
|
|
|
mutex_lock(&test_fw_mutex);
|
|
|
|
if (test_fw_config->reqs) {
|
|
|
|
pr_err("Must call release_all_firmware prior to changing config\n");
|
|
|
|
rc = -EINVAL;
|
|
|
|
mutex_unlock(&test_fw_mutex);
|
|
|
|
goto out;
|
|
|
|
}
|
|
|
|
|
test_firmware: prevent race conditions by a correct implementation of locking
[ Upstream commit 4acfe3dfde685a5a9eaec5555351918e2d7266a1 ]
Dan Carpenter spotted a race condition in a couple of situations like
these in the test_firmware driver:
static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg)
{
u8 val;
int ret;
ret = kstrtou8(buf, 10, &val);
if (ret)
return ret;
mutex_lock(&test_fw_mutex);
*(u8 *)cfg = val;
mutex_unlock(&test_fw_mutex);
/* Always return full write size even if we didn't consume all */
return size;
}
static ssize_t config_num_requests_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
int rc;
mutex_lock(&test_fw_mutex);
if (test_fw_config->reqs) {
pr_err("Must call release_all_firmware prior to changing config\n");
rc = -EINVAL;
mutex_unlock(&test_fw_mutex);
goto out;
}
mutex_unlock(&test_fw_mutex);
rc = test_dev_config_update_u8(buf, count,
&test_fw_config->num_requests);
out:
return rc;
}
static ssize_t config_read_fw_idx_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
return test_dev_config_update_u8(buf, count,
&test_fw_config->read_fw_idx);
}
The function test_dev_config_update_u8() is called from both the locked
and the unlocked context, function config_num_requests_store() and
config_read_fw_idx_store() which can both be called asynchronously as
they are driver's methods, while test_dev_config_update_u8() and siblings
change their argument pointed to by u8 *cfg or similar pointer.
To avoid deadlock on test_fw_mutex, the lock is dropped before calling
test_dev_config_update_u8() and re-acquired within test_dev_config_update_u8()
itself, but alas this creates a race condition.
Having two locks wouldn't assure a race-proof mutual exclusion.
This situation is best avoided by the introduction of a new, unlocked
function __test_dev_config_update_u8() which can be called from the locked
context and reducing test_dev_config_update_u8() to:
static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg)
{
int ret;
mutex_lock(&test_fw_mutex);
ret = __test_dev_config_update_u8(buf, size, cfg);
mutex_unlock(&test_fw_mutex);
return ret;
}
doing the locking and calling the unlocked primitive, which enables both
locked and unlocked versions without duplication of code.
The similar approach was applied to all functions called from the locked
and the unlocked context, which safely mitigates both deadlocks and race
conditions in the driver.
__test_dev_config_update_bool(), __test_dev_config_update_u8() and
__test_dev_config_update_size_t() unlocked versions of the functions
were introduced to be called from the locked contexts as a workaround
without releasing the main driver's lock and thereof causing a race
condition.
The test_dev_config_update_bool(), test_dev_config_update_u8() and
test_dev_config_update_size_t() locked versions of the functions
are being called from driver methods without the unnecessary multiplying
of the locking and unlocking code for each method, and complicating
the code with saving of the return value across lock.
Fixes: 7feebfa487b92 ("test_firmware: add support for request_firmware_into_buf")
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Russ Weight <russell.h.weight@intel.com>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Tianfei Zhang <tianfei.zhang@intel.com>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Colin Ian King <colin.i.king@gmail.com>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: linux-kselftest@vger.kernel.org
Cc: stable@vger.kernel.org # v5.4
Suggested-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
Link: https://lore.kernel.org/r/20230509084746.48259-1-mirsad.todorovac@alu.unizg.hr
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
2023-05-09 16:47:45 +08:00
|
|
|
rc = __test_dev_config_update_size_t(buf, count,
|
|
|
|
&test_fw_config->buf_size);
|
|
|
|
mutex_unlock(&test_fw_mutex);
|
2020-10-03 01:38:28 +08:00
|
|
|
|
|
|
|
out:
|
|
|
|
return rc;
|
|
|
|
}
|
|
|
|
|
|
|
|
static ssize_t config_buf_size_show(struct device *dev,
|
|
|
|
struct device_attribute *attr,
|
|
|
|
char *buf)
|
|
|
|
{
|
|
|
|
return test_dev_config_show_size_t(buf, test_fw_config->buf_size);
|
|
|
|
}
|
|
|
|
static DEVICE_ATTR_RW(config_buf_size);
|
|
|
|
|
|
|
|
static ssize_t config_file_offset_store(struct device *dev,
|
|
|
|
struct device_attribute *attr,
|
|
|
|
const char *buf, size_t count)
|
|
|
|
{
|
|
|
|
int rc;
|
|
|
|
|
|
|
|
mutex_lock(&test_fw_mutex);
|
|
|
|
if (test_fw_config->reqs) {
|
|
|
|
pr_err("Must call release_all_firmware prior to changing config\n");
|
|
|
|
rc = -EINVAL;
|
|
|
|
mutex_unlock(&test_fw_mutex);
|
|
|
|
goto out;
|
|
|
|
}
|
|
|
|
|
test_firmware: prevent race conditions by a correct implementation of locking
[ Upstream commit 4acfe3dfde685a5a9eaec5555351918e2d7266a1 ]
Dan Carpenter spotted a race condition in a couple of situations like
these in the test_firmware driver:
static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg)
{
u8 val;
int ret;
ret = kstrtou8(buf, 10, &val);
if (ret)
return ret;
mutex_lock(&test_fw_mutex);
*(u8 *)cfg = val;
mutex_unlock(&test_fw_mutex);
/* Always return full write size even if we didn't consume all */
return size;
}
static ssize_t config_num_requests_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
int rc;
mutex_lock(&test_fw_mutex);
if (test_fw_config->reqs) {
pr_err("Must call release_all_firmware prior to changing config\n");
rc = -EINVAL;
mutex_unlock(&test_fw_mutex);
goto out;
}
mutex_unlock(&test_fw_mutex);
rc = test_dev_config_update_u8(buf, count,
&test_fw_config->num_requests);
out:
return rc;
}
static ssize_t config_read_fw_idx_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
return test_dev_config_update_u8(buf, count,
&test_fw_config->read_fw_idx);
}
The function test_dev_config_update_u8() is called from both the locked
and the unlocked context, function config_num_requests_store() and
config_read_fw_idx_store() which can both be called asynchronously as
they are driver's methods, while test_dev_config_update_u8() and siblings
change their argument pointed to by u8 *cfg or similar pointer.
To avoid deadlock on test_fw_mutex, the lock is dropped before calling
test_dev_config_update_u8() and re-acquired within test_dev_config_update_u8()
itself, but alas this creates a race condition.
Having two locks wouldn't assure a race-proof mutual exclusion.
This situation is best avoided by the introduction of a new, unlocked
function __test_dev_config_update_u8() which can be called from the locked
context and reducing test_dev_config_update_u8() to:
static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg)
{
int ret;
mutex_lock(&test_fw_mutex);
ret = __test_dev_config_update_u8(buf, size, cfg);
mutex_unlock(&test_fw_mutex);
return ret;
}
doing the locking and calling the unlocked primitive, which enables both
locked and unlocked versions without duplication of code.
The similar approach was applied to all functions called from the locked
and the unlocked context, which safely mitigates both deadlocks and race
conditions in the driver.
__test_dev_config_update_bool(), __test_dev_config_update_u8() and
__test_dev_config_update_size_t() unlocked versions of the functions
were introduced to be called from the locked contexts as a workaround
without releasing the main driver's lock and thereof causing a race
condition.
The test_dev_config_update_bool(), test_dev_config_update_u8() and
test_dev_config_update_size_t() locked versions of the functions
are being called from driver methods without the unnecessary multiplying
of the locking and unlocking code for each method, and complicating
the code with saving of the return value across lock.
Fixes: 7feebfa487b92 ("test_firmware: add support for request_firmware_into_buf")
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Russ Weight <russell.h.weight@intel.com>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Tianfei Zhang <tianfei.zhang@intel.com>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Colin Ian King <colin.i.king@gmail.com>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: linux-kselftest@vger.kernel.org
Cc: stable@vger.kernel.org # v5.4
Suggested-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
Link: https://lore.kernel.org/r/20230509084746.48259-1-mirsad.todorovac@alu.unizg.hr
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
2023-05-09 16:47:45 +08:00
|
|
|
rc = __test_dev_config_update_size_t(buf, count,
|
|
|
|
&test_fw_config->file_offset);
|
|
|
|
mutex_unlock(&test_fw_mutex);
|
2020-10-03 01:38:28 +08:00
|
|
|
|
|
|
|
out:
|
|
|
|
return rc;
|
|
|
|
}
|
|
|
|
|
|
|
|
static ssize_t config_file_offset_show(struct device *dev,
|
|
|
|
struct device_attribute *attr,
|
|
|
|
char *buf)
|
|
|
|
{
|
|
|
|
return test_dev_config_show_size_t(buf, test_fw_config->file_offset);
|
|
|
|
}
|
|
|
|
static DEVICE_ATTR_RW(config_file_offset);
|
|
|
|
|
|
|
|
static ssize_t config_partial_store(struct device *dev,
|
|
|
|
struct device_attribute *attr,
|
|
|
|
const char *buf, size_t count)
|
|
|
|
{
|
|
|
|
return test_dev_config_update_bool(buf,
|
|
|
|
count,
|
|
|
|
&test_fw_config->partial);
|
|
|
|
}
|
|
|
|
|
|
|
|
static ssize_t config_partial_show(struct device *dev,
|
|
|
|
struct device_attribute *attr,
|
|
|
|
char *buf)
|
|
|
|
{
|
|
|
|
return test_dev_config_show_bool(buf, test_fw_config->partial);
|
|
|
|
}
|
|
|
|
static DEVICE_ATTR_RW(config_partial);
|
|
|
|
|
2017-07-21 04:13:42 +08:00
|
|
|
static ssize_t config_sync_direct_store(struct device *dev,
|
|
|
|
struct device_attribute *attr,
|
|
|
|
const char *buf, size_t count)
|
|
|
|
{
|
|
|
|
int rc = test_dev_config_update_bool(buf, count,
|
|
|
|
&test_fw_config->sync_direct);
|
|
|
|
|
|
|
|
if (rc == count)
|
|
|
|
test_fw_config->req_firmware = test_fw_config->sync_direct ?
|
|
|
|
request_firmware_direct :
|
|
|
|
request_firmware;
|
|
|
|
return rc;
|
|
|
|
}
|
|
|
|
|
|
|
|
static ssize_t config_sync_direct_show(struct device *dev,
|
|
|
|
struct device_attribute *attr,
|
|
|
|
char *buf)
|
|
|
|
{
|
|
|
|
return test_dev_config_show_bool(buf, test_fw_config->sync_direct);
|
|
|
|
}
|
2017-12-20 02:15:07 +08:00
|
|
|
static DEVICE_ATTR_RW(config_sync_direct);
|
2017-07-21 04:13:42 +08:00
|
|
|
|
|
|
|
static ssize_t config_send_uevent_store(struct device *dev,
|
|
|
|
struct device_attribute *attr,
|
|
|
|
const char *buf, size_t count)
|
|
|
|
{
|
|
|
|
return test_dev_config_update_bool(buf, count,
|
|
|
|
&test_fw_config->send_uevent);
|
|
|
|
}
|
|
|
|
|
|
|
|
static ssize_t config_send_uevent_show(struct device *dev,
|
|
|
|
struct device_attribute *attr,
|
|
|
|
char *buf)
|
|
|
|
{
|
|
|
|
return test_dev_config_show_bool(buf, test_fw_config->send_uevent);
|
|
|
|
}
|
2017-12-20 02:15:07 +08:00
|
|
|
static DEVICE_ATTR_RW(config_send_uevent);
|
2017-07-21 04:13:42 +08:00
|
|
|
|
|
|
|
static ssize_t config_read_fw_idx_store(struct device *dev,
|
|
|
|
struct device_attribute *attr,
|
|
|
|
const char *buf, size_t count)
|
|
|
|
{
|
|
|
|
return test_dev_config_update_u8(buf, count,
|
|
|
|
&test_fw_config->read_fw_idx);
|
|
|
|
}
|
|
|
|
|
|
|
|
static ssize_t config_read_fw_idx_show(struct device *dev,
|
|
|
|
struct device_attribute *attr,
|
|
|
|
char *buf)
|
|
|
|
{
|
|
|
|
return test_dev_config_show_u8(buf, test_fw_config->read_fw_idx);
|
|
|
|
}
|
2017-12-20 02:15:07 +08:00
|
|
|
static DEVICE_ATTR_RW(config_read_fw_idx);
|
2017-07-21 04:13:42 +08:00
|
|
|
|
|
|
|
|
2014-07-15 05:38:12 +08:00
|
|
|
static ssize_t trigger_request_store(struct device *dev,
|
|
|
|
struct device_attribute *attr,
|
|
|
|
const char *buf, size_t count)
|
|
|
|
{
|
|
|
|
int rc;
|
|
|
|
char *name;
|
|
|
|
|
2015-12-10 06:50:26 +08:00
|
|
|
name = kstrndup(buf, count, GFP_KERNEL);
|
2014-07-15 05:38:12 +08:00
|
|
|
if (!name)
|
|
|
|
return -ENOSPC;
|
|
|
|
|
|
|
|
pr_info("loading '%s'\n", name);
|
|
|
|
|
|
|
|
mutex_lock(&test_fw_mutex);
|
|
|
|
release_firmware(test_firmware);
|
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: 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>
2023-05-09 16:47:49 +08:00
|
|
|
if (test_fw_config->reqs)
|
|
|
|
__test_release_all_firmware();
|
2014-07-15 05:38:12 +08:00
|
|
|
test_firmware = NULL;
|
|
|
|
rc = request_firmware(&test_firmware, name, dev);
|
2015-12-10 06:50:25 +08:00
|
|
|
if (rc) {
|
2014-07-15 05:38:12 +08:00
|
|
|
pr_info("load of '%s' failed: %d\n", name, rc);
|
2015-12-10 06:50:25 +08:00
|
|
|
goto out;
|
|
|
|
}
|
|
|
|
pr_info("loaded: %zu\n", test_firmware->size);
|
|
|
|
rc = count;
|
|
|
|
|
|
|
|
out:
|
2014-07-15 05:38:12 +08:00
|
|
|
mutex_unlock(&test_fw_mutex);
|
|
|
|
|
|
|
|
kfree(name);
|
|
|
|
|
2015-12-10 06:50:25 +08:00
|
|
|
return rc;
|
2014-07-15 05:38:12 +08:00
|
|
|
}
|
|
|
|
static DEVICE_ATTR_WO(trigger_request);
|
|
|
|
|
2020-01-16 00:35:49 +08:00
|
|
|
#ifdef CONFIG_EFI_EMBEDDED_FIRMWARE
|
2020-09-10 06:53:54 +08:00
|
|
|
extern struct list_head efi_embedded_fw_list;
|
|
|
|
extern bool efi_embedded_fw_checked;
|
|
|
|
|
2020-01-16 00:35:49 +08:00
|
|
|
static ssize_t trigger_request_platform_store(struct device *dev,
|
|
|
|
struct device_attribute *attr,
|
|
|
|
const char *buf, size_t count)
|
|
|
|
{
|
|
|
|
static const u8 test_data[] = {
|
|
|
|
0x55, 0xaa, 0x55, 0xaa, 0x01, 0x02, 0x03, 0x04,
|
|
|
|
0x55, 0xaa, 0x55, 0xaa, 0x05, 0x06, 0x07, 0x08,
|
|
|
|
0x55, 0xaa, 0x55, 0xaa, 0x10, 0x20, 0x30, 0x40,
|
|
|
|
0x55, 0xaa, 0x55, 0xaa, 0x50, 0x60, 0x70, 0x80
|
|
|
|
};
|
|
|
|
struct efi_embedded_fw efi_embedded_fw;
|
|
|
|
const struct firmware *firmware = NULL;
|
2020-09-10 06:53:54 +08:00
|
|
|
bool saved_efi_embedded_fw_checked;
|
2020-01-16 00:35:49 +08:00
|
|
|
char *name;
|
|
|
|
int rc;
|
|
|
|
|
|
|
|
name = kstrndup(buf, count, GFP_KERNEL);
|
|
|
|
if (!name)
|
|
|
|
return -ENOSPC;
|
|
|
|
|
|
|
|
pr_info("inserting test platform fw '%s'\n", name);
|
|
|
|
efi_embedded_fw.name = name;
|
|
|
|
efi_embedded_fw.data = (void *)test_data;
|
|
|
|
efi_embedded_fw.length = sizeof(test_data);
|
|
|
|
list_add(&efi_embedded_fw.list, &efi_embedded_fw_list);
|
2020-09-10 06:53:54 +08:00
|
|
|
saved_efi_embedded_fw_checked = efi_embedded_fw_checked;
|
|
|
|
efi_embedded_fw_checked = true;
|
2020-01-16 00:35:49 +08:00
|
|
|
|
|
|
|
pr_info("loading '%s'\n", name);
|
|
|
|
rc = firmware_request_platform(&firmware, name, dev);
|
|
|
|
if (rc) {
|
|
|
|
pr_info("load of '%s' failed: %d\n", name, rc);
|
|
|
|
goto out;
|
|
|
|
}
|
|
|
|
if (firmware->size != sizeof(test_data) ||
|
|
|
|
memcmp(firmware->data, test_data, sizeof(test_data)) != 0) {
|
|
|
|
pr_info("firmware contents mismatch for '%s'\n", name);
|
|
|
|
rc = -EINVAL;
|
|
|
|
goto out;
|
|
|
|
}
|
|
|
|
pr_info("loaded: %zu\n", firmware->size);
|
|
|
|
rc = count;
|
|
|
|
|
|
|
|
out:
|
2020-09-10 06:53:54 +08:00
|
|
|
efi_embedded_fw_checked = saved_efi_embedded_fw_checked;
|
2020-01-16 00:35:49 +08:00
|
|
|
release_firmware(firmware);
|
|
|
|
list_del(&efi_embedded_fw.list);
|
|
|
|
kfree(name);
|
|
|
|
|
|
|
|
return rc;
|
|
|
|
}
|
|
|
|
static DEVICE_ATTR_WO(trigger_request_platform);
|
|
|
|
#endif
|
|
|
|
|
2015-12-10 06:50:27 +08:00
|
|
|
static DECLARE_COMPLETION(async_fw_done);
|
|
|
|
|
|
|
|
static void trigger_async_request_cb(const struct firmware *fw, void *context)
|
|
|
|
{
|
|
|
|
test_firmware = fw;
|
|
|
|
complete(&async_fw_done);
|
|
|
|
}
|
|
|
|
|
|
|
|
static ssize_t trigger_async_request_store(struct device *dev,
|
|
|
|
struct device_attribute *attr,
|
|
|
|
const char *buf, size_t count)
|
|
|
|
{
|
|
|
|
int rc;
|
|
|
|
char *name;
|
|
|
|
|
|
|
|
name = kstrndup(buf, count, GFP_KERNEL);
|
|
|
|
if (!name)
|
|
|
|
return -ENOSPC;
|
|
|
|
|
|
|
|
pr_info("loading '%s'\n", name);
|
|
|
|
|
|
|
|
mutex_lock(&test_fw_mutex);
|
|
|
|
release_firmware(test_firmware);
|
|
|
|
test_firmware = NULL;
|
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: 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>
2023-05-09 16:47:49 +08:00
|
|
|
if (test_fw_config->reqs)
|
|
|
|
__test_release_all_firmware();
|
2015-12-10 06:50:27 +08:00
|
|
|
rc = request_firmware_nowait(THIS_MODULE, 1, name, dev, GFP_KERNEL,
|
|
|
|
NULL, trigger_async_request_cb);
|
|
|
|
if (rc) {
|
|
|
|
pr_info("async load of '%s' failed: %d\n", name, rc);
|
|
|
|
kfree(name);
|
|
|
|
goto out;
|
|
|
|
}
|
|
|
|
/* Free 'name' ASAP, to test for race conditions */
|
|
|
|
kfree(name);
|
|
|
|
|
|
|
|
wait_for_completion(&async_fw_done);
|
|
|
|
|
|
|
|
if (test_firmware) {
|
|
|
|
pr_info("loaded: %zu\n", test_firmware->size);
|
|
|
|
rc = count;
|
|
|
|
} else {
|
|
|
|
pr_err("failed to async load firmware\n");
|
2019-08-23 02:40:04 +08:00
|
|
|
rc = -ENOMEM;
|
2015-12-10 06:50:27 +08:00
|
|
|
}
|
|
|
|
|
|
|
|
out:
|
|
|
|
mutex_unlock(&test_fw_mutex);
|
|
|
|
|
|
|
|
return rc;
|
|
|
|
}
|
|
|
|
static DEVICE_ATTR_WO(trigger_async_request);
|
|
|
|
|
2017-01-24 00:11:10 +08:00
|
|
|
static ssize_t trigger_custom_fallback_store(struct device *dev,
|
|
|
|
struct device_attribute *attr,
|
|
|
|
const char *buf, size_t count)
|
|
|
|
{
|
|
|
|
int rc;
|
|
|
|
char *name;
|
|
|
|
|
|
|
|
name = kstrndup(buf, count, GFP_KERNEL);
|
|
|
|
if (!name)
|
|
|
|
return -ENOSPC;
|
|
|
|
|
|
|
|
pr_info("loading '%s' using custom fallback mechanism\n", name);
|
|
|
|
|
|
|
|
mutex_lock(&test_fw_mutex);
|
|
|
|
release_firmware(test_firmware);
|
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: 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>
2023-05-09 16:47:49 +08:00
|
|
|
if (test_fw_config->reqs)
|
|
|
|
__test_release_all_firmware();
|
2017-01-24 00:11:10 +08:00
|
|
|
test_firmware = NULL;
|
2021-04-25 10:00:24 +08:00
|
|
|
rc = request_firmware_nowait(THIS_MODULE, FW_ACTION_NOUEVENT, name,
|
2017-01-24 00:11:10 +08:00
|
|
|
dev, GFP_KERNEL, NULL,
|
|
|
|
trigger_async_request_cb);
|
|
|
|
if (rc) {
|
|
|
|
pr_info("async load of '%s' failed: %d\n", name, rc);
|
|
|
|
kfree(name);
|
|
|
|
goto out;
|
|
|
|
}
|
|
|
|
/* Free 'name' ASAP, to test for race conditions */
|
|
|
|
kfree(name);
|
|
|
|
|
|
|
|
wait_for_completion(&async_fw_done);
|
|
|
|
|
|
|
|
if (test_firmware) {
|
|
|
|
pr_info("loaded: %zu\n", test_firmware->size);
|
|
|
|
rc = count;
|
|
|
|
} else {
|
|
|
|
pr_err("failed to async load firmware\n");
|
|
|
|
rc = -ENODEV;
|
|
|
|
}
|
|
|
|
|
|
|
|
out:
|
|
|
|
mutex_unlock(&test_fw_mutex);
|
|
|
|
|
|
|
|
return rc;
|
|
|
|
}
|
|
|
|
static DEVICE_ATTR_WO(trigger_custom_fallback);
|
|
|
|
|
2017-07-21 04:13:42 +08:00
|
|
|
static int test_fw_run_batch_request(void *data)
|
|
|
|
{
|
|
|
|
struct test_batched_req *req = data;
|
|
|
|
|
|
|
|
if (!req) {
|
|
|
|
test_fw_config->test_result = -EINVAL;
|
|
|
|
return -EINVAL;
|
|
|
|
}
|
|
|
|
|
2019-08-23 02:40:04 +08:00
|
|
|
if (test_fw_config->into_buf) {
|
|
|
|
void *test_buf;
|
|
|
|
|
|
|
|
test_buf = kzalloc(TEST_FIRMWARE_BUF_SIZE, GFP_KERNEL);
|
|
|
|
if (!test_buf)
|
|
|
|
return -ENOSPC;
|
|
|
|
|
2020-10-03 01:38:28 +08:00
|
|
|
if (test_fw_config->partial)
|
|
|
|
req->rc = request_partial_firmware_into_buf
|
|
|
|
(&req->fw,
|
|
|
|
req->name,
|
|
|
|
req->dev,
|
|
|
|
test_buf,
|
|
|
|
test_fw_config->buf_size,
|
|
|
|
test_fw_config->file_offset);
|
|
|
|
else
|
|
|
|
req->rc = request_firmware_into_buf
|
|
|
|
(&req->fw,
|
|
|
|
req->name,
|
|
|
|
req->dev,
|
|
|
|
test_buf,
|
|
|
|
test_fw_config->buf_size);
|
2019-08-23 02:40:04 +08:00
|
|
|
if (!req->fw)
|
|
|
|
kfree(test_buf);
|
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: 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>
2023-05-09 16:47:49 +08:00
|
|
|
else
|
|
|
|
req->fw_buf = test_buf;
|
2019-08-23 02:40:04 +08:00
|
|
|
} else {
|
|
|
|
req->rc = test_fw_config->req_firmware(&req->fw,
|
|
|
|
req->name,
|
|
|
|
req->dev);
|
|
|
|
}
|
|
|
|
|
2017-07-21 04:13:42 +08:00
|
|
|
if (req->rc) {
|
|
|
|
pr_info("#%u: batched sync load failed: %d\n",
|
|
|
|
req->idx, req->rc);
|
|
|
|
if (!test_fw_config->test_result)
|
|
|
|
test_fw_config->test_result = req->rc;
|
|
|
|
} else if (req->fw) {
|
|
|
|
req->sent = true;
|
|
|
|
pr_info("#%u: batched sync loaded %zu\n",
|
|
|
|
req->idx, req->fw->size);
|
|
|
|
}
|
|
|
|
complete(&req->completion);
|
|
|
|
|
|
|
|
req->task = NULL;
|
|
|
|
|
|
|
|
return 0;
|
|
|
|
}
|
|
|
|
|
|
|
|
/*
|
|
|
|
* We use a kthread as otherwise the kernel serializes all our sync requests
|
|
|
|
* and we would not be able to mimic batched requests on a sync call. Batched
|
|
|
|
* requests on a sync call can for instance happen on a device driver when
|
|
|
|
* multiple cards are used and firmware loading happens outside of probe.
|
|
|
|
*/
|
|
|
|
static ssize_t trigger_batched_requests_store(struct device *dev,
|
|
|
|
struct device_attribute *attr,
|
|
|
|
const char *buf, size_t count)
|
|
|
|
{
|
|
|
|
struct test_batched_req *req;
|
|
|
|
int rc;
|
|
|
|
u8 i;
|
|
|
|
|
|
|
|
mutex_lock(&test_fw_mutex);
|
|
|
|
|
treewide: Use array_size() in vzalloc()
The vzalloc() function has no 2-factor argument form, so multiplication
factors need to be wrapped in array_size(). This patch replaces cases of:
vzalloc(a * b)
with:
vzalloc(array_size(a, b))
as well as handling cases of:
vzalloc(a * b * c)
with:
vzalloc(array3_size(a, b, c))
This does, however, attempt to ignore constant size factors like:
vzalloc(4 * 1024)
though any constants defined via macros get caught up in the conversion.
Any factors with a sizeof() of "unsigned char", "char", and "u8" were
dropped, since they're redundant.
The Coccinelle script used for this was:
// Fix redundant parens around sizeof().
@@
type TYPE;
expression THING, E;
@@
(
vzalloc(
- (sizeof(TYPE)) * E
+ sizeof(TYPE) * E
, ...)
|
vzalloc(
- (sizeof(THING)) * E
+ sizeof(THING) * E
, ...)
)
// Drop single-byte sizes and redundant parens.
@@
expression COUNT;
typedef u8;
typedef __u8;
@@
(
vzalloc(
- sizeof(u8) * (COUNT)
+ COUNT
, ...)
|
vzalloc(
- sizeof(__u8) * (COUNT)
+ COUNT
, ...)
|
vzalloc(
- sizeof(char) * (COUNT)
+ COUNT
, ...)
|
vzalloc(
- sizeof(unsigned char) * (COUNT)
+ COUNT
, ...)
|
vzalloc(
- sizeof(u8) * COUNT
+ COUNT
, ...)
|
vzalloc(
- sizeof(__u8) * COUNT
+ COUNT
, ...)
|
vzalloc(
- sizeof(char) * COUNT
+ COUNT
, ...)
|
vzalloc(
- sizeof(unsigned char) * COUNT
+ COUNT
, ...)
)
// 2-factor product with sizeof(type/expression) and identifier or constant.
@@
type TYPE;
expression THING;
identifier COUNT_ID;
constant COUNT_CONST;
@@
(
vzalloc(
- sizeof(TYPE) * (COUNT_ID)
+ array_size(COUNT_ID, sizeof(TYPE))
, ...)
|
vzalloc(
- sizeof(TYPE) * COUNT_ID
+ array_size(COUNT_ID, sizeof(TYPE))
, ...)
|
vzalloc(
- sizeof(TYPE) * (COUNT_CONST)
+ array_size(COUNT_CONST, sizeof(TYPE))
, ...)
|
vzalloc(
- sizeof(TYPE) * COUNT_CONST
+ array_size(COUNT_CONST, sizeof(TYPE))
, ...)
|
vzalloc(
- sizeof(THING) * (COUNT_ID)
+ array_size(COUNT_ID, sizeof(THING))
, ...)
|
vzalloc(
- sizeof(THING) * COUNT_ID
+ array_size(COUNT_ID, sizeof(THING))
, ...)
|
vzalloc(
- sizeof(THING) * (COUNT_CONST)
+ array_size(COUNT_CONST, sizeof(THING))
, ...)
|
vzalloc(
- sizeof(THING) * COUNT_CONST
+ array_size(COUNT_CONST, sizeof(THING))
, ...)
)
// 2-factor product, only identifiers.
@@
identifier SIZE, COUNT;
@@
vzalloc(
- SIZE * COUNT
+ array_size(COUNT, SIZE)
, ...)
// 3-factor product with 1 sizeof(type) or sizeof(expression), with
// redundant parens removed.
@@
expression THING;
identifier STRIDE, COUNT;
type TYPE;
@@
(
vzalloc(
- sizeof(TYPE) * (COUNT) * (STRIDE)
+ array3_size(COUNT, STRIDE, sizeof(TYPE))
, ...)
|
vzalloc(
- sizeof(TYPE) * (COUNT) * STRIDE
+ array3_size(COUNT, STRIDE, sizeof(TYPE))
, ...)
|
vzalloc(
- sizeof(TYPE) * COUNT * (STRIDE)
+ array3_size(COUNT, STRIDE, sizeof(TYPE))
, ...)
|
vzalloc(
- sizeof(TYPE) * COUNT * STRIDE
+ array3_size(COUNT, STRIDE, sizeof(TYPE))
, ...)
|
vzalloc(
- sizeof(THING) * (COUNT) * (STRIDE)
+ array3_size(COUNT, STRIDE, sizeof(THING))
, ...)
|
vzalloc(
- sizeof(THING) * (COUNT) * STRIDE
+ array3_size(COUNT, STRIDE, sizeof(THING))
, ...)
|
vzalloc(
- sizeof(THING) * COUNT * (STRIDE)
+ array3_size(COUNT, STRIDE, sizeof(THING))
, ...)
|
vzalloc(
- sizeof(THING) * COUNT * STRIDE
+ array3_size(COUNT, STRIDE, sizeof(THING))
, ...)
)
// 3-factor product with 2 sizeof(variable), with redundant parens removed.
@@
expression THING1, THING2;
identifier COUNT;
type TYPE1, TYPE2;
@@
(
vzalloc(
- sizeof(TYPE1) * sizeof(TYPE2) * COUNT
+ array3_size(COUNT, sizeof(TYPE1), sizeof(TYPE2))
, ...)
|
vzalloc(
- sizeof(TYPE1) * sizeof(THING2) * (COUNT)
+ array3_size(COUNT, sizeof(TYPE1), sizeof(TYPE2))
, ...)
|
vzalloc(
- sizeof(THING1) * sizeof(THING2) * COUNT
+ array3_size(COUNT, sizeof(THING1), sizeof(THING2))
, ...)
|
vzalloc(
- sizeof(THING1) * sizeof(THING2) * (COUNT)
+ array3_size(COUNT, sizeof(THING1), sizeof(THING2))
, ...)
|
vzalloc(
- sizeof(TYPE1) * sizeof(THING2) * COUNT
+ array3_size(COUNT, sizeof(TYPE1), sizeof(THING2))
, ...)
|
vzalloc(
- sizeof(TYPE1) * sizeof(THING2) * (COUNT)
+ array3_size(COUNT, sizeof(TYPE1), sizeof(THING2))
, ...)
)
// 3-factor product, only identifiers, with redundant parens removed.
@@
identifier STRIDE, SIZE, COUNT;
@@
(
vzalloc(
- (COUNT) * STRIDE * SIZE
+ array3_size(COUNT, STRIDE, SIZE)
, ...)
|
vzalloc(
- COUNT * (STRIDE) * SIZE
+ array3_size(COUNT, STRIDE, SIZE)
, ...)
|
vzalloc(
- COUNT * STRIDE * (SIZE)
+ array3_size(COUNT, STRIDE, SIZE)
, ...)
|
vzalloc(
- (COUNT) * (STRIDE) * SIZE
+ array3_size(COUNT, STRIDE, SIZE)
, ...)
|
vzalloc(
- COUNT * (STRIDE) * (SIZE)
+ array3_size(COUNT, STRIDE, SIZE)
, ...)
|
vzalloc(
- (COUNT) * STRIDE * (SIZE)
+ array3_size(COUNT, STRIDE, SIZE)
, ...)
|
vzalloc(
- (COUNT) * (STRIDE) * (SIZE)
+ array3_size(COUNT, STRIDE, SIZE)
, ...)
|
vzalloc(
- COUNT * STRIDE * SIZE
+ array3_size(COUNT, STRIDE, SIZE)
, ...)
)
// Any remaining multi-factor products, first at least 3-factor products
// when they're not all constants...
@@
expression E1, E2, E3;
constant C1, C2, C3;
@@
(
vzalloc(C1 * C2 * C3, ...)
|
vzalloc(
- E1 * E2 * E3
+ array3_size(E1, E2, E3)
, ...)
)
// And then all remaining 2 factors products when they're not all constants.
@@
expression E1, E2;
constant C1, C2;
@@
(
vzalloc(C1 * C2, ...)
|
vzalloc(
- E1 * E2
+ array_size(E1, E2)
, ...)
)
Signed-off-by: Kees Cook <keescook@chromium.org>
2018-06-13 05:27:37 +08:00
|
|
|
test_fw_config->reqs =
|
|
|
|
vzalloc(array3_size(sizeof(struct test_batched_req),
|
|
|
|
test_fw_config->num_requests, 2));
|
2017-07-21 04:13:42 +08:00
|
|
|
if (!test_fw_config->reqs) {
|
|
|
|
rc = -ENOMEM;
|
|
|
|
goto out_unlock;
|
|
|
|
}
|
|
|
|
|
|
|
|
pr_info("batched sync firmware loading '%s' %u times\n",
|
|
|
|
test_fw_config->name, test_fw_config->num_requests);
|
|
|
|
|
|
|
|
for (i = 0; i < test_fw_config->num_requests; i++) {
|
|
|
|
req = &test_fw_config->reqs[i];
|
|
|
|
req->fw = NULL;
|
|
|
|
req->idx = i;
|
|
|
|
req->name = test_fw_config->name;
|
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: 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>
2023-05-09 16:47:49 +08:00
|
|
|
req->fw_buf = NULL;
|
2017-07-21 04:13:42 +08:00
|
|
|
req->dev = dev;
|
|
|
|
init_completion(&req->completion);
|
|
|
|
req->task = kthread_run(test_fw_run_batch_request, req,
|
|
|
|
"%s-%u", KBUILD_MODNAME, req->idx);
|
|
|
|
if (!req->task || IS_ERR(req->task)) {
|
|
|
|
pr_err("Setting up thread %u failed\n", req->idx);
|
|
|
|
req->task = NULL;
|
|
|
|
rc = -ENOMEM;
|
|
|
|
goto out_bail;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
rc = count;
|
|
|
|
|
|
|
|
/*
|
|
|
|
* We require an explicit release to enable more time and delay of
|
|
|
|
* calling release_firmware() to improve our chances of forcing a
|
|
|
|
* batched request. If we instead called release_firmware() right away
|
|
|
|
* then we might miss on an opportunity of having a successful firmware
|
|
|
|
* request pass on the opportunity to be come a batched request.
|
|
|
|
*/
|
|
|
|
|
|
|
|
out_bail:
|
|
|
|
for (i = 0; i < test_fw_config->num_requests; i++) {
|
|
|
|
req = &test_fw_config->reqs[i];
|
|
|
|
if (req->task || req->sent)
|
|
|
|
wait_for_completion(&req->completion);
|
|
|
|
}
|
|
|
|
|
|
|
|
/* Override any worker error if we had a general setup error */
|
|
|
|
if (rc < 0)
|
|
|
|
test_fw_config->test_result = rc;
|
|
|
|
|
|
|
|
out_unlock:
|
|
|
|
mutex_unlock(&test_fw_mutex);
|
|
|
|
|
|
|
|
return rc;
|
|
|
|
}
|
|
|
|
static DEVICE_ATTR_WO(trigger_batched_requests);
|
|
|
|
|
|
|
|
/*
|
|
|
|
* We wait for each callback to return with the lock held, no need to lock here
|
|
|
|
*/
|
|
|
|
static void trigger_batched_cb(const struct firmware *fw, void *context)
|
|
|
|
{
|
|
|
|
struct test_batched_req *req = context;
|
|
|
|
|
|
|
|
if (!req) {
|
|
|
|
test_fw_config->test_result = -EINVAL;
|
|
|
|
return;
|
|
|
|
}
|
|
|
|
|
|
|
|
/* forces *some* batched requests to queue up */
|
|
|
|
if (!req->idx)
|
|
|
|
ssleep(2);
|
|
|
|
|
|
|
|
req->fw = fw;
|
|
|
|
|
|
|
|
/*
|
|
|
|
* Unfortunately the firmware API gives us nothing other than a null FW
|
|
|
|
* if the firmware was not found on async requests. Best we can do is
|
|
|
|
* just assume -ENOENT. A better API would pass the actual return
|
|
|
|
* value to the callback.
|
|
|
|
*/
|
|
|
|
if (!fw && !test_fw_config->test_result)
|
|
|
|
test_fw_config->test_result = -ENOENT;
|
|
|
|
|
|
|
|
complete(&req->completion);
|
|
|
|
}
|
|
|
|
|
|
|
|
static
|
|
|
|
ssize_t trigger_batched_requests_async_store(struct device *dev,
|
|
|
|
struct device_attribute *attr,
|
|
|
|
const char *buf, size_t count)
|
|
|
|
{
|
|
|
|
struct test_batched_req *req;
|
|
|
|
bool send_uevent;
|
|
|
|
int rc;
|
|
|
|
u8 i;
|
|
|
|
|
|
|
|
mutex_lock(&test_fw_mutex);
|
|
|
|
|
treewide: Use array_size() in vzalloc()
The vzalloc() function has no 2-factor argument form, so multiplication
factors need to be wrapped in array_size(). This patch replaces cases of:
vzalloc(a * b)
with:
vzalloc(array_size(a, b))
as well as handling cases of:
vzalloc(a * b * c)
with:
vzalloc(array3_size(a, b, c))
This does, however, attempt to ignore constant size factors like:
vzalloc(4 * 1024)
though any constants defined via macros get caught up in the conversion.
Any factors with a sizeof() of "unsigned char", "char", and "u8" were
dropped, since they're redundant.
The Coccinelle script used for this was:
// Fix redundant parens around sizeof().
@@
type TYPE;
expression THING, E;
@@
(
vzalloc(
- (sizeof(TYPE)) * E
+ sizeof(TYPE) * E
, ...)
|
vzalloc(
- (sizeof(THING)) * E
+ sizeof(THING) * E
, ...)
)
// Drop single-byte sizes and redundant parens.
@@
expression COUNT;
typedef u8;
typedef __u8;
@@
(
vzalloc(
- sizeof(u8) * (COUNT)
+ COUNT
, ...)
|
vzalloc(
- sizeof(__u8) * (COUNT)
+ COUNT
, ...)
|
vzalloc(
- sizeof(char) * (COUNT)
+ COUNT
, ...)
|
vzalloc(
- sizeof(unsigned char) * (COUNT)
+ COUNT
, ...)
|
vzalloc(
- sizeof(u8) * COUNT
+ COUNT
, ...)
|
vzalloc(
- sizeof(__u8) * COUNT
+ COUNT
, ...)
|
vzalloc(
- sizeof(char) * COUNT
+ COUNT
, ...)
|
vzalloc(
- sizeof(unsigned char) * COUNT
+ COUNT
, ...)
)
// 2-factor product with sizeof(type/expression) and identifier or constant.
@@
type TYPE;
expression THING;
identifier COUNT_ID;
constant COUNT_CONST;
@@
(
vzalloc(
- sizeof(TYPE) * (COUNT_ID)
+ array_size(COUNT_ID, sizeof(TYPE))
, ...)
|
vzalloc(
- sizeof(TYPE) * COUNT_ID
+ array_size(COUNT_ID, sizeof(TYPE))
, ...)
|
vzalloc(
- sizeof(TYPE) * (COUNT_CONST)
+ array_size(COUNT_CONST, sizeof(TYPE))
, ...)
|
vzalloc(
- sizeof(TYPE) * COUNT_CONST
+ array_size(COUNT_CONST, sizeof(TYPE))
, ...)
|
vzalloc(
- sizeof(THING) * (COUNT_ID)
+ array_size(COUNT_ID, sizeof(THING))
, ...)
|
vzalloc(
- sizeof(THING) * COUNT_ID
+ array_size(COUNT_ID, sizeof(THING))
, ...)
|
vzalloc(
- sizeof(THING) * (COUNT_CONST)
+ array_size(COUNT_CONST, sizeof(THING))
, ...)
|
vzalloc(
- sizeof(THING) * COUNT_CONST
+ array_size(COUNT_CONST, sizeof(THING))
, ...)
)
// 2-factor product, only identifiers.
@@
identifier SIZE, COUNT;
@@
vzalloc(
- SIZE * COUNT
+ array_size(COUNT, SIZE)
, ...)
// 3-factor product with 1 sizeof(type) or sizeof(expression), with
// redundant parens removed.
@@
expression THING;
identifier STRIDE, COUNT;
type TYPE;
@@
(
vzalloc(
- sizeof(TYPE) * (COUNT) * (STRIDE)
+ array3_size(COUNT, STRIDE, sizeof(TYPE))
, ...)
|
vzalloc(
- sizeof(TYPE) * (COUNT) * STRIDE
+ array3_size(COUNT, STRIDE, sizeof(TYPE))
, ...)
|
vzalloc(
- sizeof(TYPE) * COUNT * (STRIDE)
+ array3_size(COUNT, STRIDE, sizeof(TYPE))
, ...)
|
vzalloc(
- sizeof(TYPE) * COUNT * STRIDE
+ array3_size(COUNT, STRIDE, sizeof(TYPE))
, ...)
|
vzalloc(
- sizeof(THING) * (COUNT) * (STRIDE)
+ array3_size(COUNT, STRIDE, sizeof(THING))
, ...)
|
vzalloc(
- sizeof(THING) * (COUNT) * STRIDE
+ array3_size(COUNT, STRIDE, sizeof(THING))
, ...)
|
vzalloc(
- sizeof(THING) * COUNT * (STRIDE)
+ array3_size(COUNT, STRIDE, sizeof(THING))
, ...)
|
vzalloc(
- sizeof(THING) * COUNT * STRIDE
+ array3_size(COUNT, STRIDE, sizeof(THING))
, ...)
)
// 3-factor product with 2 sizeof(variable), with redundant parens removed.
@@
expression THING1, THING2;
identifier COUNT;
type TYPE1, TYPE2;
@@
(
vzalloc(
- sizeof(TYPE1) * sizeof(TYPE2) * COUNT
+ array3_size(COUNT, sizeof(TYPE1), sizeof(TYPE2))
, ...)
|
vzalloc(
- sizeof(TYPE1) * sizeof(THING2) * (COUNT)
+ array3_size(COUNT, sizeof(TYPE1), sizeof(TYPE2))
, ...)
|
vzalloc(
- sizeof(THING1) * sizeof(THING2) * COUNT
+ array3_size(COUNT, sizeof(THING1), sizeof(THING2))
, ...)
|
vzalloc(
- sizeof(THING1) * sizeof(THING2) * (COUNT)
+ array3_size(COUNT, sizeof(THING1), sizeof(THING2))
, ...)
|
vzalloc(
- sizeof(TYPE1) * sizeof(THING2) * COUNT
+ array3_size(COUNT, sizeof(TYPE1), sizeof(THING2))
, ...)
|
vzalloc(
- sizeof(TYPE1) * sizeof(THING2) * (COUNT)
+ array3_size(COUNT, sizeof(TYPE1), sizeof(THING2))
, ...)
)
// 3-factor product, only identifiers, with redundant parens removed.
@@
identifier STRIDE, SIZE, COUNT;
@@
(
vzalloc(
- (COUNT) * STRIDE * SIZE
+ array3_size(COUNT, STRIDE, SIZE)
, ...)
|
vzalloc(
- COUNT * (STRIDE) * SIZE
+ array3_size(COUNT, STRIDE, SIZE)
, ...)
|
vzalloc(
- COUNT * STRIDE * (SIZE)
+ array3_size(COUNT, STRIDE, SIZE)
, ...)
|
vzalloc(
- (COUNT) * (STRIDE) * SIZE
+ array3_size(COUNT, STRIDE, SIZE)
, ...)
|
vzalloc(
- COUNT * (STRIDE) * (SIZE)
+ array3_size(COUNT, STRIDE, SIZE)
, ...)
|
vzalloc(
- (COUNT) * STRIDE * (SIZE)
+ array3_size(COUNT, STRIDE, SIZE)
, ...)
|
vzalloc(
- (COUNT) * (STRIDE) * (SIZE)
+ array3_size(COUNT, STRIDE, SIZE)
, ...)
|
vzalloc(
- COUNT * STRIDE * SIZE
+ array3_size(COUNT, STRIDE, SIZE)
, ...)
)
// Any remaining multi-factor products, first at least 3-factor products
// when they're not all constants...
@@
expression E1, E2, E3;
constant C1, C2, C3;
@@
(
vzalloc(C1 * C2 * C3, ...)
|
vzalloc(
- E1 * E2 * E3
+ array3_size(E1, E2, E3)
, ...)
)
// And then all remaining 2 factors products when they're not all constants.
@@
expression E1, E2;
constant C1, C2;
@@
(
vzalloc(C1 * C2, ...)
|
vzalloc(
- E1 * E2
+ array_size(E1, E2)
, ...)
)
Signed-off-by: Kees Cook <keescook@chromium.org>
2018-06-13 05:27:37 +08:00
|
|
|
test_fw_config->reqs =
|
|
|
|
vzalloc(array3_size(sizeof(struct test_batched_req),
|
|
|
|
test_fw_config->num_requests, 2));
|
2017-07-21 04:13:42 +08:00
|
|
|
if (!test_fw_config->reqs) {
|
|
|
|
rc = -ENOMEM;
|
|
|
|
goto out;
|
|
|
|
}
|
|
|
|
|
|
|
|
pr_info("batched loading '%s' custom fallback mechanism %u times\n",
|
|
|
|
test_fw_config->name, test_fw_config->num_requests);
|
|
|
|
|
2021-04-25 10:00:24 +08:00
|
|
|
send_uevent = test_fw_config->send_uevent ? FW_ACTION_UEVENT :
|
|
|
|
FW_ACTION_NOUEVENT;
|
2017-07-21 04:13:42 +08:00
|
|
|
|
|
|
|
for (i = 0; i < test_fw_config->num_requests; i++) {
|
|
|
|
req = &test_fw_config->reqs[i];
|
|
|
|
req->name = test_fw_config->name;
|
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: 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>
2023-05-09 16:47:49 +08:00
|
|
|
req->fw_buf = NULL;
|
2017-07-21 04:13:42 +08:00
|
|
|
req->fw = NULL;
|
|
|
|
req->idx = i;
|
|
|
|
init_completion(&req->completion);
|
|
|
|
rc = request_firmware_nowait(THIS_MODULE, send_uevent,
|
|
|
|
req->name,
|
|
|
|
dev, GFP_KERNEL, req,
|
|
|
|
trigger_batched_cb);
|
|
|
|
if (rc) {
|
|
|
|
pr_info("#%u: batched async load failed setup: %d\n",
|
|
|
|
i, rc);
|
|
|
|
req->rc = rc;
|
|
|
|
goto out_bail;
|
|
|
|
} else
|
|
|
|
req->sent = true;
|
|
|
|
}
|
|
|
|
|
|
|
|
rc = count;
|
|
|
|
|
|
|
|
out_bail:
|
|
|
|
|
|
|
|
/*
|
|
|
|
* We require an explicit release to enable more time and delay of
|
|
|
|
* calling release_firmware() to improve our chances of forcing a
|
|
|
|
* batched request. If we instead called release_firmware() right away
|
|
|
|
* then we might miss on an opportunity of having a successful firmware
|
|
|
|
* request pass on the opportunity to be come a batched request.
|
|
|
|
*/
|
|
|
|
|
|
|
|
for (i = 0; i < test_fw_config->num_requests; i++) {
|
|
|
|
req = &test_fw_config->reqs[i];
|
|
|
|
if (req->sent)
|
|
|
|
wait_for_completion(&req->completion);
|
|
|
|
}
|
|
|
|
|
|
|
|
/* Override any worker error if we had a general setup error */
|
|
|
|
if (rc < 0)
|
|
|
|
test_fw_config->test_result = rc;
|
|
|
|
|
|
|
|
out:
|
|
|
|
mutex_unlock(&test_fw_mutex);
|
|
|
|
|
|
|
|
return rc;
|
|
|
|
}
|
|
|
|
static DEVICE_ATTR_WO(trigger_batched_requests_async);
|
|
|
|
|
|
|
|
static ssize_t test_result_show(struct device *dev,
|
|
|
|
struct device_attribute *attr,
|
|
|
|
char *buf)
|
|
|
|
{
|
|
|
|
return test_dev_config_show_int(buf, test_fw_config->test_result);
|
|
|
|
}
|
|
|
|
static DEVICE_ATTR_RO(test_result);
|
|
|
|
|
|
|
|
static ssize_t release_all_firmware_store(struct device *dev,
|
|
|
|
struct device_attribute *attr,
|
|
|
|
const char *buf, size_t count)
|
|
|
|
{
|
|
|
|
test_release_all_firmware();
|
|
|
|
return count;
|
|
|
|
}
|
|
|
|
static DEVICE_ATTR_WO(release_all_firmware);
|
|
|
|
|
|
|
|
static ssize_t read_firmware_show(struct device *dev,
|
|
|
|
struct device_attribute *attr,
|
|
|
|
char *buf)
|
|
|
|
{
|
|
|
|
struct test_batched_req *req;
|
|
|
|
u8 idx;
|
|
|
|
ssize_t rc = 0;
|
|
|
|
|
|
|
|
mutex_lock(&test_fw_mutex);
|
|
|
|
|
|
|
|
idx = test_fw_config->read_fw_idx;
|
|
|
|
if (idx >= test_fw_config->num_requests) {
|
|
|
|
rc = -ERANGE;
|
|
|
|
goto out;
|
|
|
|
}
|
|
|
|
|
|
|
|
if (!test_fw_config->reqs) {
|
|
|
|
rc = -EINVAL;
|
|
|
|
goto out;
|
|
|
|
}
|
|
|
|
|
|
|
|
req = &test_fw_config->reqs[idx];
|
|
|
|
if (!req->fw) {
|
|
|
|
pr_err("#%u: failed to async load firmware\n", idx);
|
|
|
|
rc = -ENOENT;
|
|
|
|
goto out;
|
|
|
|
}
|
|
|
|
|
|
|
|
pr_info("#%u: loaded %zu\n", idx, req->fw->size);
|
|
|
|
|
|
|
|
if (req->fw->size > PAGE_SIZE) {
|
|
|
|
pr_err("Testing interface must use PAGE_SIZE firmware for now\n");
|
|
|
|
rc = -EINVAL;
|
2018-10-19 20:58:01 +08:00
|
|
|
goto out;
|
2017-07-21 04:13:42 +08:00
|
|
|
}
|
|
|
|
memcpy(buf, req->fw->data, req->fw->size);
|
|
|
|
|
|
|
|
rc = req->fw->size;
|
|
|
|
out:
|
|
|
|
mutex_unlock(&test_fw_mutex);
|
|
|
|
|
|
|
|
return rc;
|
|
|
|
}
|
|
|
|
static DEVICE_ATTR_RO(read_firmware);
|
|
|
|
|
2017-01-24 00:11:06 +08:00
|
|
|
#define TEST_FW_DEV_ATTR(name) &dev_attr_##name.attr
|
|
|
|
|
|
|
|
static struct attribute *test_dev_attrs[] = {
|
2017-07-21 04:13:42 +08:00
|
|
|
TEST_FW_DEV_ATTR(reset),
|
|
|
|
|
|
|
|
TEST_FW_DEV_ATTR(config),
|
|
|
|
TEST_FW_DEV_ATTR(config_name),
|
|
|
|
TEST_FW_DEV_ATTR(config_num_requests),
|
2019-08-23 02:40:04 +08:00
|
|
|
TEST_FW_DEV_ATTR(config_into_buf),
|
2020-10-03 01:38:28 +08:00
|
|
|
TEST_FW_DEV_ATTR(config_buf_size),
|
|
|
|
TEST_FW_DEV_ATTR(config_file_offset),
|
|
|
|
TEST_FW_DEV_ATTR(config_partial),
|
2017-07-21 04:13:42 +08:00
|
|
|
TEST_FW_DEV_ATTR(config_sync_direct),
|
|
|
|
TEST_FW_DEV_ATTR(config_send_uevent),
|
|
|
|
TEST_FW_DEV_ATTR(config_read_fw_idx),
|
|
|
|
|
|
|
|
/* These don't use the config at all - they could be ported! */
|
2017-01-24 00:11:06 +08:00
|
|
|
TEST_FW_DEV_ATTR(trigger_request),
|
|
|
|
TEST_FW_DEV_ATTR(trigger_async_request),
|
2017-01-24 00:11:10 +08:00
|
|
|
TEST_FW_DEV_ATTR(trigger_custom_fallback),
|
2020-01-16 00:35:49 +08:00
|
|
|
#ifdef CONFIG_EFI_EMBEDDED_FIRMWARE
|
|
|
|
TEST_FW_DEV_ATTR(trigger_request_platform),
|
|
|
|
#endif
|
2017-07-21 04:13:42 +08:00
|
|
|
|
|
|
|
/* These use the config and can use the test_result */
|
|
|
|
TEST_FW_DEV_ATTR(trigger_batched_requests),
|
|
|
|
TEST_FW_DEV_ATTR(trigger_batched_requests_async),
|
|
|
|
|
|
|
|
TEST_FW_DEV_ATTR(release_all_firmware),
|
|
|
|
TEST_FW_DEV_ATTR(test_result),
|
|
|
|
TEST_FW_DEV_ATTR(read_firmware),
|
2017-01-24 00:11:06 +08:00
|
|
|
NULL,
|
|
|
|
};
|
|
|
|
|
|
|
|
ATTRIBUTE_GROUPS(test_dev);
|
|
|
|
|
2017-01-24 00:11:05 +08:00
|
|
|
static struct miscdevice test_fw_misc_device = {
|
|
|
|
.minor = MISC_DYNAMIC_MINOR,
|
|
|
|
.name = "test_firmware",
|
|
|
|
.fops = &test_fw_fops,
|
2017-01-24 00:11:06 +08:00
|
|
|
.groups = test_dev_groups,
|
2017-01-24 00:11:05 +08:00
|
|
|
};
|
|
|
|
|
2014-07-15 05:38:12 +08:00
|
|
|
static int __init test_firmware_init(void)
|
|
|
|
{
|
|
|
|
int rc;
|
|
|
|
|
2017-07-21 04:13:42 +08:00
|
|
|
test_fw_config = kzalloc(sizeof(struct test_config), GFP_KERNEL);
|
|
|
|
if (!test_fw_config)
|
|
|
|
return -ENOMEM;
|
|
|
|
|
|
|
|
rc = __test_firmware_config_init();
|
2019-07-14 14:11:35 +08:00
|
|
|
if (rc) {
|
|
|
|
kfree(test_fw_config);
|
|
|
|
pr_err("could not init firmware test config: %d\n", rc);
|
2017-07-21 04:13:42 +08:00
|
|
|
return rc;
|
2019-07-14 14:11:35 +08:00
|
|
|
}
|
2017-07-21 04:13:42 +08:00
|
|
|
|
2014-07-15 05:38:12 +08:00
|
|
|
rc = misc_register(&test_fw_misc_device);
|
|
|
|
if (rc) {
|
2022-11-19 11:57:21 +08:00
|
|
|
__test_firmware_config_free();
|
2017-07-21 04:13:42 +08:00
|
|
|
kfree(test_fw_config);
|
2014-07-15 05:38:12 +08:00
|
|
|
pr_err("could not register misc device: %d\n", rc);
|
|
|
|
return rc;
|
|
|
|
}
|
2015-12-10 06:50:27 +08:00
|
|
|
|
2014-07-15 05:38:12 +08:00
|
|
|
pr_warn("interface ready\n");
|
|
|
|
|
|
|
|
return 0;
|
|
|
|
}
|
|
|
|
|
|
|
|
module_init(test_firmware_init);
|
|
|
|
|
|
|
|
static void __exit test_firmware_exit(void)
|
|
|
|
{
|
2017-07-21 04:13:42 +08:00
|
|
|
mutex_lock(&test_fw_mutex);
|
2014-07-15 05:38:12 +08:00
|
|
|
release_firmware(test_firmware);
|
|
|
|
misc_deregister(&test_fw_misc_device);
|
2017-07-21 04:13:42 +08:00
|
|
|
__test_firmware_config_free();
|
|
|
|
kfree(test_fw_config);
|
|
|
|
mutex_unlock(&test_fw_mutex);
|
|
|
|
|
2014-07-15 05:38:12 +08:00
|
|
|
pr_warn("removed interface\n");
|
|
|
|
}
|
|
|
|
|
|
|
|
module_exit(test_firmware_exit);
|
|
|
|
|
|
|
|
MODULE_AUTHOR("Kees Cook <keescook@chromium.org>");
|
|
|
|
MODULE_LICENSE("GPL");
|