linux-kernel - Re: [PATCH] firmware_loader: Use init

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: <Zddt-U-6SdxkxqmD@bombadil.infradead.org>
Date: Thu, 22 Feb 2024 07:53:29 -0800
From: Luis Chamberlain <mcgrof@...nel.org>
To: John Garry <john.g.garry@...cle.com>
Cc: russ.weight@...ux.dev, gregkh@...uxfoundation.org, rafael@...nel.org,
	linux-kernel@...r.kernel.org, masahiroy@...nel.org
Subject: Re: [PATCH] firmware_loader: Use init_utsname()->release

On Thu, Feb 22, 2024 at 02:58:19PM +0000, John Garry wrote:
> Instead of using UTS_RELEASE, use init_utsname()->release, which means
> that we don't need to rebuild the code just for the git head commit
> changing.
> 
> Since UTS_RELEASE is used for fw_path[] and this points to const data,
> append init_utsname()->release dynamically to an intermediate string.
> 
> The check for appending uts release is if the string in fw_path[] ends
> in '/'. Since fw_path[] may include a path from the kernel command line
> in fw_path_para, and it would be valid for this string to end in '/',
> check for fw_path_para when appending uts release. 
> 
> Signed-off-by: John Garry <john.g.garry@...cle.com>
> ---
> Note: As mentioned by Masahiro in [0], when CONFIG_MODVERSIONS=y it
> could be possible for a driver to be built as a module with a different
> kernel baseline and so use a different UTS_RELEASE from the baseline. So
> now using init_utsname()->release could lead to a change in behaviour
> in this driver. However, considering the nature of this driver and how it
> would not make sense to build as module against a different tree, this

would not make sense to build it as an external module against against a
different tree, so this change should not have any effect to users.

> change should be ok.
> 
> [0] https://lore.kernel.org/lkml/CAK7LNAQ_r5yUjNpOppLkDBQ12sDxBYQTvRZGn1ng8D1POfZr_A@mail.gmail.com/

Thanks for doing this, could you send a v2 with the "Note:" removed but
instead include that blurb as part of the commit log as it is important
information to retain.

Could you also test the selftests to ensure nothing is broken ?

/tools/testing/selftests/firmware/fw_run_tests.sh

Reviewed-by: Luis Chamberlain <mcgrof@...nel.org>

  Luis