netdev - Re: [PATCH RFC 0/4] Introduce uts

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]

Date: Tue, 6 Feb 2024 08:10:32 +0900
From: Masahiro Yamada <masahiroy@...nel.org>
To: John Garry <john.g.garry@...cle.com>
Cc: mcgrof@...nel.org, russ.weight@...ux.dev, gregkh@...uxfoundation.org, 
	rafael@...nel.org, rostedt@...dmis.org, mhiramat@...nel.org, 
	mathieu.desnoyers@...icios.com, davem@...emloft.net, edumazet@...gle.com, 
	kuba@...nel.org, pabeni@...hat.com, keescook@...omium.org, nathan@...nel.org, 
	nicolas@...sle.eu, linux-kernel@...r.kernel.org, 
	linux-trace-kernel@...r.kernel.org, netdev@...r.kernel.org, 
	linux-kbuild@...r.kernel.org
Subject: Re: [PATCH RFC 0/4] Introduce uts_release

On Mon, Feb 5, 2024 at 5:25 PM John Garry <john.g.garry@...cle.com> wrote:
>
> On 02/02/2024 15:01, Masahiro Yamada wrote:
> >> --
> >> 2.35.3
> >
> > As you see, several drivers store UTS_RELEASE in their driver data,
> > and even print it in debug print.
> >
> >
> > I do not see why it is useful.
>
> I would tend to agree, and mentioned that earlier.
>
> > As you discussed in 3/4, if UTS_RELEASE is unneeded,
> > it is better to get rid of it.
>
> Jakub replied about this.
>
> >
> >
> > If such version information is useful for drivers, the intention is
> > whether the version of the module, or the version of vmlinux.
> > That is a question.
> > They differ when CONFIG_MODVERSION.
> >
>
> I think often this information in UTS_RELEASE is shared as informative
> only, so the user can conveniently know the specific kernel git version.
>
> >
> > When module developers intend to printk the git version
> > from which the module was compiled from,
> > presumably they want to use UTS_RELEASE, which
> > was expanded at the compile time of the module.
> >
> > If you replace it with uts_release, it is the git version
> > of vmlinux.
> >
> >
> > Of course, the replacement is safe for always-builtin code.
> >
> >
> >
> > Lastly, we can avoid using UTS_RELEASE without relying
> > on your patch.
> >
> >
> >
> > For example, commit 3a3a11e6e5a2bc0595c7e36ae33c861c9e8c75b1
> > replaced  UTS_RELEASE with init_uts_ns.name.release
> >
> >
> > So, is your uts_release a shorthand of init_uts_ns.name.release?
>
> Yes - well that both are strings containing UTS_RELEASE. Using a struct
> sub-member is bit ungainly, but I suppose that we should not be making
> life easy for people using this.
>
> However we already have init_utsname in:
>
> static inline struct new_utsname *init_utsname(void)
> {
>         return &init_uts_ns.name;
> }
>
> So could use init_utsname()->release, which is a bit nicer.
>
> >
> >
> >
> > I think what you can contribute are:
> >
> >   - Explore the UTS_RELEASE users, and check if you can get rid of it.
>
> Unfortunately I expect resistance for this. I also expect places like FW
> loader it is necessary. And when this is used in sysfs, people will say
> that it is part of the ABI now.
>
> How about I send the patch to update to use init_uts_ns and mention also
> that it would be better to not use at all, if possible? I can cc you.


OK.


As I mentioned in the previous reply, the replacement is safe
for builtin code.

When you touch modular code, please pay a little more care,
because UTS_RELEASE and init_utsname()->release
may differ when CONFIG_MODVERSIONS=y.







>
> >
> >   - Where UTS_RELEASE is useful, consider if it is possible
> >     to replace it with init_uts_ns.name.release
>
> ok, but, as above, could use init_utsname()->release also


I am fine with it.


init_utsname()->release is more commonly used
(but less common than UTS_RELEASE)



$ git grep   'init_utsname()->release' | wc
     28      92    2065
$ git grep   'init_uts_ns.name.release' | wc
      7      34     587
$ git grep   'UTS_RELEASE' | wc
     57     304    4741




-- 
Best Regards
Masahiro Yamada