It turns out I had misunderstood how the x86_match_cpu() function works.
It evaluates a logical OR of the matching conditions, not logical AND.
This caused the CPU feature checks to pass even if only SSE2 (but not
AES-NI) was supported (ir vice versa), leading to potential crashes if
something tried to use the registered algs.
This patch fixes the checks to ensure that both required CPU features
are supported. The MODULE_DEVICE_TABLE declaration is specified only for
the AES-NI feature array, because I'm not sure what having multiple such
declarations would cause and I believe it should suffice to match on the
more important feature at the alias level and let the init routine do
the full check.
Signed-off-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx>
---
Hi Herbert,
this patch fixes the CPU checks of the AEGIS AES-NI/SSE2 implementations
that have been introduced in 4.18. Once reviewed, it should go to Linus'
tree since it may cause crashes on some systems if the corresponding
configs are enabled.
@x86 folks, please take a look if I use the MODULE_DEVICE_TABLE macro
correctly here (I'm not sure how to properly declare that the module
needs two CPU features to be both supported; all other modules I saw had
either only single match rule or required just one of the rules to
match).
Thanks,
Ondrej Mosnacek
arch/x86/crypto/aegis128-aesni-glue.c | 8 ++++++--
arch/x86/crypto/aegis128l-aesni-glue.c | 8 ++++++--
arch/x86/crypto/aegis256-aesni-glue.c | 8 ++++++--
3 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/arch/x86/crypto/aegis128-aesni-glue.c b/arch/x86/crypto/aegis128-aesni-glue.c
index 5de7c0d46edf..6a5abed59593 100644
--- a/arch/x86/crypto/aegis128-aesni-glue.c
+++ b/arch/x86/crypto/aegis128-aesni-glue.c
@@ -377,14 +377,18 @@ static struct aead_alg crypto_aegis128_aesni_alg[] = {
static const struct x86_cpu_id aesni_cpu_id[] = {
X86_FEATURE_MATCH(X86_FEATURE_AES),
- X86_FEATURE_MATCH(X86_FEATURE_XMM2),
{}
};
MODULE_DEVICE_TABLE(x86cpu, aesni_cpu_id);
+static const struct x86_cpu_id sse2_cpu_id[] = {
+ X86_FEATURE_MATCH(X86_FEATURE_XMM2),
+ {}
+};
+
static int __init crypto_aegis128_aesni_module_init(void)
{
- if (!x86_match_cpu(aesni_cpu_id))
+ if (!x86_match_cpu(aesni_cpu_id) || !x86_match_cpu(sse2_cpu_id))
return -ENODEV;
return crypto_register_aeads(crypto_aegis128_aesni_alg,
diff --git a/arch/x86/crypto/aegis128l-aesni-glue.c b/arch/x86/crypto/aegis128l-aesni-glue.c
index 876e4866e633..691c52701c2d 100644
--- a/arch/x86/crypto/aegis128l-aesni-glue.c
+++ b/arch/x86/crypto/aegis128l-aesni-glue.c
@@ -377,14 +377,18 @@ static struct aead_alg crypto_aegis128l_aesni_alg[] = {
static const struct x86_cpu_id aesni_cpu_id[] = {
X86_FEATURE_MATCH(X86_FEATURE_AES),
- X86_FEATURE_MATCH(X86_FEATURE_XMM2),
{}
};
MODULE_DEVICE_TABLE(x86cpu, aesni_cpu_id);
+static const struct x86_cpu_id sse2_cpu_id[] = {
+ X86_FEATURE_MATCH(X86_FEATURE_XMM2),
+ {}
+};
+
static int __init crypto_aegis128l_aesni_module_init(void)
{
- if (!x86_match_cpu(aesni_cpu_id))
+ if (!x86_match_cpu(aesni_cpu_id) || !x86_match_cpu(sse2_cpu_id))
return -ENODEV;
return crypto_register_aeads(crypto_aegis128l_aesni_alg,
diff --git a/arch/x86/crypto/aegis256-aesni-glue.c b/arch/x86/crypto/aegis256-aesni-glue.c
index 2b5dd3af8f4d..481b5e4f4fd0 100644
--- a/arch/x86/crypto/aegis256-aesni-glue.c
+++ b/arch/x86/crypto/aegis256-aesni-glue.c
@@ -377,14 +377,18 @@ static struct aead_alg crypto_aegis256_aesni_alg[] = {
static const struct x86_cpu_id aesni_cpu_id[] = {
X86_FEATURE_MATCH(X86_FEATURE_AES),
- X86_FEATURE_MATCH(X86_FEATURE_XMM2),
{}
};
MODULE_DEVICE_TABLE(x86cpu, aesni_cpu_id);
+static const struct x86_cpu_id sse2_cpu_id[] = {
+ X86_FEATURE_MATCH(X86_FEATURE_XMM2),
+ {}
+};
+
static int __init crypto_aegis256_aesni_module_init(void)
{
- if (!x86_match_cpu(aesni_cpu_id))
+ if (!x86_match_cpu(aesni_cpu_id) || !x86_match_cpu(sse2_cpu_id))
return -ENODEV;
return crypto_register_aeads(crypto_aegis256_aesni_alg,
--
2.17.1
