Some functions were called both from locked and unlocked context, so
the lock was dropped prematurely, introducing a race condition when
deadlock was avoided.
Having two locks wouldn't assure a race-proof mutual exclusion.
__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 causing a race condition.
This should guarantee mutual exclusion and prevent any race conditions.
Locked versions simply allow for mutual exclusion and call the unlocked
counterparts, to avoid duplication of code.
trigger_batched_requests_store() and trigger_batched_requests_async_store()
now return -EBUSY if called with test_fw_config->reqs already allocated,
so the memory leak is prevented.
The same functions now keep track of the allocated buf for firmware in
req->fw_buf as release_firmware() will not deallocate this storage for us.
Additionally, in __test_release_all_firmware(), req->fw_buf is released
before calling release_firmware(req->fw),
foreach test_fw_config->reqs[i], i = 0 .. test_fw_config->num_requests-1
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Luis Chamberlain <[email protected]>
Cc: Russ Weight <[email protected]>
Cc: Tianfei zhang <[email protected]>
Cc: Christophe JAILLET <[email protected]>
Cc: Zhengchao Shao <[email protected]>
Cc: Colin Ian King <[email protected]>
Cc: [email protected]
Cc: Takashi Iwai <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Scott Branden <[email protected]>
Cc: Luis R. Rodriguez <[email protected]>
Suggested-by: Dan Carpenter <[email protected]>
Signed-off-by: Mirsad Goran Todorovac <[email protected]>
---
v3 -> v4
- fix additional memory leaks of the allocated firmware buffers
- fix noticed racing conditions in conformance with the existing code
- make it a single patch
lib/test_firmware.c | 81 +++++++++++++++++++++++++++++++++++----------
1 file changed, 63 insertions(+), 18 deletions(-)
diff --git a/lib/test_firmware.c b/lib/test_firmware.c
index 05ed84c2fc4c..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);
@@ -353,16 +360,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 +390,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 +402,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 +418,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 +427,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 +496,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 +543,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 +573,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;
@@ -652,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) {
@@ -752,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) {
@@ -794,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,
@@ -856,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,
@@ -895,6 +928,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));
@@ -911,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,
@@ -993,6 +1032,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));
@@ -1010,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);
--
2.30.2
On Fri, Apr 21, 2023 at 08:52:06PM +0200, Mirsad Goran Todorovac wrote:
> Some functions were called both from locked and unlocked context, so
> the lock was dropped prematurely, introducing a race condition when
> deadlock was avoided.
>
> Having two locks wouldn't assure a race-proof mutual exclusion.
>
> __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 causing a race condition.
>
> This should guarantee mutual exclusion and prevent any race conditions.
>
> Locked versions simply allow for mutual exclusion and call the unlocked
> counterparts, to avoid duplication of code.
>
> trigger_batched_requests_store() and trigger_batched_requests_async_store()
> now return -EBUSY if called with test_fw_config->reqs already allocated,
> so the memory leak is prevented.
>
> The same functions now keep track of the allocated buf for firmware in
> req->fw_buf as release_firmware() will not deallocate this storage for us.
>
> Additionally, in __test_release_all_firmware(), req->fw_buf is released
> before calling release_firmware(req->fw),
> foreach test_fw_config->reqs[i], i = 0 .. test_fw_config->num_requests-1
>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Luis Chamberlain <[email protected]>
> Cc: Russ Weight <[email protected]>
> Cc: Tianfei zhang <[email protected]>
> Cc: Christophe JAILLET <[email protected]>
> Cc: Zhengchao Shao <[email protected]>
> Cc: Colin Ian King <[email protected]>
> Cc: [email protected]
> Cc: Takashi Iwai <[email protected]>
> Cc: Kees Cook <[email protected]>
> Cc: Scott Branden <[email protected]>
> Cc: Luis R. Rodriguez <[email protected]>
> Suggested-by: Dan Carpenter <[email protected]>
> Signed-off-by: Mirsad Goran Todorovac <[email protected]>
Mirad, thanks for this work, good stuff! So the patch just needs to be
adjust with:
Fixes: 7feebfa487b92 ("test_firmware: add support for request_firmware_into_buf"
Cc: [email protected] # v5.4
Then, can you split the patch in two, one which fixes the memory leaks
and another that deals with the mutexes. The second patch might be a fix
for the original code but I can't tell until I see the changes split out.
The commit log should account for the memory leak and be clear how it
happens. The other commit log for the second patch should clarify what
it fixes and why as well.
Luis
On 25.4.2023. 20:27, Luis Chamberlain wrote:
> On Fri, Apr 21, 2023 at 08:52:06PM +0200, Mirsad Goran Todorovac wrote:
>> Some functions were called both from locked and unlocked context, so
>> the lock was dropped prematurely, introducing a race condition when
>> deadlock was avoided.
>>
>> Having two locks wouldn't assure a race-proof mutual exclusion.
>>
>> __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 causing a race condition.
>>
>> This should guarantee mutual exclusion and prevent any race conditions.
>>
>> Locked versions simply allow for mutual exclusion and call the unlocked
>> counterparts, to avoid duplication of code.
>>
>> trigger_batched_requests_store() and trigger_batched_requests_async_store()
>> now return -EBUSY if called with test_fw_config->reqs already allocated,
>> so the memory leak is prevented.
>>
>> The same functions now keep track of the allocated buf for firmware in
>> req->fw_buf as release_firmware() will not deallocate this storage for us.
>>
>> Additionally, in __test_release_all_firmware(), req->fw_buf is released
>> before calling release_firmware(req->fw),
>> foreach test_fw_config->reqs[i], i = 0 .. test_fw_config->num_requests-1
>>
>> Cc: Greg Kroah-Hartman <[email protected]>
>> Cc: Luis Chamberlain <[email protected]>
>> Cc: Russ Weight <[email protected]>
>> Cc: Tianfei zhang <[email protected]>
>> Cc: Christophe JAILLET <[email protected]>
>> Cc: Zhengchao Shao <[email protected]>
>> Cc: Colin Ian King <[email protected]>
>> Cc: [email protected]
>> Cc: Takashi Iwai <[email protected]>
>> Cc: Kees Cook <[email protected]>
>> Cc: Scott Branden <[email protected]>
>> Cc: Luis R. Rodriguez <[email protected]>
>> Suggested-by: Dan Carpenter <[email protected]>
>> Signed-off-by: Mirsad Goran Todorovac <[email protected]>
>
> Mirad, thanks for this work, good stuff! So the patch just needs to be
> adjust with:
>
> Fixes: 7feebfa487b92 ("test_firmware: add support for request_firmware_into_buf"
> Cc: [email protected] # v5.4
>
> Then, can you split the patch in two, one which fixes the memory leaks
> and another that deals with the mutexes. The second patch might be a fix
> for the original code but I can't tell until I see the changes split out.
>
> The commit log should account for the memory leak and be clear how it
> happens. The other commit log for the second patch should clarify what
> it fixes and why as well.
It seems to me that there is something wrong with the patchwork, as this commit
had not yet appeared in 5.4 LTS stable tree?
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/log/?qt=grep&q=Mirsad&h=linux-5.4.y
I was accidentally looking whether it passed, and it seems like more than a month
since the other commits propagated :-/
Thanks,
Mirsad
--
Mirsad Todorovac
System engineer
Faculty of Graphic Arts | Academy of Fine Arts
University of Zagreb
Republic of Croatia, the European Union
Sistem inženjer
Grafički fakultet | Akademija likovnih umjetnosti
Sveučilište u Zagrebu
On Thu, Jul 27, 2023 at 08:16:17AM +0200, Mirsad Todorovac wrote:
> On 25.4.2023. 20:27, Luis Chamberlain wrote:
> > On Fri, Apr 21, 2023 at 08:52:06PM +0200, Mirsad Goran Todorovac wrote:
> > > Some functions were called both from locked and unlocked context, so
> > > the lock was dropped prematurely, introducing a race condition when
> > > deadlock was avoided.
> > >
> > > Having two locks wouldn't assure a race-proof mutual exclusion.
> > >
> > > __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 causing a race condition.
> > >
> > > This should guarantee mutual exclusion and prevent any race conditions.
> > >
> > > Locked versions simply allow for mutual exclusion and call the unlocked
> > > counterparts, to avoid duplication of code.
> > >
> > > trigger_batched_requests_store() and trigger_batched_requests_async_store()
> > > now return -EBUSY if called with test_fw_config->reqs already allocated,
> > > so the memory leak is prevented.
> > >
> > > The same functions now keep track of the allocated buf for firmware in
> > > req->fw_buf as release_firmware() will not deallocate this storage for us.
> > >
> > > Additionally, in __test_release_all_firmware(), req->fw_buf is released
> > > before calling release_firmware(req->fw),
> > > foreach test_fw_config->reqs[i], i = 0 .. test_fw_config->num_requests-1
> > >
> > > Cc: Greg Kroah-Hartman <[email protected]>
> > > Cc: Luis Chamberlain <[email protected]>
> > > Cc: Russ Weight <[email protected]>
> > > Cc: Tianfei zhang <[email protected]>
> > > Cc: Christophe JAILLET <[email protected]>
> > > Cc: Zhengchao Shao <[email protected]>
> > > Cc: Colin Ian King <[email protected]>
> > > Cc: [email protected]
> > > Cc: Takashi Iwai <[email protected]>
> > > Cc: Kees Cook <[email protected]>
> > > Cc: Scott Branden <[email protected]>
> > > Cc: Luis R. Rodriguez <[email protected]>
> > > Suggested-by: Dan Carpenter <[email protected]>
> > > Signed-off-by: Mirsad Goran Todorovac <[email protected]>
> >
> > Mirad, thanks for this work, good stuff! So the patch just needs to be
> > adjust with:
> >
> > Fixes: 7feebfa487b92 ("test_firmware: add support for request_firmware_into_buf"
> > Cc: [email protected] # v5.4
> >
> > Then, can you split the patch in two, one which fixes the memory leaks
> > and another that deals with the mutexes. The second patch might be a fix
> > for the original code but I can't tell until I see the changes split out.
> >
> > The commit log should account for the memory leak and be clear how it
> > happens. The other commit log for the second patch should clarify what
> > it fixes and why as well.
>
> It seems to me that there is something wrong with the patchwork, as this commit
> had not yet appeared in 5.4 LTS stable tree?
Did you resend a new v5 with the requested changes?
Luis
On 7/27/23 17:36, Luis Chamberlain wrote:
> On Thu, Jul 27, 2023 at 08:16:17AM +0200, Mirsad Todorovac wrote:
>> On 25.4.2023. 20:27, Luis Chamberlain wrote:
>>> On Fri, Apr 21, 2023 at 08:52:06PM +0200, Mirsad Goran Todorovac wrote:
>>>> Some functions were called both from locked and unlocked context, so
>>>> the lock was dropped prematurely, introducing a race condition when
>>>> deadlock was avoided.
>>>>
>>>> Having two locks wouldn't assure a race-proof mutual exclusion.
>>>>
>>>> __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 causing a race condition.
>>>>
>>>> This should guarantee mutual exclusion and prevent any race conditions.
>>>>
>>>> Locked versions simply allow for mutual exclusion and call the unlocked
>>>> counterparts, to avoid duplication of code.
>>>>
>>>> trigger_batched_requests_store() and trigger_batched_requests_async_store()
>>>> now return -EBUSY if called with test_fw_config->reqs already allocated,
>>>> so the memory leak is prevented.
>>>>
>>>> The same functions now keep track of the allocated buf for firmware in
>>>> req->fw_buf as release_firmware() will not deallocate this storage for us.
>>>>
>>>> Additionally, in __test_release_all_firmware(), req->fw_buf is released
>>>> before calling release_firmware(req->fw),
>>>> foreach test_fw_config->reqs[i], i = 0 .. test_fw_config->num_requests-1
>>>>
>>>> Cc: Greg Kroah-Hartman <[email protected]>
>>>> Cc: Luis Chamberlain <[email protected]>
>>>> Cc: Russ Weight <[email protected]>
>>>> Cc: Tianfei zhang <[email protected]>
>>>> Cc: Christophe JAILLET <[email protected]>
>>>> Cc: Zhengchao Shao <[email protected]>
>>>> Cc: Colin Ian King <[email protected]>
>>>> Cc: [email protected]
>>>> Cc: Takashi Iwai <[email protected]>
>>>> Cc: Kees Cook <[email protected]>
>>>> Cc: Scott Branden <[email protected]>
>>>> Cc: Luis R. Rodriguez <[email protected]>
>>>> Suggested-by: Dan Carpenter <[email protected]>
>>>> Signed-off-by: Mirsad Goran Todorovac <[email protected]>
>>>
>>> Mirad, thanks for this work, good stuff! So the patch just needs to be
>>> adjust with:
>>>
>>> Fixes: 7feebfa487b92 ("test_firmware: add support for request_firmware_into_buf"
>>> Cc: [email protected] # v5.4
>>>
>>> Then, can you split the patch in two, one which fixes the memory leaks
>>> and another that deals with the mutexes. The second patch might be a fix
>>> for the original code but I can't tell until I see the changes split out.
>>>
>>> The commit log should account for the memory leak and be clear how it
>>> happens. The other commit log for the second patch should clarify what
>>> it fixes and why as well.
>>
>> It seems to me that there is something wrong with the patchwork, as this commit
>> had not yet appeared in 5.4 LTS stable tree?
>
> Did you resend a new v5 with the requested changes?
Actually, I am not sure what is the procedure, for it is accepted and visible in 5.10+ trees?
Mirsad
On Fri, Jul 28, 2023 at 12:14:22AM +0200, Mirsad Todorovac wrote:
> On 7/27/23 17:36, Luis Chamberlain wrote:
> > On Thu, Jul 27, 2023 at 08:16:17AM +0200, Mirsad Todorovac wrote:
> > > On 25.4.2023. 20:27, Luis Chamberlain wrote:
> > > > On Fri, Apr 21, 2023 at 08:52:06PM +0200, Mirsad Goran Todorovac wrote:
> > > > > Some functions were called both from locked and unlocked context, so
> > > > > the lock was dropped prematurely, introducing a race condition when
> > > > > deadlock was avoided.
> > > > >
> > > > > Having two locks wouldn't assure a race-proof mutual exclusion.
> > > > >
> > > > > __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 causing a race condition.
> > > > >
> > > > > This should guarantee mutual exclusion and prevent any race conditions.
> > > > >
> > > > > Locked versions simply allow for mutual exclusion and call the unlocked
> > > > > counterparts, to avoid duplication of code.
> > > > >
> > > > > trigger_batched_requests_store() and trigger_batched_requests_async_store()
> > > > > now return -EBUSY if called with test_fw_config->reqs already allocated,
> > > > > so the memory leak is prevented.
> > > > >
> > > > > The same functions now keep track of the allocated buf for firmware in
> > > > > req->fw_buf as release_firmware() will not deallocate this storage for us.
> > > > >
> > > > > Additionally, in __test_release_all_firmware(), req->fw_buf is released
> > > > > before calling release_firmware(req->fw),
> > > > > foreach test_fw_config->reqs[i], i = 0 .. test_fw_config->num_requests-1
> > > > >
> > > > > Cc: Greg Kroah-Hartman <[email protected]>
> > > > > Cc: Luis Chamberlain <[email protected]>
> > > > > Cc: Russ Weight <[email protected]>
> > > > > Cc: Tianfei zhang <[email protected]>
> > > > > Cc: Christophe JAILLET <[email protected]>
> > > > > Cc: Zhengchao Shao <[email protected]>
> > > > > Cc: Colin Ian King <[email protected]>
> > > > > Cc: [email protected]
> > > > > Cc: Takashi Iwai <[email protected]>
> > > > > Cc: Kees Cook <[email protected]>
> > > > > Cc: Scott Branden <[email protected]>
> > > > > Cc: Luis R. Rodriguez <[email protected]>
> > > > > Suggested-by: Dan Carpenter <[email protected]>
> > > > > Signed-off-by: Mirsad Goran Todorovac <[email protected]>
> > > >
> > > > Mirad, thanks for this work, good stuff! So the patch just needs to be
> > > > adjust with:
> > > >
> > > > Fixes: 7feebfa487b92 ("test_firmware: add support for request_firmware_into_buf"
> > > > Cc: [email protected] # v5.4
> > > >
> > > > Then, can you split the patch in two, one which fixes the memory leaks
> > > > and another that deals with the mutexes. The second patch might be a fix
> > > > for the original code but I can't tell until I see the changes split out.
> > > >
> > > > The commit log should account for the memory leak and be clear how it
> > > > happens. The other commit log for the second patch should clarify what
> > > > it fixes and why as well.
> > >
> > > It seems to me that there is something wrong with the patchwork, as this commit
> > > had not yet appeared in 5.4 LTS stable tree?
> >
> > Did you resend a new v5 with the requested changes?
>
> Actually, I am not sure what is the procedure, for it is accepted and visible in 5.10+ trees?
Ah its upstream but not on v5.4. Just send a patch to stable folks and
include its commit IDs as stable patches typically do.
Luis