linux-kernel - Re: [PATCH 2/2] crypto: asymmetric_keys - simplify asymmetric_key_hex_to_key

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] [day] [month] [year] [list]

Message-ID: <aOyjhBVI1osybEGb@wunner.de>
Date: Mon, 13 Oct 2025 09:00:20 +0200
From: Lukas Wunner <lukas@...ner.de>
To: Thorsten Blum <thorsten.blum@...ux.dev>
Cc: David Howells <dhowells@...hat.com>,
	Ignat Korchagin <ignat@...udflare.com>,
	Herbert Xu <herbert@...dor.apana.org.au>,
	"David S. Miller" <davem@...emloft.net>, keyrings@...r.kernel.org,
	linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] crypto: asymmetric_keys - simplify
 asymmetric_key_hex_to_key_id

On Sun, Oct 12, 2025 at 03:23:02PM +0200, Thorsten Blum wrote:
> On 12. Oct 2025, at 14:10, Lukas Wunner wrote:
> > On Tue, Oct 07, 2025 at 08:52:21PM +0200, Thorsten Blum wrote:
> > > Use struct_size() to calculate the number of bytes to allocate for the
> > > asymmetric key id.
> > 
> > Why?  To what end?  To guard against an overflow?
> 
> I find struct_size() to be more readable because it explicitly
> communicates the relationship between the flexible array member 'data'
> and 'asciihexlen / 2', which the open-coded version doesn't.
> 
> 'sizeof(struct asymmetric_key_id) + asciihexlen / 2' works because the
> flexible array 'data' is an unsigned char (1 byte). This will probably
> never change, but struct_size() would still work even if it did change
> to a data type that isn't exactly 1 byte.
> 
> Additionally, struct_size() has some extra compile-time checks (e.g.,
> __must_be_array()).

My view is that mere readability improvements (which are subjective)
and theoretical safety gains are not sufficient to justify a change.

In general submission of many small treewide refactoring changes in a
shotgun fashion is problematic because they occupy reviewers' and
maintainers' time (which is a scarce resource) and they mess up the
git history (complicating git blame), often for little to no gain.
I've heard one x86 maintainer say he considers them "more harm than good
nowadays".

In this particular case, the struct size calculation can never overflow
because even if asciihexlen has SIZE_MAX, the division by 2 ensures that
adding sizeof(struct asymmetric_key_id) doesn't cause an overflow.
So this patch doesn't seem to solve an actual problem.

Thanks,

Lukas