[RESEND PATCH v5 1/3] test_firmware: prevent race conditions by a correct implementation of locking
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 --- lib/test_firmware.c | 52 ++++++++++++++++++++++++++++++--------------- 1 file changed, 35 insertions(+), 17 deletions(-)
diff --git a/lib/test_firmware.c b/lib/test_firmware.c index 05ed84c2fc4c..35417e0af3f4 100644 --- a/lib/test_firmware.c +++ b/lib/test_firmware.c @@ -353,16 +353,26 @@ static ssize_t config_test_show_str(char *dst, return len; }
-static int test_dev_config_update_bool(const char *buf, size_t size, +static inline int __test_dev_config_update_bool(const char *buf, size_t size, bool *cfg) { int ret;
- mutex_lock(&test_fw_mutex); if (kstrtobool(buf, cfg) < 0) ret = -EINVAL; else ret = size; + + 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); mutex_unlock(&test_fw_mutex);
return ret; @@ -373,7 +383,8 @@ static ssize_t test_dev_config_show_bool(char *buf, bool val) return snprintf(buf, PAGE_SIZE, "%d\n", val); }
-static int test_dev_config_update_size_t(const char *buf, +static int __test_dev_config_update_size_t( + const char *buf, size_t size, size_t *cfg) { @@ -384,9 +395,7 @@ static int test_dev_config_update_size_t(const char *buf, if (ret) return ret;
- mutex_lock(&test_fw_mutex); *(size_t *)cfg = new; - mutex_unlock(&test_fw_mutex);
/* Always return full write size even if we didn't consume all */ return size; @@ -402,7 +411,7 @@ static ssize_t test_dev_config_show_int(char *buf, int val) return snprintf(buf, PAGE_SIZE, "%d\n", val); }
-static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg) +static int __test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg) { u8 val; int ret; @@ -411,14 +420,23 @@ static int test_dev_config_update_u8(const char *buf, size_t size, u8 *cfg) 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 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; +} + static ssize_t test_dev_config_show_u8(char *buf, u8 val) { return snprintf(buf, PAGE_SIZE, "%u\n", val); @@ -471,10 +489,10 @@ static ssize_t config_num_requests_store(struct device *dev, 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); + rc = __test_dev_config_update_u8(buf, count, + &test_fw_config->num_requests); + mutex_unlock(&test_fw_mutex);
out: return rc; @@ -518,10 +536,10 @@ static ssize_t config_buf_size_store(struct device *dev, mutex_unlock(&test_fw_mutex); goto out; } - mutex_unlock(&test_fw_mutex);
- rc = test_dev_config_update_size_t(buf, count, - &test_fw_config->buf_size); + rc = __test_dev_config_update_size_t(buf, count, + &test_fw_config->buf_size); + mutex_unlock(&test_fw_mutex);
out: return rc; @@ -548,10 +566,10 @@ static ssize_t config_file_offset_store(struct device *dev, mutex_unlock(&test_fw_mutex); goto out; } - mutex_unlock(&test_fw_mutex);
- rc = test_dev_config_update_size_t(buf, count, - &test_fw_config->file_offset); + rc = __test_dev_config_update_size_t(buf, count, + &test_fw_config->file_offset); + mutex_unlock(&test_fw_mutex);
out: return rc;
Dan Carpenter spotted that test_fw_config->reqs will be leaked if trigger_batched_requests_store() is called two or more times. The same appears with trigger_batched_requests_async_store().
This bug wasn't trigger by the tests, but observed by Dan's visual inspection of the code.
The recommended workaround was to return -EBUSY if test_fw_config->reqs is already allocated.
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: 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 Suggested-by: Takashi Iwai tiwai@suse.de Signed-off-by: Mirsad Goran Todorovac mirsad.todorovac@alu.unizg.hr --- lib/test_firmware.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/lib/test_firmware.c b/lib/test_firmware.c index 35417e0af3f4..91b232ed3161 100644 --- a/lib/test_firmware.c +++ b/lib/test_firmware.c @@ -913,6 +913,11 @@ static ssize_t trigger_batched_requests_store(struct device *dev,
mutex_lock(&test_fw_mutex);
+ if (test_fw_config->reqs) { + rc = -EBUSY; + goto out_bail; + } + test_fw_config->reqs = vzalloc(array3_size(sizeof(struct test_batched_req), test_fw_config->num_requests, 2)); @@ -1011,6 +1016,11 @@ ssize_t trigger_batched_requests_async_store(struct device *dev,
mutex_lock(&test_fw_mutex);
+ if (test_fw_config->reqs) { + rc = -EBUSY; + goto out_bail; + } + test_fw_config->reqs = vzalloc(array3_size(sizeof(struct test_batched_req), test_fw_config->num_requests, 2));
Hi Dan,
On 5/9/23 10:47, Mirsad Goran Todorovac wrote:
Dan Carpenter spotted that test_fw_config->reqs will be leaked if trigger_batched_requests_store() is called two or more times. The same appears with trigger_batched_requests_async_store().
This bug wasn't trigger by the tests, but observed by Dan's visual inspection of the code.
The recommended workaround was to return -EBUSY if test_fw_config->reqs is already allocated.
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: 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 Suggested-by: Takashi Iwai tiwai@suse.de Signed-off-by: Mirsad Goran Todorovac mirsad.todorovac@alu.unizg.hr
lib/test_firmware.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/lib/test_firmware.c b/lib/test_firmware.c index 35417e0af3f4..91b232ed3161 100644 --- a/lib/test_firmware.c +++ b/lib/test_firmware.c @@ -913,6 +913,11 @@ static ssize_t trigger_batched_requests_store(struct device *dev, mutex_lock(&test_fw_mutex);
- if (test_fw_config->reqs) {
rc = -EBUSY;goto out_bail;- }
- test_fw_config->reqs = vzalloc(array3_size(sizeof(struct test_batched_req), test_fw_config->num_requests, 2));
@@ -1011,6 +1016,11 @@ ssize_t trigger_batched_requests_async_store(struct device *dev, mutex_lock(&test_fw_mutex);
- if (test_fw_config->reqs) {
rc = -EBUSY;goto out_bail;- }
- test_fw_config->reqs = vzalloc(array3_size(sizeof(struct test_batched_req), test_fw_config->num_requests, 2));
I was just thinking, since returning -EBUSY for the case of already allocated test_fw_config->reqs was your suggestion and your idea, maybe it would be OK to properly reflect that in Co-developed-by: or Signed-off-by: , but if I understood well, the CoC requires that I am explicitly approved of those?
Thanks, Mirsad
On Fri, May 12, 2023 at 02:34:29PM +0200, Mirsad Todorovac wrote:
@@ -1011,6 +1016,11 @@ ssize_t trigger_batched_requests_async_store(struct device *dev, mutex_lock(&test_fw_mutex);
- if (test_fw_config->reqs) {
rc = -EBUSY;goto out_bail;- }
- test_fw_config->reqs = vzalloc(array3_size(sizeof(struct test_batched_req), test_fw_config->num_requests, 2));
I was just thinking, since returning -EBUSY for the case of already allocated test_fw_config->reqs was your suggestion and your idea, maybe it would be OK to properly reflect that in Co-developed-by: or Signed-off-by: , but if I understood well, the CoC requires that I am explicitly approved of those?
If everyone else is okay, let's just apply this as-is. You did all the hard bits.
regards, dan carpenter
On 12. 05. 2023. 15:09, Dan Carpenter wrote:
On Fri, May 12, 2023 at 02:34:29PM +0200, Mirsad Todorovac wrote:
@@ -1011,6 +1016,11 @@ ssize_t trigger_batched_requests_async_store(struct device *dev, mutex_lock(&test_fw_mutex);
- if (test_fw_config->reqs) {
rc = -EBUSY;goto out_bail;- }
- test_fw_config->reqs = vzalloc(array3_size(sizeof(struct test_batched_req), test_fw_config->num_requests, 2));
I was just thinking, since returning -EBUSY for the case of already allocated test_fw_config->reqs was your suggestion and your idea, maybe it would be OK to properly reflect that in Co-developed-by: or Signed-off-by: , but if I understood well, the CoC requires that I am explicitly approved of those?
If everyone else is okay, let's just apply this as-is. You did all the hard bits.
regards, dan carpenter
If it is OK with you, then I hope I have your Reviewed-by:
I'm kinda still uncertain about the proper procedure. This certainly isn't "the perfect patch" :-)
Best regards, Mirsad
On Fri, May 12, 2023 at 08:58:58PM +0200, Mirsad Goran Todorovac wrote:
On 12. 05. 2023. 15:09, Dan Carpenter wrote:
On Fri, May 12, 2023 at 02:34:29PM +0200, Mirsad Todorovac wrote:
@@ -1011,6 +1016,11 @@ ssize_t trigger_batched_requests_async_store(struct device *dev, mutex_lock(&test_fw_mutex);
- if (test_fw_config->reqs) {
rc = -EBUSY;goto out_bail;- }
- test_fw_config->reqs = vzalloc(array3_size(sizeof(struct test_batched_req), test_fw_config->num_requests, 2));
I was just thinking, since returning -EBUSY for the case of already allocated test_fw_config->reqs was your suggestion and your idea, maybe it would be OK to properly reflect that in Co-developed-by: or Signed-off-by: , but if I understood well, the CoC requires that I am explicitly approved of those?
If everyone else is okay, let's just apply this as-is. You did all the hard bits.
regards, dan carpenter
If it is OK with you, then I hope I have your Reviewed-by:
Wow. Sorry for all the delay on this.
Reviewed-by: Dan Carpenter dan.carpenter@linaro.org
I'm kinda still uncertain about the proper procedure. This certainly isn't "the perfect patch" :-)
Heh.
regards, dan carpenter
On 5/18/23 17:20, Dan Carpenter wrote:
On Fri, May 12, 2023 at 08:58:58PM +0200, Mirsad Goran Todorovac wrote:
On 12. 05. 2023. 15:09, Dan Carpenter wrote:
On Fri, May 12, 2023 at 02:34:29PM +0200, Mirsad Todorovac wrote:
@@ -1011,6 +1016,11 @@ ssize_t trigger_batched_requests_async_store(struct device *dev, mutex_lock(&test_fw_mutex);
- if (test_fw_config->reqs) {
rc = -EBUSY;goto out_bail;- }
test_fw_config->reqs = vzalloc(array3_size(sizeof(struct test_batched_req), test_fw_config->num_requests, 2));I was just thinking, since returning -EBUSY for the case of already allocated test_fw_config->reqs was your suggestion and your idea, maybe it would be OK to properly reflect that in Co-developed-by: or Signed-off-by: , but if I understood well, the CoC requires that I am explicitly approved of those?
If everyone else is okay, let's just apply this as-is. You did all the hard bits.
regards, dan carpenter
If it is OK with you, then I hope I have your Reviewed-by:
Wow. Sorry for all the delay on this.
No, not at all. I don't want to be a nag and overwhelm developers. :-)
Reviewed-by: Dan Carpenter dan.carpenter@linaro.org
Thank you.
I suppose this is for 2/3.
Did you consider reviewing the other two patches?
I'm kinda still uncertain about the proper procedure. This certainly isn't "the perfect patch" :-)
Heh.
regards, dan carpenter
Well, I have about come to the limits of CONFIG_DEBUG_KMEMLEAK setting, with a happy catch of about a dozen bugs, but this is still less than 0.1% of the expected 11,000 bugs for a codebase sized 10.9 million line.
So I am considering the use of a static analysis tool. Like Smatch.
Thank Heavens, most of the code is modular, and about 90% of the functions are static and thereof, of course, having the scope limited to their module.
I am still only catching bugs like memleaks and lockups when they manifest, proactive search for bugs is a new level I suppose.
Best regards, Mirsad
On Thu, May 18, 2023 at 06:20:37PM +0300, Dan Carpenter wrote:
On Fri, May 12, 2023 at 08:58:58PM +0200, Mirsad Goran Todorovac wrote:
On 12. 05. 2023. 15:09, Dan Carpenter wrote:
On Fri, May 12, 2023 at 02:34:29PM +0200, Mirsad Todorovac wrote:
@@ -1011,6 +1016,11 @@ ssize_t trigger_batched_requests_async_store(struct device *dev, mutex_lock(&test_fw_mutex);
- if (test_fw_config->reqs) {
rc = -EBUSY;goto out_bail;- }
- test_fw_config->reqs = vzalloc(array3_size(sizeof(struct test_batched_req), test_fw_config->num_requests, 2));
I was just thinking, since returning -EBUSY for the case of already allocated test_fw_config->reqs was your suggestion and your idea, maybe it would be OK to properly reflect that in Co-developed-by: or Signed-off-by: , but if I understood well, the CoC requires that I am explicitly approved of those?
If everyone else is okay, let's just apply this as-is. You did all the hard bits.
regards, dan carpenter
If it is OK with you, then I hope I have your Reviewed-by:
Wow. Sorry for all the delay on this.
Reviewed-by: Dan Carpenter dan.carpenter@linaro.org
Thanks for doing this work! It looks much better now split up!
For all 3 patches:
Acked-by: Luis Chamberlain mcgrof@kernel.org
Greg, can you pick these up?
Luis
On 5/24/23 07:34, Luis Chamberlain wrote:
On Thu, May 18, 2023 at 06:20:37PM +0300, Dan Carpenter wrote:
On Fri, May 12, 2023 at 08:58:58PM +0200, Mirsad Goran Todorovac wrote:
On 12. 05. 2023. 15:09, Dan Carpenter wrote:
On Fri, May 12, 2023 at 02:34:29PM +0200, Mirsad Todorovac wrote:
@@ -1011,6 +1016,11 @@ ssize_t trigger_batched_requests_async_store(struct device *dev, mutex_lock(&test_fw_mutex);
- if (test_fw_config->reqs) {
rc = -EBUSY;goto out_bail;- }
test_fw_config->reqs = vzalloc(array3_size(sizeof(struct test_batched_req), test_fw_config->num_requests, 2));I was just thinking, since returning -EBUSY for the case of already allocated test_fw_config->reqs was your suggestion and your idea, maybe it would be OK to properly reflect that in Co-developed-by: or Signed-off-by: , but if I understood well, the CoC requires that I am explicitly approved of those?
If everyone else is okay, let's just apply this as-is. You did all the hard bits.
regards, dan carpenter
If it is OK with you, then I hope I have your Reviewed-by:
Wow. Sorry for all the delay on this.
Reviewed-by: Dan Carpenter dan.carpenter@linaro.org
Thanks for doing this work! It looks much better now split up!
No problem. It's a great exercise for the little grey cells :-)
For all 3 patches:
Acked-by: Luis Chamberlain mcgrof@kernel.org
Thanks, Mirsad
Greg, can you pick these up?
Luis
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 --- lib/test_firmware.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/lib/test_firmware.c b/lib/test_firmware.c index 91b232ed3161..1d7d480b8eeb 100644 --- a/lib/test_firmware.c +++ b/lib/test_firmware.c @@ -45,6 +45,7 @@ struct test_batched_req { bool sent; const struct firmware *fw; const char *name; + const char *fw_buf; struct completion completion; struct task_struct *task; struct device *dev; @@ -175,8 +176,14 @@ static void __test_release_all_firmware(void)
for (i = 0; i < test_fw_config->num_requests; i++) { req = &test_fw_config->reqs[i]; - if (req->fw) + if (req->fw) { + if (req->fw_buf) { + kfree_const(req->fw_buf); + req->fw_buf = NULL; + } release_firmware(req->fw); + req->fw = NULL; + } }
vfree(test_fw_config->reqs); @@ -670,6 +677,8 @@ static ssize_t trigger_request_store(struct device *dev,
mutex_lock(&test_fw_mutex); release_firmware(test_firmware); + if (test_fw_config->reqs) + __test_release_all_firmware(); test_firmware = NULL; rc = request_firmware(&test_firmware, name, dev); if (rc) { @@ -770,6 +779,8 @@ static ssize_t trigger_async_request_store(struct device *dev, mutex_lock(&test_fw_mutex); release_firmware(test_firmware); test_firmware = NULL; + if (test_fw_config->reqs) + __test_release_all_firmware(); rc = request_firmware_nowait(THIS_MODULE, 1, name, dev, GFP_KERNEL, NULL, trigger_async_request_cb); if (rc) { @@ -812,6 +823,8 @@ static ssize_t trigger_custom_fallback_store(struct device *dev,
mutex_lock(&test_fw_mutex); release_firmware(test_firmware); + if (test_fw_config->reqs) + __test_release_all_firmware(); test_firmware = NULL; rc = request_firmware_nowait(THIS_MODULE, FW_ACTION_NOUEVENT, name, dev, GFP_KERNEL, NULL, @@ -874,6 +887,8 @@ static int test_fw_run_batch_request(void *data) test_fw_config->buf_size); if (!req->fw) kfree(test_buf); + else + req->fw_buf = test_buf; } else { req->rc = test_fw_config->req_firmware(&req->fw, req->name, @@ -934,6 +949,7 @@ static ssize_t trigger_batched_requests_store(struct device *dev, req->fw = NULL; req->idx = i; req->name = test_fw_config->name; + req->fw_buf = NULL; req->dev = dev; init_completion(&req->completion); req->task = kthread_run(test_fw_run_batch_request, req, @@ -1038,6 +1054,7 @@ ssize_t trigger_batched_requests_async_store(struct device *dev, for (i = 0; i < test_fw_config->num_requests; i++) { req = &test_fw_config->reqs[i]; req->name = test_fw_config->name; + req->fw_buf = NULL; req->fw = NULL; req->idx = i; init_completion(&req->completion);
linux-kselftest-mirror@lists.linaro.org
-
Dan Carpenter -
Luis Chamberlain -
Mirsad Goran Todorovac -
Mirsad Todorovac