The module_load callback should still run for builtin codetags that
define it, even in a non-modular kernel. (i.e. for the cmod->mod == NULL
case).

Signed-off-by: Kees Cook <kees@kernel.org>
---
Cc: Suren Baghdasaryan <surenb@google.com>
Cc: Kent Overstreet <kent.overstreet@linux.dev>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Christoph Lameter <cl@linux.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Roman Gushchin <roman.gushchin@linux.dev>
Cc: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Cc: linux-mm@kvack.org
---
 lib/codetag.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/lib/codetag.c b/lib/codetag.c
index 5ace625f2328..ef7634c7ee18 100644
--- a/lib/codetag.c
+++ b/lib/codetag.c
@@ -125,7 +125,6 @@ static inline size_t range_size(const struct codetag_type *cttype,
 			cttype->desc.tag_size;
 }
 
-#ifdef CONFIG_MODULES
 static void *get_symbol(struct module *mod, const char *prefix, const char *name)
 {
 	DECLARE_SEQ_BUF(sb, KSYM_NAME_LEN);
@@ -199,6 +198,7 @@ static int codetag_module_init(struct codetag_type *cttype, struct module *mod)
 	return 0;
 }
 
+#ifdef CONFIG_MODULES
 void codetag_load_module(struct module *mod)
 {
 	struct codetag_type *cttype;
@@ -248,9 +248,6 @@ bool codetag_unload_module(struct module *mod)
 
 	return unload_ok;
 }
-
-#else /* CONFIG_MODULES */
-static int codetag_module_init(struct codetag_type *cttype, struct module *mod) { return 0; }
 #endif /* CONFIG_MODULES */
 
 struct codetag_type *
-- 
2.34.1

On Fri, Aug 9, 2024 at 12:33 AM Kees Cook <kees@kernel.org> wrote:
>
> The module_load callback should still run for builtin codetags that
> define it, even in a non-modular kernel. (i.e. for the cmod->mod == NULL
> case).
>
> Signed-off-by: Kees Cook <kees@kernel.org>

Hi Kees,
I finally got some time and started reviewing your patches.
Coincidentally I recently posted a fix for this issue at
https://lore.kernel.org/all/20240828231536.1770519-1-surenb@google.com/
Your fix is missing a small part when codetag_module_init() is using
mod->name while struct module is undefined (CONFIG_MODULES=n) and you
should see this build error:

In file included from ./include/linux/kernel.h:31,
                 from ./include/linux/cpumask.h:11,
                 from ./include/linux/smp.h:13,
                 from ./include/linux/lockdep.h:14,
                 from ./include/linux/radix-tree.h:14,
                 from ./include/linux/idr.h:15,
                 from lib/codetag.c:3:
lib/codetag.c: In function ‘codetag_module_init’:
  CC      drivers/acpi/acpica/extrace.o
lib/codetag.c:167:34: error: invalid use of undefined type ‘struct module’
  167 |                         mod ? mod->name : "(built-in)");
      |                                  ^~

Thanks,
Suren.


> ---
> Cc: Suren Baghdasaryan <surenb@google.com>
> Cc: Kent Overstreet <kent.overstreet@linux.dev>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Christoph Lameter <cl@linux.com>
> Cc: Pekka Enberg <penberg@kernel.org>
> Cc: David Rientjes <rientjes@google.com>
> Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Roman Gushchin <roman.gushchin@linux.dev>
> Cc: Hyeonggon Yoo <42.hyeyoo@gmail.com>
> Cc: linux-mm@kvack.org
> ---
>  lib/codetag.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/lib/codetag.c b/lib/codetag.c
> index 5ace625f2328..ef7634c7ee18 100644
> --- a/lib/codetag.c
> +++ b/lib/codetag.c
> @@ -125,7 +125,6 @@ static inline size_t range_size(const struct codetag_type *cttype,
>                         cttype->desc.tag_size;
>  }
>
> -#ifdef CONFIG_MODULES
>  static void *get_symbol(struct module *mod, const char *prefix, const char *name)
>  {
>         DECLARE_SEQ_BUF(sb, KSYM_NAME_LEN);
> @@ -199,6 +198,7 @@ static int codetag_module_init(struct codetag_type *cttype, struct module *mod)
>         return 0;
>  }
>
> +#ifdef CONFIG_MODULES
>  void codetag_load_module(struct module *mod)
>  {
>         struct codetag_type *cttype;
> @@ -248,9 +248,6 @@ bool codetag_unload_module(struct module *mod)
>
>         return unload_ok;
>  }
> -
> -#else /* CONFIG_MODULES */
> -static int codetag_module_init(struct codetag_type *cttype, struct module *mod) { return 0; }
>  #endif /* CONFIG_MODULES */
>
>  struct codetag_type *
> --
> 2.34.1
>

On Thu, Aug 29, 2024 at 08:02:13AM -0700, Suren Baghdasaryan wrote:
> On Fri, Aug 9, 2024 at 12:33 AM Kees Cook <kees@kernel.org> wrote:
> >
> > The module_load callback should still run for builtin codetags that
> > define it, even in a non-modular kernel. (i.e. for the cmod->mod == NULL
> > case).
> >
> > Signed-off-by: Kees Cook <kees@kernel.org>
> 
> Hi Kees,
> I finally got some time and started reviewing your patches.
> Coincidentally I recently posted a fix for this issue at
> https://lore.kernel.org/all/20240828231536.1770519-1-surenb@google.com/
> Your fix is missing a small part when codetag_module_init() is using
> mod->name while struct module is undefined (CONFIG_MODULES=n) and you
> should see this build error:
> 
> In file included from ./include/linux/kernel.h:31,
>                  from ./include/linux/cpumask.h:11,
>                  from ./include/linux/smp.h:13,
>                  from ./include/linux/lockdep.h:14,
>                  from ./include/linux/radix-tree.h:14,
>                  from ./include/linux/idr.h:15,
>                  from lib/codetag.c:3:
> lib/codetag.c: In function ‘codetag_module_init’:
>   CC      drivers/acpi/acpica/extrace.o
> lib/codetag.c:167:34: error: invalid use of undefined type ‘struct module’
>   167 |                         mod ? mod->name : "(built-in)");
>       |                                  ^~

Ah-ha! Excellent. Thanks; I will double-check that your version of this
doesn't have any surprises for how I was using it here. I expect it'll
be fine.

-Kees

-- 
Kees Cook