linux-kernel - Re: [PATCH 1/2] module: Fix memory deallocation on error path in move

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2c277bfe-3086-4007-bf04-ef229e6cbfb7@suse.com>
Date: Tue, 17 Jun 2025 17:41:22 +0200
From: Petr Pavlu <petr.pavlu@...e.com>
To: Daniel Gomez <da.gomez@...nel.org>
Cc: Luis Chamberlain <mcgrof@...nel.org>,
 Sami Tolvanen <samitolvanen@...gle.com>, Daniel Gomez
 <da.gomez@...sung.com>, linux-modules@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] module: Fix memory deallocation on error path in
 move_module()

On 6/17/25 11:47 AM, Daniel Gomez wrote:
>> Do you mean the following, or something else:
>>
>> static int move_module(struct module *mod, struct load_info *info)
>> {
>> 	int i;
>> 	enum mod_mem_type t = MOD_MEM_NUM_TYPES;
>> 	int ret;
>> 	bool codetag_section_found = false;
>>
>> 	for_each_mod_mem_type(type) {
>> 		if (!mod->mem[type].size) {
>> 			mod->mem[type].base = NULL;
>> 			continue;
>> 		}
>>
>> 		ret = module_memory_alloc(mod, type);
>> 		if (ret) {
>> 			t = type;
>> 			goto out_err;
>> 		}
>> 	}
>>
>> 	[...]
>> }
>>
> 
> Yes, that's it. From your patch, moving MOD_MEM_NUM_TYPE assigment to the
> initialization and use the while() loop suggested later on.

Ok.

> 
> One thing though, we are "releasing" the memory even if we have skipped the
> allocation in the first place. So, I think it would make sense to release only
> for the types we have actually allocated. What do you think?

I noticed this too, specifically because move_module() is inconsistent
in this regard with free_mod_mem(). The latter function contains:

if (mod_mem->size)
	module_memory_free(mod, type);

However, my preference is actually to update free_mod_mem() and remove
the check. The function module_memory_free() should be a no-op if
mod->base is NULL, similarly to how calling free(NULL) is a no-op.

-- 
Thanks,
Petr