2022-01-21 19:02:41

by Jason A. Donenfeld

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

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?

Jason


2022-01-21 19:03:57

by Miles Chen

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

hi,

>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?

Actually, I am merging android-common tree and test our device.
I use arm64 and clang-r437112b.

I'm not sure which option is the right one, grep 'BLAKE' .config shows:

CONFIG_CRYPTO_BLAKE2B=y
# CONFIG_CRYPTO_BLAKE2S is not set
# CONFIG_CRYPTO_LIB_BLAKE2S_GENERIC=y

and... I found that my patch breaks arm32 build, sorry for that.

>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?

2022-01-21 19:06:52

by Jason A. Donenfeld

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

Hi Miles,

I'm actually not able to reproduce your oops. I'm using vanilla clang
13, cross compiling for arm64, with thin LTO enabled and CFI enabled.
Kernel seems to run fine.

Are there other settings that are needed to trigger this? Do you see
it in upstream clang or just the Android fork of clang?

Jason