linux-kernel - Re: [PATCH v7 03/11] kbuild: generate KSYMTAB entries by modpost

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAK7LNAQpgx5uYnQTb0Pu=uR=YPWkJX7P75p55Qva5okhRpaN_g@mail.gmail.com>
Date:   Thu, 22 Jun 2023 11:26:53 +0900
From:   Masahiro Yamada <masahiroy@...nel.org>
To:     Guenter Roeck <linux@...ck-us.net>
Cc:     linux-kbuild@...r.kernel.org, linux-kernel@...r.kernel.org,
        Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Nicolas Schier <nicolas@...sle.eu>,
        linux-um@...ts.infradead.org
Subject: Re: [PATCH v7 03/11] kbuild: generate KSYMTAB entries by modpost

On Thu, Jun 22, 2023 at 1:15 AM Guenter Roeck <linux@...ck-us.net> wrote:
>
> On Thu, Jun 08, 2023 at 11:24:20PM +0900, Masahiro Yamada wrote:
> > Commit 7b4537199a4a ("kbuild: link symbol CRCs at final link, removing
> > CONFIG_MODULE_REL_CRCS") made modpost output CRCs in the same way
> > whether the EXPORT_SYMBOL() is placed in *.c or *.S.
> >
> ...
>
> > We can do this better now; modpost can selectively emit KSYMTAB entries
> > that are really used by modules.
> >
>
> This patch results in
>
> Building alpha:defconfig ... failed
> --------------
> Error log:
> <stdin>:1519:2: warning: #warning syscall clone3 not implemented [-Wcpp]
> WARNING: modpost: "saved_config" [vmlinux] is COMMON symbol
> ERROR: modpost: vmlinux: page_is_ram: EXPORT_SYMBOL used for init symbol. Remove __init or EXPORT_SYMBOL.
>
> I don't know if other architectures are affected - linux-next is so broken
> that it is difficult to find root causes for all the breakages.


Thanks for the comprehensive build tests.

If you compare the build log, you will see
what has happened.




In Linus' tree (without this patch),

  MODPOST Module.symvers
WARNING: modpost: "saved_config" [vmlinux] is COMMON symbol
WARNING: modpost: vmlinux.o: EXPORT_SYMBOL used for init/exit symbol:
page_is_ram (section: .init.text)



In linux-next (with this patch),


  MODPOST Module.symvers
WARNING: modpost: "saved_config" [vmlinux] is COMMON symbol
ERROR: modpost: vmlinux: page_is_ram: EXPORT_SYMBOL used for init
symbol. Remove __init or EXPORT_SYMBOL.





The change is obvious - I just turned the combination
of __init and EXPORT_SYMBOL into an error.

But, it seemed too early to do this.

I will hold it back as a warning for now, as follows:



diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index bdf4244da993..412115a8202a 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1239,10 +1239,10 @@ static void check_export_symbol(struct module
*mod, struct elf_info *elf,
        s->is_func = (ELF_ST_TYPE(sym->st_info) == STT_FUNC);

        if (match(secname, PATTERNS(INIT_SECTIONS)))
-               error("%s: %s: EXPORT_SYMBOL used for init symbol.
Remove __init or EXPORT_SYMBOL.\n",
+               warn("%s: %s: EXPORT_SYMBOL used for init symbol.
Remove __init or EXPORT_SYMBOL.\n",
                      mod->name, name);
        else if (match(secname, PATTERNS(EXIT_SECTIONS)))
-               error("%s: %s: EXPORT_SYMBOL used for exit symbol.
Remove __exit or EXPORT_SYMBOL.\n",
+               warn("%s: %s: EXPORT_SYMBOL used for exit symbol.
Remove __exit or EXPORT_SYMBOL.\n",
                      mod->name, name);
 }






Fixing the alpha code is trivial.


diff --git a/arch/alpha/kernel/setup.c b/arch/alpha/kernel/setup.c
index 33bf3a627002..22131e2a9f57 100644
--- a/arch/alpha/kernel/setup.c
+++ b/arch/alpha/kernel/setup.c
@@ -385,7 +385,7 @@ setup_memory(void *kernel_end)
 #endif /* CONFIG_BLK_DEV_INITRD */
 }

-int __init
+int
 page_is_ram(unsigned long pfn)
 {
        struct memclust_struct * cluster;







I do not know much about the warning for "saved_config".



__attribute((common)) was added in the following commit:

https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/commit/?id=3c7940961fbf9f252e20f9c455f2fe63f273294c


It was more than 20 years ago, and there is
no commit description.
I do not know the intention of __attribute((common)).

I hope the maintainers will fix the warnings,
but I do not know if it is likely to happen.

MAINTAINERS says "Odd Fixes"

If you find a build regression, please let me know.
So far, I did not get new reports from 0day bot.


Thanks.




> Guenter
>
> ---
> Bisect log:
>
> # bad: [15e71592dbae49a674429c618a10401d7f992ac3] Add linux-next specific files for 20230621
> # good: [45a3e24f65e90a047bef86f927ebdc4c710edaa1] Linux 6.4-rc7
> git bisect start 'HEAD' 'v6.4-rc7'
> # bad: [e867e67cd55ae460c860ffd896c7fc96add2821c] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git
> git bisect bad e867e67cd55ae460c860ffd896c7fc96add2821c
> # bad: [57b289d5b1005a9c39d6d6567e0ef6115bd59cea] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/riscv/linux.git
> git bisect bad 57b289d5b1005a9c39d6d6567e0ef6115bd59cea
> # bad: [dc6399fc9ae6d2530fc38fb3ae96bcc8393bd66f] Merge branch 'for-next/perf' of git://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git
> git bisect bad dc6399fc9ae6d2530fc38fb3ae96bcc8393bd66f
> # good: [6d366ba598334a0457d917a7bf38efd118c5b7be] Merge branch 'mm-stable' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> git bisect good 6d366ba598334a0457d917a7bf38efd118c5b7be
> # good: [82fe2e45cdb00de4fa648050ae33bdadf9b3294a] perf pmus: Check if we can encode the PMU number in perf_event_attr.type
> git bisect good 82fe2e45cdb00de4fa648050ae33bdadf9b3294a
> # bad: [d2fa756910f88c2f5871775483744407cbf67933] Merge branch 'for-next' of git://git.infradead.org/users/hch/dma-mapping.git
> git bisect bad d2fa756910f88c2f5871775483744407cbf67933
> # good: [1b990bc8edc396a37a3ff1a43f7c329c361ee07c] Merge branch 'mm-nonmm-unstable' into mm-everything
> git bisect good 1b990bc8edc396a37a3ff1a43f7c329c361ee07c
> # good: [cff6e7f50bd315e5b39c4e46c704ac587ceb965f] kbuild: Add CLANG_FLAGS to as-instr
> git bisect good cff6e7f50bd315e5b39c4e46c704ac587ceb965f
> # bad: [8f3847e175a0044e2212fef772e7fa912270cd6d] ia64,export.h: replace EXPORT_DATA_SYMBOL* with EXPORT_SYMBOL*
> git bisect bad 8f3847e175a0044e2212fef772e7fa912270cd6d
> # good: [3a3f1e573a105328a2cca45a7cfbebabbf5e3192] modpost: fix off by one in is_executable_section()
> git bisect good 3a3f1e573a105328a2cca45a7cfbebabbf5e3192
> # good: [92e74fb6e6196d642505ae2b74a8e327202afef9] scripts/kallsyms: constify long_options
> git bisect good 92e74fb6e6196d642505ae2b74a8e327202afef9
> # good: [92e2921eeafdfca9acd9b83f07d2b7ca099bac24] ARC: define ASM_NL and __ALIGN(_STR) outside #ifdef __ASSEMBLY__ guard
> git bisect good 92e2921eeafdfca9acd9b83f07d2b7ca099bac24
> # bad: [bb2aa9a94b41b883037a56709d995c269204ade0] kbuild: generate KSYMTAB entries by modpost
> git bisect bad bb2aa9a94b41b883037a56709d995c269204ade0
> # good: [94d6cb68124b7a63f24fcc345795ba5f9a27e694] modpost: pass struct module pointer to check_section_mismatch()
> git bisect good 94d6cb68124b7a63f24fcc345795ba5f9a27e694
> # first bad commit: [bb2aa9a94b41b883037a56709d995c269204ade0] kbuild: generate KSYMTAB entries by modpost



-- 
Best Regards
Masahiro Yamada