Thread (35 messages) 35 messages, 9 authors, 2022-01-26

Re: [PATCH] lib/crypto: blake2s: fix a CFI failure

From: Ard Biesheuvel <ardb@kernel.org>
Date: 2022-01-19 09:14:08
Also in: linux-crypto, linux-mediatek, lkml

On Wed, 19 Jan 2022 at 10:09, Ard Biesheuvel [off-list ref] wrote:
quoted hunk ↗ jump to hunk
(+ Sami, Eric)

On Wed, 19 Jan 2022 at 10:00, Jason A. Donenfeld [off-list ref] wrote:
quoted
Hi Miles,

Thanks for the patch. Could you let me know which architecture and
compiler this was broken on? If I had to guess, I'd wager arm32, and
you hit this by enabling optimized blake2s?

If so, I'm not sure the problem is with weak symbols. Why should CFI
break weak symbols? Rather, perhaps the issue is that the function is
defined in blake2s-core.S? Are there some CFI macros we need for that
definition?
We should try to understand why CFI thinks the prototypes of the two
symbols are different. There are still a number of issues with CFI, so
papering over them by reverting stuff that we want for good reasons is
not the way to go imo.

In the short term, you can work around it by avoiding the indirect
call to blake2s_compress, e.g.,
diff --git a/lib/crypto/blake2s.c b/lib/crypto/blake2s.c
index 93f2ae051370..fef2ff678431 100644
--- a/lib/crypto/blake2s.c
+++ b/lib/crypto/blake2s.c
@@ -16,9 +16,15 @@
 #include <linux/init.h>
 #include <linux/bug.h>

+static void __blake2s_compress(struct blake2s_state *state, const u8 *block,
+                              size_t nblocks, const u32 inc)
+{
+       return blake2s_compress(state, block, nblocks, inc);
+}
+
 void blake2s_update(struct blake2s_state *state, const u8 *in, size_t inlen)
 {
-       __blake2s_update(state, in, inlen, blake2s_compress);
+       __blake2s_update(state, in, inlen, __blake2s_compress);
 }
 EXPORT_SYMBOL(blake2s_update);
Ehm, maybe not. As Jason points out, the typedef does not have quite
the right type, so that is most likely the culprit, and this
workaround would trigger CFI in exactly the same way.

Interestingly, the compiler does not seem to mind, right? Or are you
seeing any build time warnings on the reference to blake2s_compress in
the call to __blake2s_update() ?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help