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: <97f26140-bf53-4c4d-bf63-2dd353a3ec85@suse.com>
Date: Mon, 16 Jun 2025 15:58:09 +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/14/25 11:28 PM, Daniel Gomez wrote:
>> This seems to be off by one. For instance, if the loop reaches the last
>> valid type in mod_mem_type, MOD_INIT_RODATA, and successfully allocates
>> its memory, the variable t gets set to MOD_INIT_RODATA. Subsequently, if
>> an error occurs later in move_module() and control is transferred to
>> out_err, the deallocation starts from t-1, and therefore MOD_INIT_RODATA
>> doesn't get freed.
>>
>> If we want to always start from the last type found, the code would need
>> to be:
>>
>> 		[...]
>> 		ret = module_memory_alloc(mod, type);
>> 		if (ret)
>> 			goto out_err;
>> 		t = type + 1;
>> 	}
>>
>> I can adjust it in this way if it is preferred.
>>
> 
> My earlier suggestion was incorrect. We can simply initialize the memory
> type t to MOD_MEM_NUM_TYPES since it's only used in the error path of
> module_memory_alloc().

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;
		}
	}

	[...]
}

-- 
Thanks,
Petr