On 5/12/24 8:32 PM, Jarkko Sakkinen wrote:
On Mon May 13, 2024 at 4:11 AM EEST, Joachim Vandersmissen wrote:
On 5/12/24 6:11 PM, Jarkko Sakkinen wrote:
On Sat May 11, 2024 at 9:23 AM EEST, Joachim Vandersmissen wrote:
v4: FIPS_SIGNATURE_SELFTEST_RSA is no longer user-configurable and will
be set when the dependencies are fulfilled.
---8<---
This is in wrong place. If the patch is applied it will be included to
the kernel git log. Please put your log before diffstat.
I will keep it in mind for the next round.
In preparation of adding new ECDSA self-tests, the existing data is
moved to a separate file. A new configuration option is added to
control the compilation of the separate file. This configuration option
also enforces dependencies that were missing from the existing
CONFIG_FIPS_SIGNATURE_SELFTEST option.
1. Please just call the thing by its name instead of building tension
with "the new configuration option".
2. Lacks the motivation of adding a new configuration option.
The configuration option is there to ensure that the RSA (or ECDSA)
self-tests only get compiled in when RSA (or ECDSA) is actually enabled.
Otherwise, the self-test will panic on boot. I can make this more
explicit in the commit message.
The old fips_signature_selftest is no longer an init function, but now
a helper function called from fips_signature_selftest_rsa.
This is confusing, please remove.
Fair enough, I'll remove it from the commit message.
Yeah, I mean it is good to enough to have a code change no need to
document it here :-)
So why just send this and not this plus the selftest? Feels incomplete
to me.
Do you mean the ECDSA self-test? I didn't include that one here because
I didn't want to make the commit too big.
So, I'd suggest to make a patch set with the second patch containing
the tests.
I think this is part of a patch set, is it not? There should be a 2/2
patch ("Add ECDSA signature verification self-test"), you should be on
CC for that one too.
BR, Jarkko
