| Message ID | 20220809141117.641543-1-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | modpost: fix module versioning when a symbol lacks valid CRC | expand |
On Tue, Aug 9, 2022 at 11:13 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > Since commit 7b4537199a4a ("kbuild: link symbol CRCs at final link, > removing CONFIG_MODULE_REL_CRCS"), module versioning is broken on > some architectures. Loading a module fails with "disagrees about > version of symbol module_layout". > > On such architectures (e.g. ARCH=sparc build with sparc64_defconfig), > modpost shows a warning, like follows: > > WARNING: modpost: EXPORT symbol "_mcount" [vmlinux] version generation failed, symbol will not be versioned. > Is "_mcount" prototyped in <asm/asm-prototypes.h>? > > Previously, it was a harmless warning (CRC check was just skipped), > but now wrong CRCs are used for comparison because invalid CRCs are > just skipped. > > $ sparc64-linux-gnu-nm -n vmlinux > [snip] > 0000000000c2cea0 r __ksymtab__kstrtol > 0000000000c2ceb8 r __ksymtab__kstrtoul > 0000000000c2ced0 r __ksymtab__local_bh_enable > 0000000000c2cee8 r __ksymtab__mcount > 0000000000c2cf00 r __ksymtab__printk > 0000000000c2cf18 r __ksymtab__raw_read_lock > 0000000000c2cf30 r __ksymtab__raw_read_lock_bh > [snip] > 0000000000c53b34 D __crc__kstrtol > 0000000000c53b38 D __crc__kstrtoul > 0000000000c53b3c D __crc__local_bh_enable > 0000000000c53b40 D __crc__printk > 0000000000c53b44 D __crc__raw_read_lock > 0000000000c53b48 D __crc__raw_read_lock_bh > > Please notice __crc__mcount is missing here. > > When the module subsystem looks up a CRC that comes after, it results > in reading out a wrong address. For example, when __crc__printk is > needed, the module subsystem reads 0xc53b44 instead of 0xc53b40. > > All CRC entries must be output for correct index accessing. Invalid > CRCs will be unused, but are needed to keep the one-to-one mapping > between __ksymtab_* and __crc_*. > > The best is to fix all modpost warnings, but several warnings are still > remaining on less popular architectures. > > Fixes: 7b4537199a4a ("kbuild: link symbol CRCs at final link, removing CONFIG_MODULE_REL_CRCS") > Reported-by: matoro <matoro_mailinglist_kernel@matoro.tk> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > Tested-by: matoro <matoro_mailinglist_kernel@matoro.tk> Applied to linux-kbuild/fixes. > --- > > scripts/mod/modpost.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > index 8484c0798f28..22dc332eebfa 100644 > --- a/scripts/mod/modpost.c > +++ b/scripts/mod/modpost.c > @@ -2018,13 +2018,11 @@ static void add_exported_symbols(struct buffer *buf, struct module *mod) > /* record CRCs for exported symbols */ > buf_printf(buf, "\n"); > list_for_each_entry(sym, &mod->exported_symbols, list) { > - if (!sym->crc_valid) { > + if (!sym->crc_valid) > warn("EXPORT symbol \"%s\" [%s%s] version generation failed, symbol will not be versioned.\n" > "Is \"%s\" prototyped in <asm/asm-prototypes.h>?\n", > sym->name, mod->name, mod->is_vmlinux ? "" : ".ko", > sym->name); > - continue; > - } > > buf_printf(buf, "SYMBOL_CRC(%s, 0x%08x, \"%s\");\n", > sym->name, sym->crc, sym->is_gpl_only ? "_gpl" : ""); > -- > 2.34.1 >
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 8484c0798f28..22dc332eebfa 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -2018,13 +2018,11 @@ static void add_exported_symbols(struct buffer *buf, struct module *mod) /* record CRCs for exported symbols */ buf_printf(buf, "\n"); list_for_each_entry(sym, &mod->exported_symbols, list) { - if (!sym->crc_valid) { + if (!sym->crc_valid) warn("EXPORT symbol \"%s\" [%s%s] version generation failed, symbol will not be versioned.\n" "Is \"%s\" prototyped in <asm/asm-prototypes.h>?\n", sym->name, mod->name, mod->is_vmlinux ? "" : ".ko", sym->name); - continue; - } buf_printf(buf, "SYMBOL_CRC(%s, 0x%08x, \"%s\");\n", sym->name, sym->crc, sym->is_gpl_only ? "_gpl" : "");
Since commit 7b4537199a4a ("kbuild: link symbol CRCs at final link, removing CONFIG_MODULE_REL_CRCS"), module versioning is broken on some architectures. Loading a module fails with "disagrees about version of symbol module_layout". On such architectures (e.g. ARCH=sparc build with sparc64_defconfig), modpost shows a warning, like follows: WARNING: modpost: EXPORT symbol "_mcount" [vmlinux] version generation failed, symbol will not be versioned. Is "_mcount" prototyped in <asm/asm-prototypes.h>? Previously, it was a harmless warning (CRC check was just skipped), but now wrong CRCs are used for comparison because invalid CRCs are just skipped. $ sparc64-linux-gnu-nm -n vmlinux [snip] 0000000000c2cea0 r __ksymtab__kstrtol 0000000000c2ceb8 r __ksymtab__kstrtoul 0000000000c2ced0 r __ksymtab__local_bh_enable 0000000000c2cee8 r __ksymtab__mcount 0000000000c2cf00 r __ksymtab__printk 0000000000c2cf18 r __ksymtab__raw_read_lock 0000000000c2cf30 r __ksymtab__raw_read_lock_bh [snip] 0000000000c53b34 D __crc__kstrtol 0000000000c53b38 D __crc__kstrtoul 0000000000c53b3c D __crc__local_bh_enable 0000000000c53b40 D __crc__printk 0000000000c53b44 D __crc__raw_read_lock 0000000000c53b48 D __crc__raw_read_lock_bh Please notice __crc__mcount is missing here. When the module subsystem looks up a CRC that comes after, it results in reading out a wrong address. For example, when __crc__printk is needed, the module subsystem reads 0xc53b44 instead of 0xc53b40. All CRC entries must be output for correct index accessing. Invalid CRCs will be unused, but are needed to keep the one-to-one mapping between __ksymtab_* and __crc_*. The best is to fix all modpost warnings, but several warnings are still remaining on less popular architectures. Fixes: 7b4537199a4a ("kbuild: link symbol CRCs at final link, removing CONFIG_MODULE_REL_CRCS") Reported-by: matoro <matoro_mailinglist_kernel@matoro.tk> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Tested-by: matoro <matoro_mailinglist_kernel@matoro.tk> --- scripts/mod/modpost.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)