Re: [PATCH v4 1/1] test_firmware: fix some memory leaks and racing conditions

[Greg Kroah-Hartman]

On Fri, Apr 21, 2023 at 04:57:01PM +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(reqs->fw) foreach test_fw_config->regs[i].
> 
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> 
> Cc: Luis Chamberlain <mcgrof@xxxxxxxxxx> 
> Cc: Russ Weight <russell.h.weight@xxxxxxxxx> 
> Cc: Tianfei zhang <tianfei.zhang@xxxxxxxxx> 
> Cc: Christophe JAILLET <christophe.jaillet@xxxxxxxxxx> 
> Cc: Zhengchao Shao <shaozhengchao@xxxxxxxxxx> 
> Cc: Colin Ian King <colin.i.king@xxxxxxxxx> 
> Cc: linux-kernel@xxxxxxxxxxxxxxx 
> Cc: Takashi Iwai <tiwai@xxxxxxx>
> Cc: Kees Cook <keescook@xxxxxxxxxxxx>
> Cc: Scott Branden <sbranden@xxxxxxxxxxxx>
> Cc: Luis R. Rodriguez <mcgrof@xxxxxxxxxx>
> Suggested-by: Dan Carpenter <error27@xxxxxxxxx>
> Signed-off-by: Mirsad Goran Todorovac <mirsad.todorovac@xxxxxxxxxxxx>
> ---
>  lib/test_firmware.c | 81 +++++++++++++++++++++++++++++++++++----------
>  1 file changed, 63 insertions(+), 18 deletions(-)
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- This looks like a new version of a previously submitted patch, but you
  did not list below the --- line any changes from the previous version.
  Please read the section entitled "The canonical patch format" in the
  kernel file, Documentation/process/submitting-patches.rst for what
  needs to be done here to properly describe this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot