* [PATCH v6 5/9] pinctrl: meson: Fix typo in device table macro
From: Alexey Gladkov @ 2025-08-14 13:07 UTC (permalink / raw)
To: Nathan Chancellor, Nicolas Schier, Masahiro Yamada, Petr Pavlu,
Luis Chamberlain, Sami Tolvanen, Daniel Gomez
Cc: linux-kernel, linux-modules, linux-kbuild, Alexey Gladkov,
Xianwei Zhao, Linus Walleij, Neil Armstrong, Kevin Hilman,
linux-amlogic, linux-gpio, kernel test robot
In-Reply-To: <cover.1755170493.git.legion@kernel.org>
The typo when using the MODULE_DEVICE_TABLE macro was not noticeable
because the macro was defined only if the module was built as a separate
module.
Cc: Xianwei Zhao <xianwei.zhao@amlogic.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Neil Armstrong <neil.armstrong@linaro.org>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc: linux-amlogic@lists.infradead.org
Cc: linux-gpio@vger.kernel.org
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202507220009.8HKbNP16-lkp@intel.com/
Signed-off-by: Alexey Gladkov <legion@kernel.org>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
---
drivers/pinctrl/meson/pinctrl-amlogic-a4.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pinctrl/meson/pinctrl-amlogic-a4.c b/drivers/pinctrl/meson/pinctrl-amlogic-a4.c
index e34e984c2b38..6132710aff68 100644
--- a/drivers/pinctrl/meson/pinctrl-amlogic-a4.c
+++ b/drivers/pinctrl/meson/pinctrl-amlogic-a4.c
@@ -1093,7 +1093,7 @@ static const struct of_device_id aml_pctl_of_match[] = {
{ .compatible = "amlogic,pinctrl-s6", .data = &s6_priv_data, },
{ /* sentinel */ }
};
-MODULE_DEVICE_TABLE(of, aml_pctl_dt_match);
+MODULE_DEVICE_TABLE(of, aml_pctl_of_match);
static struct platform_driver aml_pctl_driver = {
.driver = {
--
2.50.1
^ permalink raw reply related
* [PATCH v6 6/9] modpost: Add modname to mod_device_table alias
From: Alexey Gladkov @ 2025-08-14 13:07 UTC (permalink / raw)
To: Nathan Chancellor, Nicolas Schier, Masahiro Yamada, Petr Pavlu,
Luis Chamberlain, Sami Tolvanen, Daniel Gomez
Cc: linux-kernel, linux-modules, linux-kbuild, Alexey Gladkov,
Miguel Ojeda, Andreas Hindborg, Danilo Krummrich, Alex Gaynor,
rust-for-linux
In-Reply-To: <cover.1755170493.git.legion@kernel.org>
At this point, if a symbol is compiled as part of the kernel,
information about which module the symbol belongs to is lost.
To save this it is possible to add the module name to the alias name.
It's not very pretty, but it's possible for now.
Cc: Miguel Ojeda <ojeda@kernel.org>
Cc: Andreas Hindborg <a.hindborg@kernel.org>
Cc: Danilo Krummrich <dakr@kernel.org>
Cc: Alex Gaynor <alex.gaynor@gmail.com>
Cc: rust-for-linux@vger.kernel.org
Signed-off-by: Alexey Gladkov <legion@kernel.org>
---
include/linux/module.h | 14 +++++++++++++-
rust/kernel/device_id.rs | 8 ++++----
scripts/mod/file2alias.c | 18 ++++++++++++++----
3 files changed, 31 insertions(+), 9 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index 3319a5269d28..e31ee29fac6b 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -244,10 +244,22 @@ struct module_kobject *lookup_or_create_module_kobject(const char *name);
/* What your module does. */
#define MODULE_DESCRIPTION(_description) MODULE_INFO(description, _description)
+/*
+ * Format: __mod_device_table__kmod_<modname>__<type>__<name>
+ * Parts of the string `__kmod_` and `__` are used as delimiters when parsing
+ * a symbol in file2alias.c
+ */
+#define __mod_device_table(type, name) \
+ __PASTE(__mod_device_table__, \
+ __PASTE(__KBUILD_MODNAME, \
+ __PASTE(__, \
+ __PASTE(type, \
+ __PASTE(__, name)))))
+
#ifdef MODULE
/* Creates an alias so file2alias.c can find device table. */
#define MODULE_DEVICE_TABLE(type, name) \
-static typeof(name) __mod_device_table__##type##__##name \
+static typeof(name) __mod_device_table(type, name) \
__attribute__ ((used, alias(__stringify(name))))
#else /* !MODULE */
#define MODULE_DEVICE_TABLE(type, name)
diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
index 70d57814ff79..62c42da12e9d 100644
--- a/rust/kernel/device_id.rs
+++ b/rust/kernel/device_id.rs
@@ -195,10 +195,10 @@ macro_rules! module_device_table {
($table_type: literal, $module_table_name:ident, $table_name:ident) => {
#[rustfmt::skip]
#[export_name =
- concat!("__mod_device_table__", $table_type,
- "__", module_path!(),
- "_", line!(),
- "_", stringify!($table_name))
+ concat!("__mod_device_table__", line!(),
+ "__kmod_", module_path!(),
+ "__", $table_type,
+ "__", stringify!($table_name))
]
static $module_table_name: [::core::mem::MaybeUninit<u8>; $table_name.raw_ids().size()] =
unsafe { ::core::mem::transmute_copy($table_name.raw_ids()) };
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 00586119a25b..13021266a18f 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -1476,8 +1476,8 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
{
void *symval;
char *zeros = NULL;
- const char *type, *name;
- size_t typelen;
+ const char *type, *name, *modname;
+ size_t typelen, modnamelen;
static const char *prefix = "__mod_device_table__";
/* We're looking for a section relative symbol */
@@ -1488,10 +1488,20 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
if (ELF_ST_TYPE(sym->st_info) != STT_OBJECT)
return;
- /* All our symbols are of form __mod_device_table__<type>__<name>. */
+ /* All our symbols are of form __mod_device_table__kmod_<modname>__<type>__<name>. */
if (!strstarts(symname, prefix))
return;
- type = symname + strlen(prefix);
+
+ modname = strstr(symname, "__kmod_");
+ if (!modname)
+ return;
+ modname += strlen("__kmod_");
+
+ type = strstr(modname, "__");
+ if (!type)
+ return;
+ modnamelen = type - modname;
+ type += strlen("__");
name = strstr(type, "__");
if (!name)
--
2.50.1
^ permalink raw reply related
* [PATCH v6 7/9] modpost: Create modalias for builtin modules
From: Alexey Gladkov @ 2025-08-14 13:07 UTC (permalink / raw)
To: Nathan Chancellor, Nicolas Schier, Masahiro Yamada, Petr Pavlu,
Luis Chamberlain, Sami Tolvanen, Daniel Gomez
Cc: linux-kernel, linux-modules, linux-kbuild, Alexey Gladkov,
Stephen Rothwell
In-Reply-To: <cover.1755170493.git.legion@kernel.org>
For some modules, modalias is generated using the modpost utility and
the section is added to the module file.
When a module is added inside vmlinux, modpost does not generate
modalias for such modules and the information is lost.
As a result kmod (which uses modules.builtin.modinfo in userspace)
cannot determine that modalias is handled by a builtin kernel module.
$ cat /sys/devices/pci0000:00/0000:00:14.0/modalias
pci:v00008086d0000A36Dsv00001043sd00008694bc0Csc03i30
$ modinfo xhci_pci
name: xhci_pci
filename: (builtin)
license: GPL
file: drivers/usb/host/xhci-pci
description: xHCI PCI Host Controller Driver
Missing modalias "pci:v*d*sv*sd*bc0Csc03i30*" which will be generated by
modpost if the module is built separately.
To fix this it is necessary to generate the same modalias for vmlinux as
for the individual modules. Fortunately '.vmlinux.export.o' is already
generated from which '.modinfo' can be extracted in the same way as for
vmlinux.o.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Alexey Gladkov <legion@kernel.org>
Tested-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
include/linux/module.h | 4 ----
scripts/Makefile.vmlinux | 5 ++++-
scripts/mksysmap | 3 +++
scripts/mod/file2alias.c | 16 ++++++++++++++++
scripts/mod/modpost.c | 15 +++++++++++++++
scripts/mod/modpost.h | 2 ++
6 files changed, 40 insertions(+), 5 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index e31ee29fac6b..e135cc79acee 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -256,14 +256,10 @@ struct module_kobject *lookup_or_create_module_kobject(const char *name);
__PASTE(type, \
__PASTE(__, name)))))
-#ifdef MODULE
/* Creates an alias so file2alias.c can find device table. */
#define MODULE_DEVICE_TABLE(type, name) \
static typeof(name) __mod_device_table(type, name) \
__attribute__ ((used, alias(__stringify(name))))
-#else /* !MODULE */
-#define MODULE_DEVICE_TABLE(type, name)
-#endif
/* Version of form [<epoch>:]<version>[-<extra-version>].
* Or for CVS/RCS ID version, everything but the number is stripped.
diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux
index fdab5aa90215..fcc188d26ead 100644
--- a/scripts/Makefile.vmlinux
+++ b/scripts/Makefile.vmlinux
@@ -89,8 +89,11 @@ endif
remove-section-y := .modinfo
remove-section-$(CONFIG_ARCH_VMLINUX_NEEDS_RELOCS) += '.rel*'
+remove-symbols := -w --strip-symbol='__mod_device_table__*'
+
quiet_cmd_strip_relocs = OBJCOPY $@
- cmd_strip_relocs = $(OBJCOPY) $(addprefix --remove-section=,$(remove-section-y)) $< $@
+ cmd_strip_relocs = $(OBJCOPY) $(addprefix --remove-section=,$(remove-section-y)) \
+ $(remove-symbols) $< $@
targets += vmlinux
vmlinux: vmlinux.unstripped FORCE
diff --git a/scripts/mksysmap b/scripts/mksysmap
index a607a0059d11..c4531eacde20 100755
--- a/scripts/mksysmap
+++ b/scripts/mksysmap
@@ -59,6 +59,9 @@
# EXPORT_SYMBOL (namespace)
/ __kstrtabns_/d
+# MODULE_DEVICE_TABLE (symbol name)
+/ __mod_device_table__/d
+
# ---------------------------------------------------------------------------
# Ignored suffixes
# (do not forget '$' after each pattern)
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 13021266a18f..7da9735e7ab3 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -1527,5 +1527,21 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
}
}
+ if (mod->is_vmlinux) {
+ struct module_alias *alias;
+
+ /*
+ * If this is vmlinux, record the name of the builtin module.
+ * Traverse the linked list in the reverse order, and set the
+ * builtin_modname unless it has already been set in the
+ * previous call.
+ */
+ list_for_each_entry_reverse(alias, &mod->aliases, node) {
+ if (alias->builtin_modname)
+ break;
+ alias->builtin_modname = xstrndup(modname, modnamelen);
+ }
+ }
+
free(zeros);
}
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 5ca7c268294e..47c8aa2a6939 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -2067,11 +2067,26 @@ static void write_if_changed(struct buffer *b, const char *fname)
static void write_vmlinux_export_c_file(struct module *mod)
{
struct buffer buf = { };
+ struct module_alias *alias, *next;
buf_printf(&buf,
"#include <linux/export-internal.h>\n");
add_exported_symbols(&buf, mod);
+
+ buf_printf(&buf,
+ "#include <linux/module.h>\n"
+ "#undef __MODULE_INFO_PREFIX\n"
+ "#define __MODULE_INFO_PREFIX\n");
+
+ list_for_each_entry_safe(alias, next, &mod->aliases, node) {
+ buf_printf(&buf, "MODULE_INFO(%s.alias, \"%s\");\n",
+ alias->builtin_modname, alias->str);
+ list_del(&alias->node);
+ free(alias->builtin_modname);
+ free(alias);
+ }
+
write_if_changed(&buf, ".vmlinux.export.c");
free(buf.p);
}
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 9133e4c3803f..2aecb8f25c87 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -99,10 +99,12 @@ buf_write(struct buffer *buf, const char *s, int len);
* struct module_alias - auto-generated MODULE_ALIAS()
*
* @node: linked to module::aliases
+ * @modname: name of the builtin module (only for vmlinux)
* @str: a string for MODULE_ALIAS()
*/
struct module_alias {
struct list_head node;
+ char *builtin_modname;
char str[];
};
--
2.50.1
^ permalink raw reply related
* [PATCH v6 8/9] kbuild: vmlinux.unstripped should always depend on .vmlinux.export.o
From: Alexey Gladkov @ 2025-08-14 13:07 UTC (permalink / raw)
To: Nathan Chancellor, Nicolas Schier, Masahiro Yamada, Petr Pavlu,
Luis Chamberlain, Sami Tolvanen, Daniel Gomez
Cc: linux-kernel, linux-modules, linux-kbuild, Alexey Gladkov
In-Reply-To: <cover.1755170493.git.legion@kernel.org>
Since .vmlinux.export.c is used to add generated by modpost modaliases
for builtin modules the .vmlinux.export.o is no longer optional and
should always be created. The generation of this file is not dependent
on CONFIG_MODULES.
Signed-off-by: Alexey Gladkov <legion@kernel.org>
---
scripts/Makefile.vmlinux | 9 ++-------
scripts/link-vmlinux.sh | 5 +----
2 files changed, 3 insertions(+), 11 deletions(-)
diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux
index fcc188d26ead..dbbe3bf0cf23 100644
--- a/scripts/Makefile.vmlinux
+++ b/scripts/Makefile.vmlinux
@@ -53,11 +53,6 @@ endif
# vmlinux.unstripped
# ---------------------------------------------------------------------------
-ifdef CONFIG_MODULES
-targets += .vmlinux.export.o
-vmlinux.unstripped: .vmlinux.export.o
-endif
-
ifdef CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX
vmlinux.unstripped: arch/$(SRCARCH)/tools/vmlinux.arch.o
@@ -72,8 +67,8 @@ cmd_link_vmlinux = \
$< "$(LD)" "$(KBUILD_LDFLAGS)" "$(LDFLAGS_vmlinux)" "$@"; \
$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
-targets += vmlinux.unstripped
-vmlinux.unstripped: scripts/link-vmlinux.sh vmlinux.o $(KBUILD_LDS) FORCE
+targets += vmlinux.unstripped .vmlinux.export.o
+vmlinux.unstripped: scripts/link-vmlinux.sh vmlinux.o .vmlinux.export.o $(KBUILD_LDS) FORCE
+$(call if_changed_dep,link_vmlinux)
ifdef CONFIG_DEBUG_INFO_BTF
vmlinux.unstripped: $(RESOLVE_BTFIDS)
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 51367c2bfc21..433849ff7529 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -73,10 +73,7 @@ vmlinux_link()
objs="${objs} .builtin-dtbs.o"
fi
- if is_enabled CONFIG_MODULES; then
- objs="${objs} .vmlinux.export.o"
- fi
-
+ objs="${objs} .vmlinux.export.o"
objs="${objs} init/version-timestamp.o"
if [ "${SRCARCH}" = "um" ]; then
--
2.50.1
^ permalink raw reply related
* [PATCH v6 9/9] s390: vmlinux.lds.S: Reorder sections
From: Alexey Gladkov @ 2025-08-14 13:07 UTC (permalink / raw)
To: Nathan Chancellor, Nicolas Schier, Masahiro Yamada, Petr Pavlu,
Luis Chamberlain, Sami Tolvanen, Daniel Gomez
Cc: linux-kernel, linux-modules, linux-kbuild, Alexey Gladkov,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev, linux-s390,
kernel test robot
In-Reply-To: <cover.1755170493.git.legion@kernel.org>
Reorder the sections to be placed in the default segment. The
.vmlinux.info use :NONE to override the default segment and tell the
linker to not put the section in any segment at all.
>> s390x-linux-ld: .tmp_vmlinux1: warning: allocated section `.modinfo' not in segment
>> s390x-linux-ld: .tmp_vmlinux2: warning: allocated section `.modinfo' not in segment
>> s390x-linux-ld: vmlinux.unstripped: warning: allocated section `.modinfo' not in segment
Cc: Heiko Carstens <hca@linux.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Alexander Gordeev <agordeev@linux.ibm.com>
Cc: linux-s390@vger.kernel.org
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202506062053.zbkFBEnJ-lkp@intel.com/
Signed-off-by: Alexey Gladkov <legion@kernel.org>
Acked-by: Heiko Carstens <hca@linux.ibm.com>
---
arch/s390/kernel/vmlinux.lds.S | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/s390/kernel/vmlinux.lds.S b/arch/s390/kernel/vmlinux.lds.S
index 1c606dfa595d..feecf1a6ddb4 100644
--- a/arch/s390/kernel/vmlinux.lds.S
+++ b/arch/s390/kernel/vmlinux.lds.S
@@ -209,6 +209,11 @@ SECTIONS
. = ALIGN(PAGE_SIZE);
_end = . ;
+ /* Debugging sections. */
+ STABS_DEBUG
+ DWARF_DEBUG
+ ELF_DETAILS
+
/*
* uncompressed image info used by the decompressor
* it should match struct vmlinux_info
@@ -239,11 +244,6 @@ SECTIONS
#endif
} :NONE
- /* Debugging sections. */
- STABS_DEBUG
- DWARF_DEBUG
- ELF_DETAILS
-
/*
* Make sure that the .got.plt is either completely empty or it
* contains only the three reserved double words.
--
2.50.1
^ permalink raw reply related
* Re: [PATCH v6 6/9] modpost: Add modname to mod_device_table alias
From: Danilo Krummrich @ 2025-08-14 13:26 UTC (permalink / raw)
To: Alexey Gladkov
Cc: Nathan Chancellor, Nicolas Schier, Masahiro Yamada, Petr Pavlu,
Luis Chamberlain, Sami Tolvanen, Daniel Gomez, linux-kernel,
linux-modules, linux-kbuild, Miguel Ojeda, Andreas Hindborg,
Alex Gaynor, rust-for-linux
In-Reply-To: <15724fb8669dae64e3c8d31ab620f977984b2177.1755170493.git.legion@kernel.org>
On Thu Aug 14, 2025 at 3:07 PM CEST, Alexey Gladkov wrote:
> At this point, if a symbol is compiled as part of the kernel,
> information about which module the symbol belongs to is lost.
>
> To save this it is possible to add the module name to the alias name.
> It's not very pretty, but it's possible for now.
>
> Cc: Miguel Ojeda <ojeda@kernel.org>
> Cc: Andreas Hindborg <a.hindborg@kernel.org>
> Cc: Danilo Krummrich <dakr@kernel.org>
> Cc: Alex Gaynor <alex.gaynor@gmail.com>
> Cc: rust-for-linux@vger.kernel.org
> Signed-off-by: Alexey Gladkov <legion@kernel.org>
> ---
> include/linux/module.h | 14 +++++++++++++-
> rust/kernel/device_id.rs | 8 ++++----
> scripts/mod/file2alias.c | 18 ++++++++++++++----
> 3 files changed, 31 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 3319a5269d28..e31ee29fac6b 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -244,10 +244,22 @@ struct module_kobject *lookup_or_create_module_kobject(const char *name);
> /* What your module does. */
> #define MODULE_DESCRIPTION(_description) MODULE_INFO(description, _description)
>
> +/*
> + * Format: __mod_device_table__kmod_<modname>__<type>__<name>
> + * Parts of the string `__kmod_` and `__` are used as delimiters when parsing
> + * a symbol in file2alias.c
> + */
> +#define __mod_device_table(type, name) \
> + __PASTE(__mod_device_table__, \
> + __PASTE(__KBUILD_MODNAME, \
> + __PASTE(__, \
> + __PASTE(type, \
> + __PASTE(__, name)))))
> +
> #ifdef MODULE
> /* Creates an alias so file2alias.c can find device table. */
> #define MODULE_DEVICE_TABLE(type, name) \
> -static typeof(name) __mod_device_table__##type##__##name \
> +static typeof(name) __mod_device_table(type, name) \
> __attribute__ ((used, alias(__stringify(name))))
> #else /* !MODULE */
> #define MODULE_DEVICE_TABLE(type, name)
> diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
> index 70d57814ff79..62c42da12e9d 100644
> --- a/rust/kernel/device_id.rs
> +++ b/rust/kernel/device_id.rs
> @@ -195,10 +195,10 @@ macro_rules! module_device_table {
> ($table_type: literal, $module_table_name:ident, $table_name:ident) => {
> #[rustfmt::skip]
> #[export_name =
> - concat!("__mod_device_table__", $table_type,
> - "__", module_path!(),
> - "_", line!(),
> - "_", stringify!($table_name))
> + concat!("__mod_device_table__", line!(),
Why do we have line!() between "__mod_device_table__" and "__kmod_", while the
format is defined as "__mod_device_table__kmod_<modname>__<type>__<name>" above?
The previous logic was to create a unique name with
using "<module_path>_<line>_<table_name>" as "<name>". So, I think this should
actually be:
concat!("__mod_device_table__kmod_",
module_path!(),
"__", $table_type,
"__", stringify!($table_name),
"_", line!())
rather than the below.
> + "__kmod_", module_path!(),
> + "__", $table_type,
> + "__", stringify!($table_name))
> ]
> static $module_table_name: [::core::mem::MaybeUninit<u8>; $table_name.raw_ids().size()] =
> unsafe { ::core::mem::transmute_copy($table_name.raw_ids()) };
^ permalink raw reply
* Re: [PATCH v6 6/9] modpost: Add modname to mod_device_table alias
From: Alexey Gladkov @ 2025-08-14 13:54 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Nathan Chancellor, Nicolas Schier, Masahiro Yamada, Petr Pavlu,
Luis Chamberlain, Sami Tolvanen, Daniel Gomez, linux-kernel,
linux-modules, linux-kbuild, Miguel Ojeda, Andreas Hindborg,
Alex Gaynor, rust-for-linux
In-Reply-To: <DC26OG2L7OMH.31RE7460D4DHU@kernel.org>
On Thu, Aug 14, 2025 at 03:26:53PM +0200, Danilo Krummrich wrote:
> On Thu Aug 14, 2025 at 3:07 PM CEST, Alexey Gladkov wrote:
> > At this point, if a symbol is compiled as part of the kernel,
> > information about which module the symbol belongs to is lost.
> >
> > To save this it is possible to add the module name to the alias name.
> > It's not very pretty, but it's possible for now.
> >
> > Cc: Miguel Ojeda <ojeda@kernel.org>
> > Cc: Andreas Hindborg <a.hindborg@kernel.org>
> > Cc: Danilo Krummrich <dakr@kernel.org>
> > Cc: Alex Gaynor <alex.gaynor@gmail.com>
> > Cc: rust-for-linux@vger.kernel.org
> > Signed-off-by: Alexey Gladkov <legion@kernel.org>
> > ---
> > include/linux/module.h | 14 +++++++++++++-
> > rust/kernel/device_id.rs | 8 ++++----
> > scripts/mod/file2alias.c | 18 ++++++++++++++----
> > 3 files changed, 31 insertions(+), 9 deletions(-)
> >
> > diff --git a/include/linux/module.h b/include/linux/module.h
> > index 3319a5269d28..e31ee29fac6b 100644
> > --- a/include/linux/module.h
> > +++ b/include/linux/module.h
> > @@ -244,10 +244,22 @@ struct module_kobject *lookup_or_create_module_kobject(const char *name);
> > /* What your module does. */
> > #define MODULE_DESCRIPTION(_description) MODULE_INFO(description, _description)
> >
> > +/*
> > + * Format: __mod_device_table__kmod_<modname>__<type>__<name>
> > + * Parts of the string `__kmod_` and `__` are used as delimiters when parsing
> > + * a symbol in file2alias.c
> > + */
> > +#define __mod_device_table(type, name) \
> > + __PASTE(__mod_device_table__, \
> > + __PASTE(__KBUILD_MODNAME, \
> > + __PASTE(__, \
> > + __PASTE(type, \
> > + __PASTE(__, name)))))
> > +
> > #ifdef MODULE
> > /* Creates an alias so file2alias.c can find device table. */
> > #define MODULE_DEVICE_TABLE(type, name) \
> > -static typeof(name) __mod_device_table__##type##__##name \
> > +static typeof(name) __mod_device_table(type, name) \
> > __attribute__ ((used, alias(__stringify(name))))
> > #else /* !MODULE */
> > #define MODULE_DEVICE_TABLE(type, name)
> > diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
> > index 70d57814ff79..62c42da12e9d 100644
> > --- a/rust/kernel/device_id.rs
> > +++ b/rust/kernel/device_id.rs
> > @@ -195,10 +195,10 @@ macro_rules! module_device_table {
> > ($table_type: literal, $module_table_name:ident, $table_name:ident) => {
> > #[rustfmt::skip]
> > #[export_name =
> > - concat!("__mod_device_table__", $table_type,
> > - "__", module_path!(),
> > - "_", line!(),
> > - "_", stringify!($table_name))
> > + concat!("__mod_device_table__", line!(),
>
> Why do we have line!() between "__mod_device_table__" and "__kmod_", while the
> format is defined as "__mod_device_table__kmod_<modname>__<type>__<name>" above?
The "__mod_device_table__" is used to filter symbols.
The meaning part starts after "__kmod_" part. After that, order becomes
important.
> The previous logic was to create a unique name with
> using "<module_path>_<line>_<table_name>" as "<name>". So, I think this should
> actually be:
>
> concat!("__mod_device_table__kmod_",
> module_path!(),
> "__", $table_type,
> "__", stringify!($table_name),
> "_", line!())
>
> rather than the below.
No. "stringify!($table_name)" should be the last thing in this string.
This is the a symbol name that will be searched for in the elf to generate
modalias.
>
> > + "__kmod_", module_path!(),
> > + "__", $table_type,
> > + "__", stringify!($table_name))
> > ]
> > static $module_table_name: [::core::mem::MaybeUninit<u8>; $table_name.raw_ids().size()] =
> > unsafe { ::core::mem::transmute_copy($table_name.raw_ids()) };
>
--
Rgrds, legion
^ permalink raw reply
* Re: [PATCH v6 6/9] modpost: Add modname to mod_device_table alias
From: Danilo Krummrich @ 2025-08-14 14:03 UTC (permalink / raw)
To: Alexey Gladkov
Cc: Nathan Chancellor, Nicolas Schier, Masahiro Yamada, Petr Pavlu,
Luis Chamberlain, Sami Tolvanen, Daniel Gomez, linux-kernel,
linux-modules, linux-kbuild, Miguel Ojeda, Andreas Hindborg,
Alex Gaynor, rust-for-linux
In-Reply-To: <aJ3qsonmvUUErQx9@example.org>
On Thu Aug 14, 2025 at 3:54 PM CEST, Alexey Gladkov wrote:
> On Thu, Aug 14, 2025 at 03:26:53PM +0200, Danilo Krummrich wrote:
>> On Thu Aug 14, 2025 at 3:07 PM CEST, Alexey Gladkov wrote:
>> > At this point, if a symbol is compiled as part of the kernel,
>> > information about which module the symbol belongs to is lost.
>> >
>> > To save this it is possible to add the module name to the alias name.
>> > It's not very pretty, but it's possible for now.
>> >
>> > Cc: Miguel Ojeda <ojeda@kernel.org>
>> > Cc: Andreas Hindborg <a.hindborg@kernel.org>
>> > Cc: Danilo Krummrich <dakr@kernel.org>
>> > Cc: Alex Gaynor <alex.gaynor@gmail.com>
>> > Cc: rust-for-linux@vger.kernel.org
>> > Signed-off-by: Alexey Gladkov <legion@kernel.org>
>> > ---
>> > include/linux/module.h | 14 +++++++++++++-
>> > rust/kernel/device_id.rs | 8 ++++----
>> > scripts/mod/file2alias.c | 18 ++++++++++++++----
>> > 3 files changed, 31 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/include/linux/module.h b/include/linux/module.h
>> > index 3319a5269d28..e31ee29fac6b 100644
>> > --- a/include/linux/module.h
>> > +++ b/include/linux/module.h
>> > @@ -244,10 +244,22 @@ struct module_kobject *lookup_or_create_module_kobject(const char *name);
>> > /* What your module does. */
>> > #define MODULE_DESCRIPTION(_description) MODULE_INFO(description, _description)
>> >
>> > +/*
>> > + * Format: __mod_device_table__kmod_<modname>__<type>__<name>
>> > + * Parts of the string `__kmod_` and `__` are used as delimiters when parsing
>> > + * a symbol in file2alias.c
>> > + */
>> > +#define __mod_device_table(type, name) \
>> > + __PASTE(__mod_device_table__, \
>> > + __PASTE(__KBUILD_MODNAME, \
>> > + __PASTE(__, \
>> > + __PASTE(type, \
>> > + __PASTE(__, name)))))
>> > +
>> > #ifdef MODULE
>> > /* Creates an alias so file2alias.c can find device table. */
>> > #define MODULE_DEVICE_TABLE(type, name) \
>> > -static typeof(name) __mod_device_table__##type##__##name \
>> > +static typeof(name) __mod_device_table(type, name) \
>> > __attribute__ ((used, alias(__stringify(name))))
>> > #else /* !MODULE */
>> > #define MODULE_DEVICE_TABLE(type, name)
>> > diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
>> > index 70d57814ff79..62c42da12e9d 100644
>> > --- a/rust/kernel/device_id.rs
>> > +++ b/rust/kernel/device_id.rs
>> > @@ -195,10 +195,10 @@ macro_rules! module_device_table {
>> > ($table_type: literal, $module_table_name:ident, $table_name:ident) => {
>> > #[rustfmt::skip]
>> > #[export_name =
>> > - concat!("__mod_device_table__", $table_type,
>> > - "__", module_path!(),
>> > - "_", line!(),
>> > - "_", stringify!($table_name))
>> > + concat!("__mod_device_table__", line!(),
>>
>> Why do we have line!() between "__mod_device_table__" and "__kmod_", while the
>> format is defined as "__mod_device_table__kmod_<modname>__<type>__<name>" above?
>
> The "__mod_device_table__" is used to filter symbols.
> The meaning part starts after "__kmod_" part. After that, order becomes
> important.
>
>> The previous logic was to create a unique name with
>> using "<module_path>_<line>_<table_name>" as "<name>". So, I think this should
>> actually be:
>>
>> concat!("__mod_device_table__kmod_",
>> module_path!(),
>> "__", $table_type,
>> "__", stringify!($table_name),
>> "_", line!())
>>
>> rather than the below.
>
> No. "stringify!($table_name)" should be the last thing in this string.
> This is the a symbol name that will be searched for in the elf to generate
> modalias.
$table_name is not guaranteed to be unique for a certain module_path!(), hence
we need line!() to guarantee uniqueness.
The symbol name will be unique no matter where you place line!() of course, but
$table_name + line!() is the unique table name, which I think is what we want?
>>
>> > + "__kmod_", module_path!(),
>> > + "__", $table_type,
>> > + "__", stringify!($table_name))
>> > ]
>> > static $module_table_name: [::core::mem::MaybeUninit<u8>; $table_name.raw_ids().size()] =
>> > unsafe { ::core::mem::transmute_copy($table_name.raw_ids()) };
>>
>
> --
> Rgrds, legion
^ permalink raw reply
* Re: [PATCH v6 9/9] s390: vmlinux.lds.S: Reorder sections
From: Heiko Carstens @ 2025-08-14 14:16 UTC (permalink / raw)
To: Alexey Gladkov
Cc: Nathan Chancellor, Nicolas Schier, Masahiro Yamada, Petr Pavlu,
Luis Chamberlain, Sami Tolvanen, Daniel Gomez, linux-kernel,
linux-modules, linux-kbuild, Vasily Gorbik, Alexander Gordeev,
linux-s390, kernel test robot
In-Reply-To: <919570dc048786c4d07affaec4b761811c6c21c5.1755170493.git.legion@kernel.org>
On Thu, Aug 14, 2025 at 03:07:17PM +0200, Alexey Gladkov wrote:
> Reorder the sections to be placed in the default segment. The
> .vmlinux.info use :NONE to override the default segment and tell the
> linker to not put the section in any segment at all.
>
> >> s390x-linux-ld: .tmp_vmlinux1: warning: allocated section `.modinfo' not in segment
> >> s390x-linux-ld: .tmp_vmlinux2: warning: allocated section `.modinfo' not in segment
> >> s390x-linux-ld: vmlinux.unstripped: warning: allocated section `.modinfo' not in segment
>
> Cc: Heiko Carstens <hca@linux.ibm.com>
> Cc: Vasily Gorbik <gor@linux.ibm.com>
> Cc: Alexander Gordeev <agordeev@linux.ibm.com>
> Cc: linux-s390@vger.kernel.org
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202506062053.zbkFBEnJ-lkp@intel.com/
> Signed-off-by: Alexey Gladkov <legion@kernel.org>
> Acked-by: Heiko Carstens <hca@linux.ibm.com>
> ---
> arch/s390/kernel/vmlinux.lds.S | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
Is there any reason why you didn't reorder the patches?
https://lore.kernel.org/all/aIeUq0qYXoNIePwd@example.org/
^ permalink raw reply
* Re: [PATCH v6 6/9] modpost: Add modname to mod_device_table alias
From: Alexey Gladkov @ 2025-08-14 21:46 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Nathan Chancellor, Nicolas Schier, Masahiro Yamada, Petr Pavlu,
Luis Chamberlain, Sami Tolvanen, Daniel Gomez, linux-kernel,
linux-modules, linux-kbuild, Miguel Ojeda, Andreas Hindborg,
Alex Gaynor, rust-for-linux
In-Reply-To: <DC27G409IQGT.H9G83QDQ9V7R@kernel.org>
On Thu, Aug 14, 2025 at 04:03:01PM +0200, Danilo Krummrich wrote:
> On Thu Aug 14, 2025 at 3:54 PM CEST, Alexey Gladkov wrote:
> > On Thu, Aug 14, 2025 at 03:26:53PM +0200, Danilo Krummrich wrote:
> >> On Thu Aug 14, 2025 at 3:07 PM CEST, Alexey Gladkov wrote:
> >> > At this point, if a symbol is compiled as part of the kernel,
> >> > information about which module the symbol belongs to is lost.
> >> >
> >> > To save this it is possible to add the module name to the alias name.
> >> > It's not very pretty, but it's possible for now.
> >> >
> >> > Cc: Miguel Ojeda <ojeda@kernel.org>
> >> > Cc: Andreas Hindborg <a.hindborg@kernel.org>
> >> > Cc: Danilo Krummrich <dakr@kernel.org>
> >> > Cc: Alex Gaynor <alex.gaynor@gmail.com>
> >> > Cc: rust-for-linux@vger.kernel.org
> >> > Signed-off-by: Alexey Gladkov <legion@kernel.org>
> >> > ---
> >> > include/linux/module.h | 14 +++++++++++++-
> >> > rust/kernel/device_id.rs | 8 ++++----
> >> > scripts/mod/file2alias.c | 18 ++++++++++++++----
> >> > 3 files changed, 31 insertions(+), 9 deletions(-)
> >> >
> >> > diff --git a/include/linux/module.h b/include/linux/module.h
> >> > index 3319a5269d28..e31ee29fac6b 100644
> >> > --- a/include/linux/module.h
> >> > +++ b/include/linux/module.h
> >> > @@ -244,10 +244,22 @@ struct module_kobject *lookup_or_create_module_kobject(const char *name);
> >> > /* What your module does. */
> >> > #define MODULE_DESCRIPTION(_description) MODULE_INFO(description, _description)
> >> >
> >> > +/*
> >> > + * Format: __mod_device_table__kmod_<modname>__<type>__<name>
> >> > + * Parts of the string `__kmod_` and `__` are used as delimiters when parsing
> >> > + * a symbol in file2alias.c
> >> > + */
> >> > +#define __mod_device_table(type, name) \
> >> > + __PASTE(__mod_device_table__, \
> >> > + __PASTE(__KBUILD_MODNAME, \
> >> > + __PASTE(__, \
> >> > + __PASTE(type, \
> >> > + __PASTE(__, name)))))
> >> > +
> >> > #ifdef MODULE
> >> > /* Creates an alias so file2alias.c can find device table. */
> >> > #define MODULE_DEVICE_TABLE(type, name) \
> >> > -static typeof(name) __mod_device_table__##type##__##name \
> >> > +static typeof(name) __mod_device_table(type, name) \
> >> > __attribute__ ((used, alias(__stringify(name))))
> >> > #else /* !MODULE */
> >> > #define MODULE_DEVICE_TABLE(type, name)
> >> > diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
> >> > index 70d57814ff79..62c42da12e9d 100644
> >> > --- a/rust/kernel/device_id.rs
> >> > +++ b/rust/kernel/device_id.rs
> >> > @@ -195,10 +195,10 @@ macro_rules! module_device_table {
> >> > ($table_type: literal, $module_table_name:ident, $table_name:ident) => {
> >> > #[rustfmt::skip]
> >> > #[export_name =
> >> > - concat!("__mod_device_table__", $table_type,
> >> > - "__", module_path!(),
> >> > - "_", line!(),
> >> > - "_", stringify!($table_name))
> >> > + concat!("__mod_device_table__", line!(),
> >>
> >> Why do we have line!() between "__mod_device_table__" and "__kmod_", while the
> >> format is defined as "__mod_device_table__kmod_<modname>__<type>__<name>" above?
> >
> > The "__mod_device_table__" is used to filter symbols.
> > The meaning part starts after "__kmod_" part. After that, order becomes
> > important.
> >
> >> The previous logic was to create a unique name with
> >> using "<module_path>_<line>_<table_name>" as "<name>". So, I think this should
> >> actually be:
> >>
> >> concat!("__mod_device_table__kmod_",
> >> module_path!(),
> >> "__", $table_type,
> >> "__", stringify!($table_name),
> >> "_", line!())
> >>
> >> rather than the below.
> >
> > No. "stringify!($table_name)" should be the last thing in this string.
> > This is the a symbol name that will be searched for in the elf to generate
> > modalias.
>
> $table_name is not guaranteed to be unique for a certain module_path!(), hence
> we need line!() to guarantee uniqueness.
>
> The symbol name will be unique no matter where you place line!() of course, but
> $table_name + line!() is the unique table name, which I think is what we want?
Again, no. We need the entire symbol to be unique so that the linker
doesn't complain. In fact, this symbol will later be removed from the elf.
It is only needed for the modpost utility.
The modpost requires a format symbol:
__mod_device_table__<random>*__kmod_<modname>__<type>__<name>
"<random>*" may or may not exist. This is a place to add uniqueness if
needed.
The fields "<modname>”, "<type>" and "<name>" must be very specific and
must not be random. These values are used for generation.
>
> >>
> >> > + "__kmod_", module_path!(),
> >> > + "__", $table_type,
> >> > + "__", stringify!($table_name))
> >> > ]
> >> > static $module_table_name: [::core::mem::MaybeUninit<u8>; $table_name.raw_ids().size()] =
> >> > unsafe { ::core::mem::transmute_copy($table_name.raw_ids()) };
> >>
> >
> > --
> > Rgrds, legion
>
--
Rgrds, legion
^ permalink raw reply
* Re: [PATCH v6 9/9] s390: vmlinux.lds.S: Reorder sections
From: Alexey Gladkov @ 2025-08-14 21:51 UTC (permalink / raw)
To: Heiko Carstens
Cc: Nathan Chancellor, Nicolas Schier, Masahiro Yamada, Petr Pavlu,
Luis Chamberlain, Sami Tolvanen, Daniel Gomez, linux-kernel,
linux-modules, linux-kbuild, Vasily Gorbik, Alexander Gordeev,
linux-s390, kernel test robot
In-Reply-To: <20250814141658.7684Fd6-hca@linux.ibm.com>
On Thu, Aug 14, 2025 at 04:16:58PM +0200, Heiko Carstens wrote:
> On Thu, Aug 14, 2025 at 03:07:17PM +0200, Alexey Gladkov wrote:
> > Reorder the sections to be placed in the default segment. The
> > .vmlinux.info use :NONE to override the default segment and tell the
> > linker to not put the section in any segment at all.
> >
> > >> s390x-linux-ld: .tmp_vmlinux1: warning: allocated section `.modinfo' not in segment
> > >> s390x-linux-ld: .tmp_vmlinux2: warning: allocated section `.modinfo' not in segment
> > >> s390x-linux-ld: vmlinux.unstripped: warning: allocated section `.modinfo' not in segment
> >
> > Cc: Heiko Carstens <hca@linux.ibm.com>
> > Cc: Vasily Gorbik <gor@linux.ibm.com>
> > Cc: Alexander Gordeev <agordeev@linux.ibm.com>
> > Cc: linux-s390@vger.kernel.org
> > Reported-by: kernel test robot <lkp@intel.com>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202506062053.zbkFBEnJ-lkp@intel.com/
> > Signed-off-by: Alexey Gladkov <legion@kernel.org>
> > Acked-by: Heiko Carstens <hca@linux.ibm.com>
> > ---
> > arch/s390/kernel/vmlinux.lds.S | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
>
> Is there any reason why you didn't reorder the patches?
> https://lore.kernel.org/all/aIeUq0qYXoNIePwd@example.org/
Oops! My bad. This patchset was added to linux-next and seems to be in
the correct order, but then it was removed and I seem to have lost this
change.
--
Rgrds, legion
^ permalink raw reply
* Re: [PATCH v6 6/9] modpost: Add modname to mod_device_table alias
From: Danilo Krummrich @ 2025-08-14 22:17 UTC (permalink / raw)
To: Alexey Gladkov
Cc: Nathan Chancellor, Nicolas Schier, Masahiro Yamada, Petr Pavlu,
Luis Chamberlain, Sami Tolvanen, Daniel Gomez, linux-kernel,
linux-modules, linux-kbuild, Miguel Ojeda, Andreas Hindborg,
Alex Gaynor, rust-for-linux
In-Reply-To: <aJ5ZMJC4eCpDb5D8@example.org>
On Thu Aug 14, 2025 at 11:46 PM CEST, Alexey Gladkov wrote:
> Again, no. We need the entire symbol to be unique so that the linker
> doesn't complain. In fact, this symbol will later be removed from the elf.
> It is only needed for the modpost utility.
Gotcha -- I think I got confused; for the device ID parts:
Acked-by: Danilo Krummrich <dakr@kernel.org>
^ permalink raw reply
* Re: [PATCH v4] module: Rename EXPORT_SYMBOL_GPL_FOR_MODULES to EXPORT_SYMBOL_FOR_MODULES
From: Christian Brauner @ 2025-08-15 14:25 UTC (permalink / raw)
To: Daniel Gomez
Cc: Vlastimil Babka, Christoph Hellwig, Peter Zijlstra,
David Hildenbrand, Shivank Garg, Greg Kroah-Hartman,
Jiri Slaby (SUSE), Stephen Rothwell, linux-doc, linux-kernel,
linux-modules, linux-kbuild, linux-fsdevel, Nicolas Schier,
Daniel Gomez, Linus Torvalds, Matthias Maennich, Jonathan Corbet,
Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Nathan Chancellor,
Alexander Viro, Jan Kara
In-Reply-To: <2472a139-064c-4381-bc6e-a69245be01df@kernel.org>
On Tue, Aug 12, 2025 at 09:54:43AM +0200, Daniel Gomez wrote:
> On 11/08/2025 07.18, Christian Brauner wrote:
> > On Fri, 08 Aug 2025 15:28:47 +0200, Vlastimil Babka wrote:
> >> Christoph suggested that the explicit _GPL_ can be dropped from the
> >> module namespace export macro, as it's intended for in-tree modules
> >> only. It would be possible to restrict it technically, but it was
> >> pointed out [2] that some cases of using an out-of-tree build of an
> >> in-tree module with the same name are legitimate. But in that case those
> >> also have to be GPL anyway so it's unnecessary to spell it out in the
> >> macro name.
> >>
> >> [...]
> >
> > Ok, so last I remember we said that this is going upstream rather sooner
> > than later before we keep piling on users. If that's still the case I'll
> > take it via vfs.fixes unless I hear objections.
>
> This used to go through Masahiro's kbuild tree. However, since he is not
> available anymore [1] I think it makes sense that this goes through the modules
> tree. The only reason we waited until rc1 was released was because of Greg's
> advise [2]. Let me know if that makes sense to you and if so, I'll merge this
> ASAP.
At this point it would mean messing up all of vfs.fixes to drop it from
there. So I'd just leave it in there and send it to Linus. Next time
I know where it'll end up.
^ permalink raw reply
* Re: [PATCH v4] module: Rename EXPORT_SYMBOL_GPL_FOR_MODULES to EXPORT_SYMBOL_FOR_MODULES
From: Daniel Gomez @ 2025-08-15 15:39 UTC (permalink / raw)
To: Christian Brauner
Cc: Vlastimil Babka, Christoph Hellwig, Peter Zijlstra,
David Hildenbrand, Shivank Garg, Greg Kroah-Hartman,
Jiri Slaby (SUSE), Stephen Rothwell, linux-doc, linux-kernel,
linux-modules, linux-kbuild, linux-fsdevel, Nicolas Schier,
Daniel Gomez, Linus Torvalds, Matthias Maennich, Jonathan Corbet,
Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Nathan Chancellor,
Alexander Viro, Jan Kara
In-Reply-To: <20250815-darstellen-pappen-90a9edb193e5@brauner>
On 15/08/2025 07.25, Christian Brauner wrote:
> On Tue, Aug 12, 2025 at 09:54:43AM +0200, Daniel Gomez wrote:
>> On 11/08/2025 07.18, Christian Brauner wrote:j
>>> On Fri, 08 Aug 2025 15:28:47 +0200, Vlastimil Babka wrote:
>>>> Christoph suggested that the explicit _GPL_ can be dropped from the
>>>> module namespace export macro, as it's intended for in-tree modules
>>>> only. It would be possible to restrict it technically, but it was
>>>> pointed out [2] that some cases of using an out-of-tree build of an
>>>> in-tree module with the same name are legitimate. But in that case those
>>>> also have to be GPL anyway so it's unnecessary to spell it out in the
>>>> macro name.
>>>>
>>>> [...]
>>>
>>> Ok, so last I remember we said that this is going upstream rather sooner
>>> than later before we keep piling on users. If that's still the case I'll
>>> take it via vfs.fixes unless I hear objections.
>>
>> This used to go through Masahiro's kbuild tree. However, since he is not
>> available anymore [1] I think it makes sense that this goes through the modules
>> tree. The only reason we waited until rc1 was released was because of Greg's
>> advise [2]. Let me know if that makes sense to you and if so, I'll merge this
>> ASAP.
>
> At this point it would mean messing up all of vfs.fixes to drop it from
> there. So I'd just leave it in there and send it to Linus.
Got it. I was waiting for confirmation before taking it into the modules tree,
and I agree that at this point it makes sense to keep it in vfs.fixes.
> Next time I know where it'll end up.
Can you clarify what you mean by this?
^ permalink raw reply
* Re: [PATCH v2] params: Replace deprecated strcpy() with strscpy() and memcpy()
From: Daniel Gomez @ 2025-08-15 19:19 UTC (permalink / raw)
To: Petr Pavlu, Thorsten Blum
Cc: Thomas Weißschuh, Shyam Saini, Luis Chamberlain,
Dmitry Antipov, linux-kernel, linux-modules, Kees Cook
In-Reply-To: <f60327bb-6546-4d15-8bd2-a05e85d96b4f@suse.com>
On 13/08/2025 07.14, Petr Pavlu wrote:
> On 8/13/25 3:21 PM, Thorsten Blum wrote:
>> strcpy() is deprecated; use strscpy() and memcpy() instead.
>>
>> In param_set_copystring(), we can safely use memcpy() because we already
>> know the length of the source string 'val' and that it is guaranteed to
>> be NUL-terminated within the first 'kps->maxlen' bytes.
>>
>> Link: https://github.com/KSPP/linux/issues/88
>> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
>> ---
>> Changes in v2:
>> - Use memcpy() in param_set_copystring() as suggested by Petr Pavlu
>> - Link to v1: https://lore.kernel.org/lkml/20250810214456.2236-1-thorsten.blum@linux.dev/
>> ---
>> kernel/params.c | 7 ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/kernel/params.c b/kernel/params.c
>> index b92d64161b75..b96cfd693c99 100644
>> --- a/kernel/params.c
>> +++ b/kernel/params.c
>> @@ -513,13 +513,14 @@ EXPORT_SYMBOL(param_array_ops);
>> int param_set_copystring(const char *val, const struct kernel_param *kp)
>> {
>> const struct kparam_string *kps = kp->str;
>> + const size_t len = strnlen(val, kps->maxlen);
>>
>> - if (strnlen(val, kps->maxlen) == kps->maxlen) {
>> + if (len == kps->maxlen) {
>> pr_err("%s: string doesn't fit in %u chars.\n",
>> kp->name, kps->maxlen-1);
>> return -ENOSPC;
>> }
>> - strcpy(kps->string, val);
>> + memcpy(kps->string, val, len + 1);
>> return 0;
>> }
>> EXPORT_SYMBOL(param_set_copystring);
>> @@ -841,7 +842,7 @@ static void __init param_sysfs_builtin(void)
>> dot = strchr(kp->name, '.');
>> if (!dot) {
>> /* This happens for core_param() */
>> - strcpy(modname, "kernel");
>> + strscpy(modname, "kernel");
>> name_len = 0;
>> } else {
>> name_len = dot - kp->name + 1;
>
> Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
>
Reviewed-by: Daniel Gomez <da.gomez@samsung.com>
^ permalink raw reply
* modprobe returns 0 upon -EEXIST from insmod
From: Phil Sutter @ 2025-08-16 23:33 UTC (permalink / raw)
To: linux-modules; +Cc: Yi Chen
Hi,
I admittedly didn't fully analyze the cause, but on my system a call to:
# insmod /lib/module/$(uname -r)/kernel/net/netfilter/nf_conntrack_ftp.ko
fails with -EEXIST (due to a previous call to 'nfct add helper ftp inet
tcp'). A call to:
# modprobe nf_conntrack_ftp
though returns 0 even though module loading fails. Is there a bug in
modprobe error status handling?
Cheers, Phil
^ permalink raw reply
* Re: [PATCH v6 3/9] kbuild: extract modules.builtin.modinfo from vmlinux.unstripped
From: Masahiro Yamada @ 2025-08-17 12:34 UTC (permalink / raw)
To: Alexey Gladkov
Cc: Nathan Chancellor, Nicolas Schier, Petr Pavlu, Luis Chamberlain,
Sami Tolvanen, Daniel Gomez, linux-kernel, linux-modules,
linux-kbuild
In-Reply-To: <16859d94a2c8e4b1bb305defdb8b7be238499c66.1755170493.git.legion@kernel.org>
On Thu, Aug 14, 2025 at 10:08 PM Alexey Gladkov <legion@kernel.org> wrote:
>
> From: Masahiro Yamada <masahiroy@kernel.org>
>
> Currently, we assume all the data for modules.builtin.modinfo are
> available in vmlinux.o.
>
> This makes it impossible for modpost, which is invoked after vmlinux.o,
> to add additional module info.
>
> This commit moves the modules.builtin.modinfo rule after modpost.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> Signed-off-by: Alexey Gladkov <legion@kernel.org>
I got this report from Stephen
https://lore.kernel.org/lkml/20250730164047.7c4a731a@canb.auug.org.au/
Please make sure to have no regression.
If this is difficult to solve, please discard this patch,
and consider a different approach.
> ---
> scripts/Makefile.vmlinux | 26 ++++++++++++++++++++++++++
> scripts/Makefile.vmlinux_o | 26 +-------------------------
> 2 files changed, 27 insertions(+), 25 deletions(-)
>
> diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux
> index e2ceeb9e168d..fdab5aa90215 100644
> --- a/scripts/Makefile.vmlinux
> +++ b/scripts/Makefile.vmlinux
> @@ -96,6 +96,32 @@ targets += vmlinux
> vmlinux: vmlinux.unstripped FORCE
> $(call if_changed,strip_relocs)
>
> +# modules.builtin.modinfo
> +# ---------------------------------------------------------------------------
> +
> +OBJCOPYFLAGS_modules.builtin.modinfo := -j .modinfo -O binary
> +
> +targets += modules.builtin.modinfo
> +modules.builtin.modinfo: vmlinux.unstripped FORCE
> + $(call if_changed,objcopy)
> +
> +# modules.builtin
> +# ---------------------------------------------------------------------------
> +
> +__default: modules.builtin
> +
> +# The second line aids cases where multiple modules share the same object.
> +
> +quiet_cmd_modules_builtin = GEN $@
> + cmd_modules_builtin = \
> + tr '\0' '\n' < $< | \
> + sed -n 's/^[[:alnum:]:_]*\.file=//p' | \
> + tr ' ' '\n' | uniq | sed -e 's:^:kernel/:' -e 's/$$/.ko/' > $@
> +
> +targets += modules.builtin
> +modules.builtin: modules.builtin.modinfo FORCE
> + $(call if_changed,modules_builtin)
> +
> # modules.builtin.ranges
> # ---------------------------------------------------------------------------
> ifdef CONFIG_BUILTIN_MODULE_RANGES
> diff --git a/scripts/Makefile.vmlinux_o b/scripts/Makefile.vmlinux_o
> index b024ffb3e201..23c8751285d7 100644
> --- a/scripts/Makefile.vmlinux_o
> +++ b/scripts/Makefile.vmlinux_o
> @@ -1,7 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0-only
>
> PHONY := __default
> -__default: vmlinux.o modules.builtin.modinfo modules.builtin
> +__default: vmlinux.o
>
> include include/config/auto.conf
> include $(srctree)/scripts/Kbuild.include
> @@ -73,30 +73,6 @@ vmlinux.o: $(initcalls-lds) vmlinux.a $(KBUILD_VMLINUX_LIBS) FORCE
>
> targets += vmlinux.o
>
> -# modules.builtin.modinfo
> -# ---------------------------------------------------------------------------
> -
> -OBJCOPYFLAGS_modules.builtin.modinfo := -j .modinfo -O binary
> -
> -targets += modules.builtin.modinfo
> -modules.builtin.modinfo: vmlinux.o FORCE
> - $(call if_changed,objcopy)
> -
> -# modules.builtin
> -# ---------------------------------------------------------------------------
> -
> -# The second line aids cases where multiple modules share the same object.
> -
> -quiet_cmd_modules_builtin = GEN $@
> - cmd_modules_builtin = \
> - tr '\0' '\n' < $< | \
> - sed -n 's/^[[:alnum:]:_]*\.file=//p' | \
> - tr ' ' '\n' | uniq | sed -e 's:^:kernel/:' -e 's/$$/.ko/' > $@
> -
> -targets += modules.builtin
> -modules.builtin: modules.builtin.modinfo FORCE
> - $(call if_changed,modules_builtin)
> -
> # Add FORCE to the prerequisites of a target to force it to be always rebuilt.
> # ---------------------------------------------------------------------------
>
> --
> 2.50.1
>
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH v6 3/9] kbuild: extract modules.builtin.modinfo from vmlinux.unstripped
From: Alexey Gladkov @ 2025-08-17 12:52 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Nathan Chancellor, Nicolas Schier, Petr Pavlu, Luis Chamberlain,
Sami Tolvanen, Daniel Gomez, linux-kernel, linux-modules,
linux-kbuild
In-Reply-To: <CAK7LNAR-gD2H6Kk-rZjo0R3weTHCGTm0a=u2tRH1WWW6Sx6=RQ@mail.gmail.com>
On Sun, Aug 17, 2025 at 09:34:52PM +0900, Masahiro Yamada wrote:
> On Thu, Aug 14, 2025 at 10:08 PM Alexey Gladkov <legion@kernel.org> wrote:
> >
> > From: Masahiro Yamada <masahiroy@kernel.org>
> >
> > Currently, we assume all the data for modules.builtin.modinfo are
> > available in vmlinux.o.
> >
> > This makes it impossible for modpost, which is invoked after vmlinux.o,
> > to add additional module info.
> >
> > This commit moves the modules.builtin.modinfo rule after modpost.
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > Signed-off-by: Alexey Gladkov <legion@kernel.org>
>
> I got this report from Stephen
> https://lore.kernel.org/lkml/20250730164047.7c4a731a@canb.auug.org.au/
>
> Please make sure to have no regression.
> If this is difficult to solve, please discard this patch,
> and consider a different approach.
My emails must have gotten lost somewhere. Because I replied to that
message right away and provided a fix. Stephen even applied it to the tree
later. You were in CC whole time.
https://lore.kernel.org/all/20250730090025.2402129-1-legion@kernel.org/
Tomorrow I will make a new version with the corrections I missed. But now,
I'm not sure to which tree I should send it.
>
> > ---
> > scripts/Makefile.vmlinux | 26 ++++++++++++++++++++++++++
> > scripts/Makefile.vmlinux_o | 26 +-------------------------
> > 2 files changed, 27 insertions(+), 25 deletions(-)
> >
> > diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux
> > index e2ceeb9e168d..fdab5aa90215 100644
> > --- a/scripts/Makefile.vmlinux
> > +++ b/scripts/Makefile.vmlinux
> > @@ -96,6 +96,32 @@ targets += vmlinux
> > vmlinux: vmlinux.unstripped FORCE
> > $(call if_changed,strip_relocs)
> >
> > +# modules.builtin.modinfo
> > +# ---------------------------------------------------------------------------
> > +
> > +OBJCOPYFLAGS_modules.builtin.modinfo := -j .modinfo -O binary
> > +
> > +targets += modules.builtin.modinfo
> > +modules.builtin.modinfo: vmlinux.unstripped FORCE
> > + $(call if_changed,objcopy)
> > +
> > +# modules.builtin
> > +# ---------------------------------------------------------------------------
> > +
> > +__default: modules.builtin
> > +
> > +# The second line aids cases where multiple modules share the same object.
> > +
> > +quiet_cmd_modules_builtin = GEN $@
> > + cmd_modules_builtin = \
> > + tr '\0' '\n' < $< | \
> > + sed -n 's/^[[:alnum:]:_]*\.file=//p' | \
> > + tr ' ' '\n' | uniq | sed -e 's:^:kernel/:' -e 's/$$/.ko/' > $@
> > +
> > +targets += modules.builtin
> > +modules.builtin: modules.builtin.modinfo FORCE
> > + $(call if_changed,modules_builtin)
> > +
> > # modules.builtin.ranges
> > # ---------------------------------------------------------------------------
> > ifdef CONFIG_BUILTIN_MODULE_RANGES
> > diff --git a/scripts/Makefile.vmlinux_o b/scripts/Makefile.vmlinux_o
> > index b024ffb3e201..23c8751285d7 100644
> > --- a/scripts/Makefile.vmlinux_o
> > +++ b/scripts/Makefile.vmlinux_o
> > @@ -1,7 +1,7 @@
> > # SPDX-License-Identifier: GPL-2.0-only
> >
> > PHONY := __default
> > -__default: vmlinux.o modules.builtin.modinfo modules.builtin
> > +__default: vmlinux.o
> >
> > include include/config/auto.conf
> > include $(srctree)/scripts/Kbuild.include
> > @@ -73,30 +73,6 @@ vmlinux.o: $(initcalls-lds) vmlinux.a $(KBUILD_VMLINUX_LIBS) FORCE
> >
> > targets += vmlinux.o
> >
> > -# modules.builtin.modinfo
> > -# ---------------------------------------------------------------------------
> > -
> > -OBJCOPYFLAGS_modules.builtin.modinfo := -j .modinfo -O binary
> > -
> > -targets += modules.builtin.modinfo
> > -modules.builtin.modinfo: vmlinux.o FORCE
> > - $(call if_changed,objcopy)
> > -
> > -# modules.builtin
> > -# ---------------------------------------------------------------------------
> > -
> > -# The second line aids cases where multiple modules share the same object.
> > -
> > -quiet_cmd_modules_builtin = GEN $@
> > - cmd_modules_builtin = \
> > - tr '\0' '\n' < $< | \
> > - sed -n 's/^[[:alnum:]:_]*\.file=//p' | \
> > - tr ' ' '\n' | uniq | sed -e 's:^:kernel/:' -e 's/$$/.ko/' > $@
> > -
> > -targets += modules.builtin
> > -modules.builtin: modules.builtin.modinfo FORCE
> > - $(call if_changed,modules_builtin)
> > -
> > # Add FORCE to the prerequisites of a target to force it to be always rebuilt.
> > # ---------------------------------------------------------------------------
> >
> > --
> > 2.50.1
> >
>
>
> --
> Best Regards
> Masahiro Yamada
>
--
Rgrds, legion
^ permalink raw reply
* Re: modprobe returns 0 upon -EEXIST from insmod
From: Christophe Leroy @ 2025-08-17 15:54 UTC (permalink / raw)
To: Phil Sutter, linux-modules; +Cc: Yi Chen
In-Reply-To: <aKEVQhJpRdiZSliu@orbyte.nwl.cc>
Hi,
Le 17/08/2025 à 01:33, Phil Sutter a écrit :
> [Vous ne recevez pas souvent de courriers de phil@nwl.cc. D?couvrez pourquoi ceci est important ? https://aka.ms/LearnAboutSenderIdentification ]
>
> Hi,
>
> I admittedly didn't fully analyze the cause, but on my system a call to:
>
> # insmod /lib/module/$(uname -r)/kernel/net/netfilter/nf_conntrack_ftp.ko
>
> fails with -EEXIST (due to a previous call to 'nfct add helper ftp inet
> tcp'). A call to:
>
> # modprobe nf_conntrack_ftp
>
> though returns 0 even though module loading fails. Is there a bug in
> modprobe error status handling?
>
Read the man page : https://linux.die.net/man/8/modprobe
In the man page I see:
Normally, modprobe will succeed (and do nothing) if told to
insert a module which is already present or to remove a module which
isn't present.
Christophe
^ permalink raw reply
* Re: [PATCH v6 3/9] kbuild: extract modules.builtin.modinfo from vmlinux.unstripped
From: Nathan Chancellor @ 2025-08-18 6:43 UTC (permalink / raw)
To: Alexey Gladkov
Cc: Masahiro Yamada, Nicolas Schier, Petr Pavlu, Luis Chamberlain,
Sami Tolvanen, Daniel Gomez, linux-kernel, linux-modules,
linux-kbuild
In-Reply-To: <aKHQl_XC63-c-81L@example.org>
On Sun, Aug 17, 2025 at 02:52:39PM +0200, Alexey Gladkov wrote:
> My emails must have gotten lost somewhere. Because I replied to that
> message right away and provided a fix. Stephen even applied it to the tree
> later. You were in CC whole time.
>
> https://lore.kernel.org/all/20250730090025.2402129-1-legion@kernel.org/
>
> Tomorrow I will make a new version with the corrections I missed. But now,
> I'm not sure to which tree I should send it.
You can send it against kbuild-next:
https://git.kernel.org/kbuild/l/kbuild-next
Cheers,
Nathan
^ permalink raw reply
* Re: [PATCH v6 5/9] pinctrl: meson: Fix typo in device table macro
From: Linus Walleij @ 2025-08-18 9:11 UTC (permalink / raw)
To: Alexey Gladkov
Cc: Nathan Chancellor, Nicolas Schier, Masahiro Yamada, Petr Pavlu,
Luis Chamberlain, Sami Tolvanen, Daniel Gomez, linux-kernel,
linux-modules, linux-kbuild, Xianwei Zhao, Neil Armstrong,
Kevin Hilman, linux-amlogic, linux-gpio, kernel test robot
In-Reply-To: <e548b7761302defec15aa2098172eabb1ce1ad4a.1755170493.git.legion@kernel.org>
On Thu, Aug 14, 2025 at 3:08 PM Alexey Gladkov <legion@kernel.org> wrote:
> The typo when using the MODULE_DEVICE_TABLE macro was not noticeable
> because the macro was defined only if the module was built as a separate
> module.
>
> Cc: Xianwei Zhao <xianwei.zhao@amlogic.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Neil Armstrong <neil.armstrong@linaro.org>
> Cc: Kevin Hilman <khilman@baylibre.com>
> Cc: linux-amlogic@lists.infradead.org
> Cc: linux-gpio@vger.kernel.org
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202507220009.8HKbNP16-lkp@intel.com/
> Signed-off-by: Alexey Gladkov <legion@kernel.org>
> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
This patch 5/9 applied separately to pin control fixes.
Yours,
Linus Walleij
^ permalink raw reply
* Re: modprobe returns 0 upon -EEXIST from insmod
From: Phil Sutter @ 2025-08-18 9:34 UTC (permalink / raw)
To: Christophe Leroy; +Cc: linux-modules, Yi Chen
In-Reply-To: <8a87656d-577a-4d0a-85b1-5fd17d0346fe@csgroup.eu>
Hi Christophe,
On Sun, Aug 17, 2025 at 05:54:27PM +0200, Christophe Leroy wrote:
> Le 17/08/2025 à 01:33, Phil Sutter a écrit :
> > [Vous ne recevez pas souvent de courriers de phil@nwl.cc. D?couvrez pourquoi ceci est important ? https://aka.ms/LearnAboutSenderIdentification ]
> >
> > Hi,
> >
> > I admittedly didn't fully analyze the cause, but on my system a call to:
> >
> > # insmod /lib/module/$(uname -r)/kernel/net/netfilter/nf_conntrack_ftp.ko
> >
> > fails with -EEXIST (due to a previous call to 'nfct add helper ftp inet
> > tcp'). A call to:
> >
> > # modprobe nf_conntrack_ftp
> >
> > though returns 0 even though module loading fails. Is there a bug in
> > modprobe error status handling?
> >
>
> Read the man page : https://linux.die.net/man/8/modprobe
>
> In the man page I see:
>
> Normally, modprobe will succeed (and do nothing) if told to
> insert a module which is already present or to remove a module which
> isn't present.
This is not a case of already inserted module, it is not loaded before
the call to modprobe. It is the module_init callback
nf_conntrack_ftp_init() which returns -EEXIST it received from
nf_conntrack_helpers_register().
Can't user space distinguish the two causes of -EEXIST? Or in other
words, is use of -EEXIST in module_init callbacks problematic?
Cheers, Phil
^ permalink raw reply
* Re: modprobe returns 0 upon -EEXIST from insmod
From: Christophe Leroy @ 2025-08-18 10:07 UTC (permalink / raw)
To: Phil Sutter, netfilter-devel, coreteam; +Cc: linux-modules, Yi Chen, netdev
In-Reply-To: <aKLzsAX14ybEjHfJ@orbyte.nwl.cc>
[+ Netfilter lists]
Hi Phil
Le 18/08/2025 à 11:34, Phil Sutter a écrit :
> [Vous ne recevez pas souvent de courriers de phil@nwl.cc. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
>
> Hi Christophe,
>
> On Sun, Aug 17, 2025 at 05:54:27PM +0200, Christophe Leroy wrote:
>> Le 17/08/2025 à 01:33, Phil Sutter a écrit :
>>> [Vous ne recevez pas souvent de courriers de phil@nwl.cc. D?couvrez pourquoi ceci est important ? https://aka.ms/LearnAboutSenderIdentification ]
>>>
>>> Hi,
>>>
>>> I admittedly didn't fully analyze the cause, but on my system a call to:
>>>
>>> # insmod /lib/module/$(uname -r)/kernel/net/netfilter/nf_conntrack_ftp.ko
>>>
>>> fails with -EEXIST (due to a previous call to 'nfct add helper ftp inet
>>> tcp'). A call to:
>>>
>>> # modprobe nf_conntrack_ftp
>>>
>>> though returns 0 even though module loading fails. Is there a bug in
>>> modprobe error status handling?
>>>
>>
>> Read the man page : https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flinux.die.net%2Fman%2F8%2Fmodprobe&data=05%7C02%7Cchristophe.leroy%40csgroup.eu%7C34b49eb3d0544fc683e608ddde3a75b2%7C8b87af7d86474dc78df45f69a2011bb5%7C0%7C0%7C638911064858807750%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=%2F70LV37Zb%2FNeiBV59y9rvkLGh0xsqga08Nl3c5%2BVU5I%3D&reserved=0
>>
>> In the man page I see:
>>
>> Normally, modprobe will succeed (and do nothing) if told to
>> insert a module which is already present or to remove a module which
>> isn't present.
>
> This is not a case of already inserted module, it is not loaded before
> the call to modprobe. It is the module_init callback
> nf_conntrack_ftp_init() which returns -EEXIST it received from
> nf_conntrack_helpers_register().
>
> Can't user space distinguish the two causes of -EEXIST? Or in other
> words, is use of -EEXIST in module_init callbacks problematic?
So if I understand correctly the load fails because it is in conflict
with another module ?
Then I think the error returned by nf_conntrack_helpers_register()
shouldn't be EEXIST but probably EBUSY.
Christophe
^ permalink raw reply
* Re: modprobe returns 0 upon -EEXIST from insmod
From: Phil Sutter @ 2025-08-18 10:56 UTC (permalink / raw)
To: Christophe Leroy
Cc: netfilter-devel, coreteam, linux-modules, Yi Chen, netdev
In-Reply-To: <edfed2af-8b4d-4afb-b999-5c46b7d46fba@csgroup.eu>
On Mon, Aug 18, 2025 at 12:07:18PM +0200, Christophe Leroy wrote:
> [+ Netfilter lists]
>
> Hi Phil
>
> Le 18/08/2025 à 11:34, Phil Sutter a écrit :
> > [Vous ne recevez pas souvent de courriers de phil@nwl.cc. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
> >
> > Hi Christophe,
> >
> > On Sun, Aug 17, 2025 at 05:54:27PM +0200, Christophe Leroy wrote:
> >> Le 17/08/2025 à 01:33, Phil Sutter a écrit :
> >>> [Vous ne recevez pas souvent de courriers de phil@nwl.cc. D?couvrez pourquoi ceci est important ? https://aka.ms/LearnAboutSenderIdentification ]
> >>>
> >>> Hi,
> >>>
> >>> I admittedly didn't fully analyze the cause, but on my system a call to:
> >>>
> >>> # insmod /lib/module/$(uname -r)/kernel/net/netfilter/nf_conntrack_ftp.ko
> >>>
> >>> fails with -EEXIST (due to a previous call to 'nfct add helper ftp inet
> >>> tcp'). A call to:
> >>>
> >>> # modprobe nf_conntrack_ftp
> >>>
> >>> though returns 0 even though module loading fails. Is there a bug in
> >>> modprobe error status handling?
> >>>
> >>
> >> Read the man page : https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flinux.die.net%2Fman%2F8%2Fmodprobe&data=05%7C02%7Cchristophe.leroy%40csgroup.eu%7C34b49eb3d0544fc683e608ddde3a75b2%7C8b87af7d86474dc78df45f69a2011bb5%7C0%7C0%7C638911064858807750%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=%2F70LV37Zb%2FNeiBV59y9rvkLGh0xsqga08Nl3c5%2BVU5I%3D&reserved=0
> >>
> >> In the man page I see:
> >>
> >> Normally, modprobe will succeed (and do nothing) if told to
> >> insert a module which is already present or to remove a module which
> >> isn't present.
> >
> > This is not a case of already inserted module, it is not loaded before
> > the call to modprobe. It is the module_init callback
> > nf_conntrack_ftp_init() which returns -EEXIST it received from
> > nf_conntrack_helpers_register().
> >
> > Can't user space distinguish the two causes of -EEXIST? Or in other
> > words, is use of -EEXIST in module_init callbacks problematic?
>
> So if I understand correctly the load fails because it is in conflict
> with another module ?
Yes, it tries to signal that there is already a conntrack helper for
FTP. It is a stub redirecting to an implementation in user space, but
that's just details.
> Then I think the error returned by nf_conntrack_helpers_register()
> shouldn't be EEXIST but probably EBUSY.
Sounds good! We could at least adjust the module_init callback return
code from EEXIST to EBUSY so the change has minimal impact.
Thanks for your help, Christophe!
Cheers, Phil
^ permalink raw reply
* [nf PATCH] netfilter: conntrack: helper: Replace -EEXIST by -EBUSY
From: Phil Sutter @ 2025-08-18 11:22 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: netfilter-devel, Yi Chen, linux-modules, netdev, Christophe Leroy
The helper registration return value is passed-through by module_init
callbacks which modprobe confuses with the harmless -EEXIST returned
when trying to load an already loaded module.
Make sure modprobe fails so users notice their helper has not been
registered and won't work.
Suggested-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Fixes: 12f7a505331e ("netfilter: add user-space connection tracking helper infrastructure")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
net/netfilter/nf_conntrack_helper.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/nf_conntrack_helper.c b/net/netfilter/nf_conntrack_helper.c
index 4ed5878cb25b..ceb48c3ca0a4 100644
--- a/net/netfilter/nf_conntrack_helper.c
+++ b/net/netfilter/nf_conntrack_helper.c
@@ -368,7 +368,7 @@ int nf_conntrack_helper_register(struct nf_conntrack_helper *me)
(cur->tuple.src.l3num == NFPROTO_UNSPEC ||
cur->tuple.src.l3num == me->tuple.src.l3num) &&
cur->tuple.dst.protonum == me->tuple.dst.protonum) {
- ret = -EEXIST;
+ ret = -EBUSY;
goto out;
}
}
@@ -379,7 +379,7 @@ int nf_conntrack_helper_register(struct nf_conntrack_helper *me)
hlist_for_each_entry(cur, &nf_ct_helper_hash[h], hnode) {
if (nf_ct_tuple_src_mask_cmp(&cur->tuple, &me->tuple,
&mask)) {
- ret = -EEXIST;
+ ret = -EBUSY;
goto out;
}
}
--
2.49.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox