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]

Message-ID: <CAK7LNATDMjzmgpBHZFTOJCkTCqpLPq8jEjdrwzEZ3uu7WMG7jg@mail.gmail.com>
Date: Sat, 3 Feb 2024 00:01:26 +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 Wed, Jan 31, 2024 at 7:49 PM John Garry <john.g.garry@...cle.com> wrote:
>
> When hacking it is a waste of time and compute energy that we need to
> rebuild much kernel code just for changing the head git commit, like this:
>
> > touch include/generated/utsrelease.h
> > time make  -j3
> mkdir -p /home/john/mnt_sda4/john/kernel-dev2/tools/objtool && make O=/home/john/mnt_sda4/john/kernel-dev2 subdir=tools/objtool --no-print-directory -C objtool
>   INSTALL libsubcmd_headers
>   CALL    scripts/checksyscalls.sh
>   CC      init/version.o
>   AR      init/built-in.a
>   CC      kernel/sys.o
>   CC      kernel/module/main.o
>   AR      kernel/module/built-in.a
>   CC      drivers/base/firmware_loader/main.o
>   CC      kernel/trace/trace.o
>   AR      drivers/base/firmware_loader/built-in.a
>   AR      drivers/base/built-in.a
>   CC      net/ethtool/ioctl.o
>   AR      kernel/trace/built-in.a
>   AR      kernel/built-in.a
>   AR      net/ethtool/built-in.a
>   AR      net/built-in.a
>   AR      drivers/built-in.a
>   AR      built-in.a
>   ...
>
> Files like drivers/base/firmware_loader/main.c needs to be recompiled as
> it includes generated/utsrelease.h for UTS_RELEASE macro, and utsrelease.h
> is regenerated when the head commit changes.
>
> Introduce global char uts_release[] in init/version.c, which this
> mentioned code can use instead of UTS_RELEASE, meaning that we don't need
> to rebuild for changing the head commit - only init/version.c needs to be
> rebuilt. Whether all the references to UTS_RELEASE in the codebase are
> proper is a different matter.
>
> For an x86_64 defconfig build for this series on my old laptop, here is
> before and after rebuild time:
>
> before:
> real    0m53.591s
> user    1m1.842s
> sys     0m9.161s
>
> after:
> real    0m37.481s
> user    0m46.461s
> sys     0m7.199s
>
> Sending as an RFC as I need to test more of the conversions and I would
> like to also convert more UTS_RELEASE users to prove this is proper
> approach.
>
> John Garry (4):
>   init: Add uts_release
>   tracing: Use uts_release
>   net: ethtool: Use uts_release
>   firmware_loader: Use uts_release
>
>  drivers/base/firmware_loader/main.c | 39 +++++++++++++++++++++++------
>  include/linux/utsname.h             |  1 +
>  init/version.c                      |  3 +++
>  kernel/trace/trace.c                |  4 +--
>  net/ethtool/ioctl.c                 |  4 +--
>  5 files changed, 39 insertions(+), 12 deletions(-)
>
> --
> 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.
As you discussed in 3/4, if UTS_RELEASE is unneeded,
it is better to get rid of it.


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.


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?



I think what you can contribute are:

 - Explore the UTS_RELEASE users, and check if you can get rid of it.

 - Where UTS_RELEASE is useful, consider if it is possible
   to replace it with init_uts_ns.name.release



-- 
Best Regards
Masahiro Yamada