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:
pgprFBC4cSMCs.pgp
Description: OpenPGP digital signature
