linux-kernel - Re: [PATCH 10/17] tty: remove MODULE

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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZB1p5zRp7rlGGuCP@kroah.com>
Date:   Fri, 24 Mar 2023 10:14:15 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Geert Uytterhoeven <geert@...ux-m68k.org>
Cc:     Luis Chamberlain <mcgrof@...nel.org>,
        Masahiro Yamada <masahiroy@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Christoph Hellwig <hch@...radead.org>,
        Nick Alcock <nick.alcock@...cle.com>,
        linux-modules@...r.kernel.org, linux-kernel@...r.kernel.org,
        Hitomi Hasegawa <hasegawa-hitomi@...itsu.com>,
        Jiri Slaby <jirislaby@...nel.org>
Subject: Re: [PATCH 10/17] tty: remove MODULE_LICENSE in non-modules

On Fri, Mar 24, 2023 at 10:08:17AM +0100, Geert Uytterhoeven wrote:
> On Fri, Mar 10, 2023 at 8:42 PM Luis Chamberlain <mcgrof@...nel.org> wrote:
> > On Fri, Mar 10, 2023 at 08:31:30AM +0100, Greg Kroah-Hartman wrote:
> > > On Thu, Mar 09, 2023 at 02:38:10PM -0800, Luis Chamberlain wrote:
> > > > On Thu, Mar 09, 2023 at 05:15:42PM +0100, Greg Kroah-Hartman wrote:
> > > > > On Thu, Mar 02, 2023 at 09:17:52PM +0000, Nick Alcock wrote:
> > > > > > Since commit 8b41fc4454e ("kbuild: create modules.builtin without
> > > > > > Makefile.modbuiltin or tristate.conf"), MODULE_LICENSE declarations
> > > > > > are used to identify modules. As a consequence, uses of the macro
> > > > > > in non-modules will cause modprobe to misidentify their containing
> > > > > > object file as a module when it is not (false positives), and modprobe
> > > > > > might succeed rather than failing with a suitable error message.
> > > > > >
> > > > > > So remove it in the files in this commit, none of which can be built as
> > > > > > modules.
> > > > > >
> > > > > > Signed-off-by: Nick Alcock <nick.alcock@...cle.com>
> > > > > > Suggested-by: Luis Chamberlain <mcgrof@...nel.org>
> > > > > > Cc: Luis Chamberlain <mcgrof@...nel.org>
> > > > > > Cc: linux-modules@...r.kernel.org
> > > > > > Cc: linux-kernel@...r.kernel.org
> > > > > > Cc: Hitomi Hasegawa <hasegawa-hitomi@...itsu.com>
> > > > > > Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> > > > > > Cc: Jiri Slaby <jirislaby@...nel.org>
> > > > > > ---
> > > > > >  drivers/tty/n_null.c | 1 -
> > > > > >  1 file changed, 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/tty/n_null.c b/drivers/tty/n_null.c
> > > > > > index f913b665af725..c24f75942c49d 100644
> > > > > > --- a/drivers/tty/n_null.c
> > > > > > +++ b/drivers/tty/n_null.c
> > > > > > @@ -63,7 +63,6 @@ static void __exit n_null_exit(void)
> > > > > >  module_init(n_null_init);
> > > > > >  module_exit(n_null_exit);
> > > > > >
> > > > > > -MODULE_LICENSE("GPL");
> > > > > >  MODULE_AUTHOR("Alan Cox");
> > > > > >  MODULE_ALIAS_LDISC(N_NULL);
> > > > > >  MODULE_DESCRIPTION("Null ldisc driver");
> > > > > > --
> > > > > > 2.39.1.268.g9de2f9a303
> > > > > >
> > > > >
> > > > > Nope, sorry, this is not good to do, please fix kbuild instead of
> > > > > forcing a tree-wide change like this.
> > > >
> > > > Masahiro Yamada already NACK'd it such effort:
> > > >
> > > > https://lkml.kernel.org/r/CAK7LNAQLttPD=Ae==e0CYeQtS78=o_JZFK+zxa29JnUYio52Ug@mail.gmail.com
> > > >
> > > > And his descriptiuon of the reasoning and logic is explained here:
> > > >
> > > > https://lore.kernel.org/all/CAK7LNASL7_RgfASstBvN6AzhR=nMU=HsQvODf5q13Xud8tBWRQ@mail.gmail.com/
> > > >
> > > > Let me summarize it though with a few quotes from him:
> > > >
> > > > "Having false-positives in modules.builtin should be OK"
> > > > "In this sense, having always-builtin entries in module.builtin is OK."
> > >
> > > None of that matters, sorry.
> > >
> > > Again, all I am saying is that you can not have some MODULE_() macros
> > > that are ok for code that is built in, and some that are not, for
> > > "reasons" that have to do how you all are treating the build system
> > > infrastructure as you are now putting arbritrary requirements for all
> > > driver authors (of which there are thousands) to know this.
> >
> > As noted once again, it is not putting hard requirement. Future tooling
> > not yet added would just not benefit from distinguishing symbols for
> > your modules.
> >
> > I'm happy to live with module authors not wanting to remove the module
> > license tag from their modules if they can never actually be modules
> > from not benefitting from the above tooling gains as its just cherry
> > on top tooling gains.
> 
> Apparently lots of these patches have not arrived in linux-next
> without Acks (we're still discussing about this, right???).
> 
> And some of the modified files have no SPDX-License-Identifier
> lines yet, so we are losing important licensing information:
> 
> $ git grep -L SPDX-License-Identifier -- $(git show $(git log
> --oneline v6.3-rc1..linux-next/master | grep "remove MODULE_LICENSE in
> non-modules" | cut -d " " -f 1) | lsdiff --strip=1)
> drivers/bus/arm-cci.c
> drivers/bus/imx-weim.c
> drivers/bus/simple-pm-bus.c
> drivers/gpu/drm/drm_mipi_dsi.c
> drivers/irqchip/irq-mvebu-pic.c
> drivers/reset/reset-axs10x.c
> drivers/reset/reset-hsdk.c
> drivers/soc/sunxi/sunxi_sram.c
> drivers/video/fbdev/asiliantfb.c
> drivers/video/fbdev/gbefb.c
> drivers/video/fbdev/imsttfb.c
> drivers/xen/xenbus/xenbus_probe.c
> lib/glob.c

Ick, that's not ok at all.

Again, I strongly feel that removing MODULE_LICENSE() lines from files
that just don't happen to be built as a module is not ok as no other
MODULE_*() macro has this arbitrary restriction.

thanks,

greg k-h