linux-kernel - Re: [PATCH 2/3] modpost: .meminit.* is not in init section when CONFIG_MEMORY

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: <CAK7LNARX+qxzD-6ip9Q64Bvju3ACQ0uFPThkLgRqvmem-LQ9uw@mail.gmail.com>
Date: Sat, 6 Jul 2024 22:50:25 +0900
From: Masahiro Yamada <masahiroy@...nel.org>
To: Wei Yang <richard.weiyang@...il.com>
Cc: akpm@...ux-foundation.org, nathan@...nel.org, nicolas@...sle.eu, 
	linux-mm@...ck.org, linux-kernel@...r.kernel.org, 
	linux-kbuild@...r.kernel.org, Mike Rapoport <rppt@...nel.org>
Subject: Re: [PATCH 2/3] modpost: .meminit.* is not in init section when
 CONFIG_MEMORY_HOTPLUG set

On Sat, Jul 6, 2024 at 3:12 PM Wei Yang <richard.weiyang@...il.com> wrote:
>
> On Fri, Jul 05, 2024 at 06:54:56AM +0000, Wei Yang wrote:
> >On Wed, Jul 03, 2024 at 11:44:38PM +0900, Masahiro Yamada wrote:
> >>On Wed, Jul 3, 2024 at 8:40 AM Wei Yang <richard.weiyang@...il.com> wrote:
> >>>
> >>> .meminit.* is not put into init section when CONFIG_MEMORY_HOTPLUG is
> >>> set, since we define MEM_KEEP()/MEM_DISCARD() according to
> >>> CONFIG_MEMORY_HOTPLUG.
> >>>
> >>> Signed-off-by: Wei Yang <richard.weiyang@...il.com>
> >>> CC: Mike Rapoport (IBM) <rppt@...nel.org>
> >>> ---
> >>>  scripts/mod/modpost.c | 10 ++++++++++
> >>>  1 file changed, 10 insertions(+)
> >>
> >>
> >>
> >>NACK.
> >>
> >>
> >>The section mismatch is performed _unconditionally_.
> >>
> >>
> >>
> >>In the old days, we did this depending on relevant CONFIG options.
> >>It was more than 15 years ago that we stopped doing that.
> >>
> >>
> >>See this:
> >>
> >>
> >>commit eb8f689046b857874e964463619f09df06d59fad
> >>Author: Sam Ravnborg <sam@...nborg.org>
> >>Date:   Sun Jan 20 20:07:28 2008 +0100
> >>
> >>    Use separate sections for __dev/__cpu/__mem code/data
> >>
> >>
> >>
> >>
> >>So, if you wanted to check this only when CONFIG_MEMORY_HOTPLUG=n,
> >>you would need to add #ifdef CONFIG_MEMORY_HOTPLUG to include/linux/init.h
> >>
> >>That is what we did in the Linux 2.6.* era, which had much worse
> >>section mismatch coverage.
> >>
> >
> >Masahiro
> >
> >If you would give me some suggestions, I'd appreciate it a lot.
> >
> >The original thing I want to do is to put function __free_pages_core() in
> >__meminit section, since this function is only used by __init functions and
> >in memory_hotplug.c.  This means it is save to release it if
> >CONFIG_MEMORY_HOTPLUG is set.
> >
> >Then I add __meminit to function __free_pages_core() and face the warning from
> >modpost.
> >
> >  WARNING: modpost: vmlinux: section mismatch in reference: generic_online_page+0xa (section: .text) -> __free_pages_core (section: .meminit.text)
> >
> >A .text function calls init code is not proper. Then I add __meminit to
> >generic_online_page too. Then I face this warning from modpost.
> >
> >  WARNING: modpost: vmlinux: generic_online_page: EXPORT_SYMBOL used for init symbol. Remove __init or EXPORT_SYMBOL.
> >
>
> I guess I found the correct way.
>
> Add __ref to generic_online_page to not issue a warning.



Yes, __ref is used to bypass the section mismatch check.

Some functions in mm/memory_hotplug.c are annotated as __ref
to reference __meminit functions.

Adding __ref is the easy solution.



Having said that, I started to think
eb8f689046b857874e964463619f09df06d59fad was the wrong decision.
I will revert it.











-- 
Best Regards
Masahiro Yamada