From: Lukas Wunner <lukas@wunner.de>
To: Thorsten Blum <thorsten.blum@linux.dev>
Cc: David Howells <dhowells@redhat.com>,
Ignat Korchagin <ignat@cloudflare.com>,
Herbert Xu <herbert@gondor.apana.org.au>,
"David S. Miller" <davem@davemloft.net>,
keyrings@vger.kernel.org, linux-crypto@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] crypto: asymmetric_keys - simplify asymmetric_key_hex_to_key_id
Date: Mon, 13 Oct 2025 09:00:20 +0200 [thread overview]
Message-ID: <aOyjhBVI1osybEGb@wunner.de> (raw)
In-Reply-To: <A0C349D1-C7FA-40C6-971B-910122B1AAE1@linux.dev>
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
next prev parent reply other threads:[~2025-10-13 7:00 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-07 18:52 [PATCH 1/2] crypto: asymmetric_keys - prevent overflow in asymmetric_key_generate_id Thorsten Blum
2025-10-07 18:52 ` [PATCH 2/2] crypto: asymmetric_keys - simplify asymmetric_key_hex_to_key_id Thorsten Blum
2025-10-12 12:10 ` Lukas Wunner
2025-10-12 13:23 ` Thorsten Blum
2025-10-13 7:00 ` Lukas Wunner [this message]
2025-10-12 7:38 ` [PATCH 1/2] crypto: asymmetric_keys - prevent overflow in asymmetric_key_generate_id Lukas Wunner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aOyjhBVI1osybEGb@wunner.de \
--to=lukas@wunner.de \
--cc=davem@davemloft.net \
--cc=dhowells@redhat.com \
--cc=herbert@gondor.apana.org.au \
--cc=ignat@cloudflare.com \
--cc=keyrings@vger.kernel.org \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=thorsten.blum@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).