Re: linux-next: manual merge of the modules tree with the mm tree

[Stephen Rothwell]

Hi all,

On Fri, 12 Apr 2024 12:04:21 +1000 Stephen Rothwell <sfr@xxxxxxxxxxxxxxxx> wrote:
>
> Today's linux-next merge of the modules tree got a conflict in:
> 
>   kernel/module/main.c
> 
> between commit:
> 
>   58782d7a7ccd ("lib: prevent module unloading if memory is not freed")
> 
> from the mm-unstable branch of the mm tree and commit:
> 
>   a4ee8c9b86bd ("module: make module_memory_{alloc,free} more self-contained")
> 
> from the modules tree.
> 
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging.  You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
> 
> diff --cc kernel/module/main.c
> index 2d25eebc549d,d56b7df0cbb6..000000000000
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@@ -56,8 -56,8 +56,9 @@@
>   #include <linux/dynamic_debug.h>
>   #include <linux/audit.h>
>   #include <linux/cfi.h>
>  +#include <linux/codetag.h>
>   #include <linux/debugfs.h>
> + #include <linux/execmem.h>
>   #include <uapi/linux/module.h>
>   #include "internal.h"
>   
> @@@ -1204,26 -1194,51 +1195,55 @@@ static bool mod_mem_use_vmalloc(enum mo
>   		mod_mem_type_is_core_data(type);
>   }
>   
> - static void *module_memory_alloc(unsigned int size, enum mod_mem_type type)
> + static int module_memory_alloc(struct module *mod, enum mod_mem_type type)
>   {
> + 	unsigned int size = PAGE_ALIGN(mod->mem[type].size);
> + 	void *ptr;
> + 
> + 	mod->mem[type].size = size;
> + 
>   	if (mod_mem_use_vmalloc(type))
> - 		return vzalloc(size);
> - 	return module_alloc(size);
> + 		ptr = vmalloc(size);
> + 	else
> + 		ptr = execmem_alloc(EXECMEM_MODULE_TEXT, size);
> + 
> + 	if (!ptr)
> + 		return -ENOMEM;
> + 
> + 	/*
> + 	 * The pointer to these blocks of memory are stored on the module
> + 	 * structure and we keep that around so long as the module is
> + 	 * around. We only free that memory when we unload the module.
> + 	 * Just mark them as not being a leak then. The .init* ELF
> + 	 * sections *do* get freed after boot so we *could* treat them
> + 	 * slightly differently with kmemleak_ignore() and only grey
> + 	 * them out as they work as typical memory allocations which
> + 	 * *do* eventually get freed, but let's just keep things simple
> + 	 * and avoid *any* false positives.
> + 	 */
> + 	kmemleak_not_leak(ptr);
> + 
> + 	memset(ptr, 0, size);
> + 	mod->mem[type].base = ptr;
> + 
> + 	return 0;
>   }
>   
> - static void module_memory_free(void *ptr, enum mod_mem_type type,
>  -static void module_memory_free(struct module *mod, enum mod_mem_type type)
> ++static void module_memory_free(struct module *mod, enum mod_mem_type type,
>  +			       bool unload_codetags)
>   {
> + 	void *ptr = mod->mem[type].base;
> + 
>  +	if (!unload_codetags && mod_mem_type_is_core_data(type))
>  +		return;
>  +
>   	if (mod_mem_use_vmalloc(type))
>   		vfree(ptr);
>   	else
> - 		module_memfree(ptr);
> + 		execmem_free(ptr);
>   }
>   
>  -static void free_mod_mem(struct module *mod)
>  +static void free_mod_mem(struct module *mod, bool unload_codetags)
>   {
>   	for_each_mod_mem_type(type) {
>   		struct module_memory *mod_mem = &mod->mem[type];
> @@@ -1234,13 -1249,12 +1254,13 @@@
>   		/* Free lock-classes; relies on the preceding sync_rcu(). */
>   		lockdep_free_key_range(mod_mem->base, mod_mem->size);
>   		if (mod_mem->size)
> - 			module_memory_free(mod_mem->base, type,
>  -			module_memory_free(mod, type);
> ++			module_memory_free(mod, type,
>  +					   unload_codetags);
>   	}
>   
>   	/* MOD_DATA hosts mod, so free it at last */
>   	lockdep_free_key_range(mod->mem[MOD_DATA].base, mod->mem[MOD_DATA].size);
> - 	module_memory_free(mod->mem[MOD_DATA].base, MOD_DATA, unload_codetags);
>  -	module_memory_free(mod, MOD_DATA);
> ++	module_memory_free(mod, MOD_DATA, unload_codetags);
>   }
>   
>   /* Free a module, remove from lists, etc. */
> @@@ -2309,7 -2301,7 +2314,7 @@@ static int move_module(struct module *m
>   	return 0;
>   out_enomem:
>   	for (t--; t >= 0; t--)
> - 		module_memory_free(mod->mem[t].base, t, true);
>  -		module_memory_free(mod, t);
> ++		module_memory_free(mod, t, true);
>   	return ret;
>   }
>   

This is now a conflict between the mm-stable tree and Linus' tree.

-- 
Cheers,
Stephen Rothwell

Attachment: pgptZssAxvSbH.pgp
Description: OpenPGP digital signature