Re: [PATCH v2 1/2] module: Fix memory deallocation on error path in move_module()

[Daniel Gomez]

On 18/06/2025 14.26, Petr Pavlu wrote:
> The function move_module() uses the variable t to track how many memory
> types it has allocated and consequently how many should be freed if an
> error occurs.
> 
> The variable is initially set to 0 and is updated when a call to
> module_memory_alloc() fails. However, move_module() can fail for other
> reasons as well, in which case t remains set to 0 and no memory is freed.
> 
> Fix the problem by initializing t to MOD_MEM_NUM_TYPES. Additionally, make
> the deallocation loop more robust by not relying on the mod_mem_type_t enum
> having a signed integer as its underlying type.
> 
> Fixes: c7ee8aebf6c0 ("module: add stop-grap sanity check on module memcpy()")
> Signed-off-by: Petr Pavlu <petr.pavlu@xxxxxxxx>
> Reviewed-by: Sami Tolvanen <samitolvanen@xxxxxxxxxx>
> ---
>  kernel/module/main.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index 413ac6ea3702..9ac994b2f354 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -2697,7 +2697,7 @@ static int find_module_sections(struct module *mod, struct load_info *info)
>  static int move_module(struct module *mod, struct load_info *info)
>  {
>  	int i;
> -	enum mod_mem_type t = 0;
> +	enum mod_mem_type t = MOD_MEM_NUM_TYPES;
>  	int ret = -ENOMEM;
>  	bool codetag_section_found = false;
>  
> @@ -2776,7 +2776,7 @@ static int move_module(struct module *mod, struct load_info *info)
>  	return 0;
>  out_err:
>  	module_memory_restore_rox(mod);
> -	for (t--; t >= 0; t--)
> +	while (t--)
>  		module_memory_free(mod, t);
>  	if (codetag_section_found)
>  		codetag_free_module_sections(mod);

Reviewed-by: Daniel Gomez <da.gomez@xxxxxxxxxxx>