mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Petr Pavlu <petr.pavlu@suse.com>
To: Luis Chamberlain <mcgrof@kernel.org>,
	Sami Tolvanen <samitolvanen@google.com>,
	Daniel Gomez <da.gomez@samsung.com>
Cc: linux-modules@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] module: Fix memory deallocation on error path in move_module()
Date: Sun, 8 Jun 2025 09:25:34 +0200	[thread overview]
Message-ID: <f6fa3df3-38d5-4191-96d1-9a8a2152cedf@suse.com> (raw)
In-Reply-To: <20250607161823.409691-2-petr.pavlu@suse.com>

On 6/7/25 6:16 PM, 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 setting t to MOD_MEM_NUM_TYPES after all memory types
> have been allocated. 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@suse.com>
> ---
>  kernel/module/main.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index 08b59c37735e..322b38c0a782 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> [...]
>  	pr_debug("Final section addresses for %s:\n", mod->name);
> @@ -2693,8 +2694,8 @@ 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--)
> -		module_memory_free(mod, t);
> +	for (; t > 0; t--)
> +		module_memory_free(mod, t - 1);
>  	if (codetag_section_found)
>  		codetag_free_module_sections(mod);
>  

This can actually be simply:

	while (t--)
		module_memory_free(mod, t);

-- Petr

  reply	other threads:[~2025-06-08  7:25 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-07 16:16 [PATCH 0/2] " Petr Pavlu
2025-06-07 16:16 ` [PATCH 1/2] " Petr Pavlu
2025-06-08  7:25   ` Petr Pavlu [this message]
2025-06-09 22:12     ` Sami Tolvanen
2025-06-10 18:51   ` Daniel Gomez
2025-06-12 10:48     ` Petr Pavlu
2025-06-14 21:28       ` Daniel Gomez
2025-06-16 13:58         ` Petr Pavlu
2025-06-17  9:47           ` Daniel Gomez
2025-06-17 15:41             ` Petr Pavlu
2025-06-17 17:17               ` Daniel Gomez
2025-06-07 16:16 ` [PATCH 2/2] module: Avoid unnecessary return value initialization " Petr Pavlu
2025-06-09 22:13   ` Sami Tolvanen
2025-06-10 18:56   ` Daniel Gomez

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f6fa3df3-38d5-4191-96d1-9a8a2152cedf@suse.com \
    --to=petr.pavlu@suse.com \
    --cc=da.gomez@samsung.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-modules@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=samitolvanen@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®