* [PATCH -next] crypto: curve25519 - Fix selftests build error
@ 2020-01-07 13:35 YueHaibing
2020-01-07 15:46 ` Jason A. Donenfeld
0 siblings, 1 reply; 4+ messages in thread
From: YueHaibing @ 2020-01-07 13:35 UTC (permalink / raw)
To: herbert, davem, Jason; +Cc: linux-crypto, linux-kernel, YueHaibing
If CRYPTO_CURVE25519 is y, CRYPTO_LIB_CURVE25519_GENERIC will be
y, but CRYPTO_LIB_CURVE25519 may be set to m, cause build errors:
lib/crypto/curve25519-selftest.o: In function `curve25519':
curve25519-selftest.c:(.text.unlikely+0xc): undefined reference to `curve25519_arch'
lib/crypto/curve25519-selftest.o: In function `curve25519_selftest':
curve25519-selftest.c:(.init.text+0x17e): undefined reference to `curve25519_base_arch'
This splits the curve25519 test code into its own source file.
Reported-by: Hulk Robot <hulkci@huawei.com>
Fixes: aa127963f1ca ("crypto: lib/curve25519 - re-add selftests")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
lib/crypto/Makefile | 6 +++++-
lib/crypto/curve25519-selftest.c | 23 ++++++++++++++++++++++-
lib/crypto/curve25519.c | 17 -----------------
3 files changed, 27 insertions(+), 19 deletions(-)
diff --git a/lib/crypto/Makefile b/lib/crypto/Makefile
index f97f9b94..87c99da 100644
--- a/lib/crypto/Makefile
+++ b/lib/crypto/Makefile
@@ -36,5 +36,9 @@ libsha256-y := sha256.o
ifneq ($(CONFIG_CRYPTO_MANAGER_DISABLE_TESTS),y)
libblake2s-y += blake2s-selftest.o
libchacha20poly1305-y += chacha20poly1305-selftest.o
-libcurve25519-y += curve25519-selftest.o
+ifneq ($(CONFIG_CRYPTO_ARCH_HAVE_LIB_CURVE25519),)
+obj-$(CONFIG_CRYPTO_ARCH_HAVE_LIB_CURVE25519) += curve25519-selftest.o
+else
+obj-$(CONFIG_CRYPTO_LIB_CURVE25519_GENERIC) += curve25519-selftest.o
+endif
endif
diff --git a/lib/crypto/curve25519-selftest.c b/lib/crypto/curve25519-selftest.c
index c85e853..c4cfa26 100644
--- a/lib/crypto/curve25519-selftest.c
+++ b/lib/crypto/curve25519-selftest.c
@@ -4,6 +4,8 @@
*/
#include <crypto/curve25519.h>
+#include <linux/module.h>
+#include <linux/init.h>
struct curve25519_test_vector {
u8 private[CURVE25519_KEY_SIZE];
@@ -1280,7 +1282,7 @@ static const struct curve25519_test_vector curve25519_test_vectors[] __initconst
}
};
-bool __init curve25519_selftest(void)
+static bool __init curve25519_selftest(void)
{
bool success = true, ret, ret2;
size_t i = 0, j;
@@ -1319,3 +1321,22 @@ bool __init curve25519_selftest(void)
return success;
}
+
+static int __init mod_init(void)
+{
+ if (!IS_ENABLED(CONFIG_CRYPTO_MANAGER_DISABLE_TESTS) &&
+ WARN_ON(!curve25519_selftest()))
+ return -ENODEV;
+ return 0;
+}
+
+static void __exit mod_exit(void)
+{
+}
+
+module_init(mod_init);
+module_exit(mod_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Curve25519 selftest");
+MODULE_AUTHOR("Jason A. Donenfeld <Jason@zx2c4.com>");
diff --git a/lib/crypto/curve25519.c b/lib/crypto/curve25519.c
index c03ccdb..0106beb 100644
--- a/lib/crypto/curve25519.c
+++ b/lib/crypto/curve25519.c
@@ -13,8 +13,6 @@
#include <linux/module.h>
#include <linux/init.h>
-bool curve25519_selftest(void);
-
const u8 curve25519_null_point[CURVE25519_KEY_SIZE] __aligned(32) = { 0 };
const u8 curve25519_base_point[CURVE25519_KEY_SIZE] __aligned(32) = { 9 };
@@ -22,21 +20,6 @@ EXPORT_SYMBOL(curve25519_null_point);
EXPORT_SYMBOL(curve25519_base_point);
EXPORT_SYMBOL(curve25519_generic);
-static int __init mod_init(void)
-{
- if (!IS_ENABLED(CONFIG_CRYPTO_MANAGER_DISABLE_TESTS) &&
- WARN_ON(!curve25519_selftest()))
- return -ENODEV;
- return 0;
-}
-
-static void __exit mod_exit(void)
-{
-}
-
-module_init(mod_init);
-module_exit(mod_exit);
-
MODULE_LICENSE("GPL v2");
MODULE_DESCRIPTION("Curve25519 scalar multiplication");
MODULE_AUTHOR("Jason A. Donenfeld <Jason@zx2c4.com>");
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH -next] crypto: curve25519 - Fix selftests build error
2020-01-07 13:35 [PATCH -next] crypto: curve25519 - Fix selftests build error YueHaibing
@ 2020-01-07 15:46 ` Jason A. Donenfeld
2020-01-08 4:37 ` [v2 PATCH] crypto: curve25519 - Fix selftest " Herbert Xu
0 siblings, 1 reply; 4+ messages in thread
From: Jason A. Donenfeld @ 2020-01-07 15:46 UTC (permalink / raw)
To: YueHaibing; +Cc: Herbert Xu, David Miller, Linux Crypto Mailing List, LKML
Thanks for catching this. While the pattern of adding the test here
followed the already-working pattern used by the blake2s
implementation, curve25519's wiring differs in one way: the arch code
is not related to any of the generic machinery. So this seems like an
okay way of fixing this for the time being.
Reviewed-by: Jason A. Donenfeld <Jason@zx2c4.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [v2 PATCH] crypto: curve25519 - Fix selftest build error
2020-01-07 15:46 ` Jason A. Donenfeld
@ 2020-01-08 4:37 ` Herbert Xu
2020-01-11 0:16 ` Jason A. Donenfeld
0 siblings, 1 reply; 4+ messages in thread
From: Herbert Xu @ 2020-01-08 4:37 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: YueHaibing, David Miller, Linux Crypto Mailing List, LKML
On Tue, Jan 07, 2020 at 10:46:49AM -0500, Jason A. Donenfeld wrote:
> Thanks for catching this. While the pattern of adding the test here
> followed the already-working pattern used by the blake2s
> implementation, curve25519's wiring differs in one way: the arch code
> is not related to any of the generic machinery. So this seems like an
> okay way of fixing this for the time being.
There is one problem with this patch, it causes the self-test to be
enabled even when CONFIG_CRYPTO_LIB_CURVE25519 is off. So I think
what we should do is simply copy the blake2s paradigm more fully
by adding a dummy module for curve25519 that hosts just the
self-test. Something like this:
---8<---
If CRYPTO_CURVE25519 is y, CRYPTO_LIB_CURVE25519_GENERIC will be
y, but CRYPTO_LIB_CURVE25519 may be set to m, this causes build
errors:
lib/crypto/curve25519-selftest.o: In function `curve25519':
curve25519-selftest.c:(.text.unlikely+0xc): undefined reference to `curve25519_arch'
lib/crypto/curve25519-selftest.o: In function `curve25519_selftest':
curve25519-selftest.c:(.init.text+0x17e): undefined reference to `curve25519_base_arch'
This is because the curve25519 self-test code is being controlled
by the GENERIC option rather than the overall CURVE25519 option,
as is the case with blake2s. To recap, the GENERIC and ARCH options
for CURVE25519 are internal only and selected by users such as
the Crypto API, or the externally visible CURVE25519 option which
in turn is selected by wireguard. The self-test is specific to the
the external CURVE25519 option and should not be enabled by the
Crypto API.
This patch fixes this by splitting the GENERIC module from the
CURVE25519 module with the latter now containing just the self-test.
Reported-by: Hulk Robot <hulkci@huawei.com>
Fixes: aa127963f1ca ("crypto: lib/curve25519 - re-add selftests")
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/lib/crypto/Makefile b/lib/crypto/Makefile
index f97f9b941110..5241e140a7ae 100644
--- a/lib/crypto/Makefile
+++ b/lib/crypto/Makefile
@@ -19,9 +19,12 @@ libblake2s-y += blake2s.o
obj-$(CONFIG_CRYPTO_LIB_CHACHA20POLY1305) += libchacha20poly1305.o
libchacha20poly1305-y += chacha20poly1305.o
-obj-$(CONFIG_CRYPTO_LIB_CURVE25519_GENERIC) += libcurve25519.o
-libcurve25519-y := curve25519-fiat32.o
-libcurve25519-$(CONFIG_ARCH_SUPPORTS_INT128) := curve25519-hacl64.o
+obj-$(CONFIG_CRYPTO_LIB_CURVE25519_GENERIC) += libcurve25519-generic.o
+libcurve25519-generic-y := curve25519-fiat32.o
+libcurve25519-generic-$(CONFIG_ARCH_SUPPORTS_INT128) := curve25519-hacl64.o
+libcurve25519-generic-y += curve25519-generic.o
+
+obj-$(CONFIG_CRYPTO_LIB_CURVE25519) += libcurve25519.o
libcurve25519-y += curve25519.o
obj-$(CONFIG_CRYPTO_LIB_DES) += libdes.o
diff --git a/lib/crypto/curve25519.c b/lib/crypto/curve25519.c
index c03ccdb99434..288a62cd29b2 100644
--- a/lib/crypto/curve25519.c
+++ b/lib/crypto/curve25519.c
@@ -15,13 +15,6 @@
bool curve25519_selftest(void);
-const u8 curve25519_null_point[CURVE25519_KEY_SIZE] __aligned(32) = { 0 };
-const u8 curve25519_base_point[CURVE25519_KEY_SIZE] __aligned(32) = { 9 };
-
-EXPORT_SYMBOL(curve25519_null_point);
-EXPORT_SYMBOL(curve25519_base_point);
-EXPORT_SYMBOL(curve25519_generic);
-
static int __init mod_init(void)
{
if (!IS_ENABLED(CONFIG_CRYPTO_MANAGER_DISABLE_TESTS) &&
diff --git a/lib/crypto/curve25519-generic.c b/lib/crypto/curve25519-generic.c
new file mode 100644
index 000000000000..de7c99172fa2
--- /dev/null
+++ b/lib/crypto/curve25519-generic.c
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Copyright (C) 2015-2019 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
+ *
+ * This is an implementation of the Curve25519 ECDH algorithm, using either
+ * a 32-bit implementation or a 64-bit implementation with 128-bit integers,
+ * depending on what is supported by the target compiler.
+ *
+ * Information: https://cr.yp.to/ecdh.html
+ */
+
+#include <crypto/curve25519.h>
+#include <linux/module.h>
+
+const u8 curve25519_null_point[CURVE25519_KEY_SIZE] __aligned(32) = { 0 };
+const u8 curve25519_base_point[CURVE25519_KEY_SIZE] __aligned(32) = { 9 };
+
+EXPORT_SYMBOL(curve25519_null_point);
+EXPORT_SYMBOL(curve25519_base_point);
+EXPORT_SYMBOL(curve25519_generic);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Curve25519 scalar multiplication");
+MODULE_AUTHOR("Jason A. Donenfeld <Jason@zx2c4.com>");
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-01-11 0:17 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-07 13:35 [PATCH -next] crypto: curve25519 - Fix selftests build error YueHaibing
2020-01-07 15:46 ` Jason A. Donenfeld
2020-01-08 4:37 ` [v2 PATCH] crypto: curve25519 - Fix selftest " Herbert Xu
2020-01-11 0:16 ` Jason A. Donenfeld
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 NNTP newsgroup(s).