linux-kernel - Re: RFC - KBUILD_MODNAME is misleading in builtins, as seen in /proc/dynamic

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: <CAK7LNASVpBih3iSHd=RXkKNZQ-v5LVzEOuZG3H_i3fcZfsGhDA@mail.gmail.com>
Date:   Tue, 21 Mar 2023 18:03:01 +0900
From:   Masahiro Yamada <masahiroy@...nel.org>
To:     Luis Chamberlain <mcgrof@...nel.org>
Cc:     jim.cromie@...il.com, linux-modules@...r.kernel.org,
        Nick Alcock <nick.alcock@...cle.com>,
        Aaron Tomlin <atomlin@...hat.com>,
        Jason Baron <jbaron@...mai.com>,
        LKML <linux-kernel@...r.kernel.org>,
        linux-kbuild@...r.kernel.org, Greg KH <gregkh@...uxfoundation.org>
Subject: Re: RFC - KBUILD_MODNAME is misleading in builtins, as seen in /proc/dynamic_debug/control

On Tue, Mar 21, 2023 at 6:04 AM Luis Chamberlain <mcgrof@...nel.org> wrote:
>
> On Mon, Mar 20, 2023 at 01:59:28PM -0600, jim.cromie@...il.com wrote:
> > On Mon, Mar 20, 2023 at 12:35 PM Jason Baron <jbaron@...mai.com> wrote:
> > >
> > >
> > >
> > > On 3/20/23 1:05 AM, jim.cromie@...il.com wrote:
> > > > dynamic-debug METADATA uses KBUILD_MODNAME as:
> > > >
> > > > #define DEFINE_DYNAMIC_DEBUG_METADATA_CLS(name, cls, fmt)       \
> > > >          static struct _ddebug  __aligned(8)                     \
> > > >          __section("__dyndbg") name = {                          \
> > > >                  .modname = KBUILD_MODNAME,                      \
> > > >
> > > > This is going amiss for some builtins, ie those enabled here, by:
> > > >
> > > >      echo module main +pmf > /proc/dynamic_debug_control
> > > >      grep =pmf /proc/dynamic_debug/control
> > > >
> > > > init/main.c:1187 [main]initcall_blacklist =pmf "blacklisting initcall %s\n"
> > > > init/main.c:1226 [main]initcall_blacklisted =pmf "initcall %s blacklisted\n"
> > > > init/main.c:1432 [main]run_init_process =pmf "  with arguments:\n"
> > > > init/main.c:1434 [main]run_init_process =pmf "    %s\n"
> > > > init/main.c:1435 [main]run_init_process =pmf "  with environment:\n"
> > > > init/main.c:1437 [main]run_init_process =pmf "    %s\n"
> > >
> > >
> > > Hi Jim,
> > >
> > > So if I'm following correctly, this is not a new issue, the 'module'
> > > name for dynamic debug has always been this way for builtin.
> >
> > It is not a new issue - both PM and init-main have been in [main] for some time.
> >
> > I believe that with
> > cfc1d277891e module: Move all into module/
> >
> > module's module-name joined them, changing from [module] to [main]
>
> If there was a regression due to this, we'd be very interested in
> hearing about it. Aaron he did the work to move the code to its own directory.
>
> > We could do
> > > something simple and just normalize it when we initially create the
> > > table, but setting the 'module name' to 'core' or 'builtin' or something
> > > for all these?
> >
> > core and builtin would both lump all those separate modules together,
> > making it less meaningful.
> >
> > having stable names independent of M vs Y config choices is imperative, ISTM.
> >
> > Also, I dont think "only builtins are affected" captures the whole problem.
> > I dont recall amdgpu or other modules changing when built with =y
> >
> > Theres some subtlety in how KBUILD_MODNAME is set,
> > and probably many current users who like its current behavior.
> > A new var ?
> >
> > 1st, I think that anything tristate gets a sensible value,
> > but at least some of the builtin-only "modules" get basenames, by default.
>
> In general we could all benefit from an enhancement for a shortname for
> things which could be modules being built-in. We're now seeing requests
> for dynamic debug, but it could also be usefulf for Nick's future work
> to help userspace tools / tracing map kallsysms to specific modules when
> built-in.



I think I rejected it some years ago.
He comes back again and again with almost the same approaches,
until he finds a "sponsor" (it's you) who will get it in.

Recently, I rejected the Kbuild changes again.




> To that end I had suggested the current state of affairs & current difficulty
> in trying to get us a name for this here:
>
> https://lore.kernel.org/all/Y/kXDqW+7d71C4wz@bombadil.infradead.org/
>
> I ended up suggesting perhaps we need a -DPOSSIBLE_MODULE then if we
> could *somehow* pull that off perhaps then we could instead use
> -DPOSSIBLE_KBUILD_MODNAME which would ensure a consistent symbol when
> a module is built-in as well.
>
> That still leaves the difficulty in trying to gather possible-obj-m as
> a future challenge.


I do not understand your point.

Why is it important to achieve "precisely-exactly-possible-obj-m" instead of
"perhaps-possible-obj-m"?


When "modprobe foo" succeeds, the user is sure that the kernel
provides the feature "foo" (but he does not care if
"foo" is built-in or modular).

I think that is the point for kmod check also module.builtin
before saying no.


When CONFIG_FOO=y, "modprobe foo" succeeds because "foo" is available
as built-in.
When CONFIG_FOO=n, "modprobe foo" fails because "foo" is not available anywhere.
I do not see anything wrong here.

Why do we need to make "modprobe foo" fail, where the feature "foo" is
still available
but just because we cannot compile it as a module?

He spams with MODULE_LICENSE removal with no justification.

-- 
Best Regards
Masahiro Yamada