2022-09-06 03:02:40

by Peter Harliman Liem

[permalink] [raw]
Subject: [PATCH v2 2/2] crypto: inside-secure - Select CRYPTO_AES config

CRYPTO_AES is needed for aes-related algo (e.g.
safexcel-gcm-aes, safexcel-xcbc-aes, safexcel-cmac-aes).
Without it, we observe failures when allocating transform
for those algo.

Fixes: 363a90c2d517 ("crypto: safexcel/aes - switch to library version of key expansion routine")
Signed-off-by: Peter Harliman Liem <[email protected]>
---
v2:
Add fixes tag

drivers/crypto/Kconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 3e6aa319920b..b12d222e49a1 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -740,6 +740,7 @@ config CRYPTO_DEV_SAFEXCEL
select CRYPTO_SHA512
select CRYPTO_CHACHA20POLY1305
select CRYPTO_SHA3
+ select CRYPTO_AES
help
This driver interfaces with the SafeXcel EIP-97 and EIP-197 cryptographic
engines designed by Inside Secure. It currently accelerates DES, 3DES and
--
2.17.1


2022-09-06 15:19:34

by Antoine Tenart

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] crypto: inside-secure - Select CRYPTO_AES config

Quoting Peter Harliman Liem (2022-09-06 04:51:50)
> CRYPTO_AES is needed for aes-related algo (e.g.
> safexcel-gcm-aes, safexcel-xcbc-aes, safexcel-cmac-aes).
> Without it, we observe failures when allocating transform
> for those algo.
>
> Fixes: 363a90c2d517 ("crypto: safexcel/aes - switch to library version of key expansion routine")

The above commit explicitly switched crypto drivers to use the AES
library instead of the generic AES cipher one, which seems like a good
move. What are the issues you're encountering and why the AES lib makes
the driver to fail?

> Signed-off-by: Peter Harliman Liem <[email protected]>
> ---
> v2:
> Add fixes tag
>
> drivers/crypto/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
> index 3e6aa319920b..b12d222e49a1 100644
> --- a/drivers/crypto/Kconfig
> +++ b/drivers/crypto/Kconfig
> @@ -740,6 +740,7 @@ config CRYPTO_DEV_SAFEXCEL
> select CRYPTO_SHA512
> select CRYPTO_CHACHA20POLY1305
> select CRYPTO_SHA3
> + select CRYPTO_AES
> help
> This driver interfaces with the SafeXcel EIP-97 and EIP-197 cryptographic
> engines designed by Inside Secure. It currently accelerates DES, 3DES and
> --
> 2.17.1
>

2022-09-07 06:54:34

by Peter Harliman Liem

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] crypto: inside-secure - Select CRYPTO_AES config

On 6/9/2022 10:05 pm, Antoine Tenart wrote:
>> CRYPTO_AES is needed for aes-related algo (e.g.
>> safexcel-gcm-aes, safexcel-xcbc-aes, safexcel-cmac-aes).
>> Without it, we observe failures when allocating transform
>> for those algo.
>>
>> Fixes: 363a90c2d517 ("crypto: safexcel/aes - switch to library version of key expansion routine")
>
> The above commit explicitly switched crypto drivers to use the AES
> library instead of the generic AES cipher one, which seems like a good
> move. What are the issues you're encountering and why the AES lib makes
> the driver to fail?

If I load the kernel module (CONFIG_CRYPTO_MANAGER_DISABLE_TESTS is not
set), I am getting failure messages below.
IMHO this happens because some functions in the driver still rely on
generic AES cipher (e.g. refer to safexcel_aead_gcm_cra_init() or
safexcel_xcbcmac_cra_init()), therefore CONFIG_CRYPTO_AES is still needed.

Maybe the alternative is to switch all of them to use AES lib instead?
Let me know if you prefer this.

Thanks!

[ 157.683462] alg: aead: failed to allocate transform for
safexcel-gcm-aes: -2
[ 157.689054] ------------[ cut here ]------------
[ 157.693650] alg: self-tests for safexcel-gcm-aes (gcm(aes)) failed
(rc=-2)
[ 157.693696] WARNING: CPU: 3 PID: 164 at crypto/testmgr.c:5804
alg_test.part.0+0xd1/0x2f0 [cryptomgr]
[ 157.709505] Modules linked in: crypto_safexcel(+) md5 sha512_generic
libdes sha256_generic libsha256 sha1_gene
ric libsha1 libaes authenc cryptomgr akcipher crypto_acompress kpp rng
crypto_null crypto_hash skcipher aead cryp
to_algapi
[ 157.729991] CPU: 3 PID: 164 Comm: cryptomgr_test Not tainted
6.0.0-rc3+ #42
[ 157.736900] RIP: 0010:alg_test.part.0+0xd1/0x2f0 [cryptomgr]
[ 157.742512] Code: 6c 85 db 0f 84 89 00 00 00 80 3d 59 6c 05 00 00 0f
85 08 40 00 00 89 d9 4c 89 f2 48 89 ee 48
c7 c7 40 1c 09 a0 e8 cc 17 28 e2 <0f> 0b 48 8b 84 24 90 00 00 00 65 48
33 04 25 28 00 00 00 0f 85 eb
[ 157.761177] RSP: 0018:ffffc90000557e40 EFLAGS: 00010246
[ 157.766359] RAX: 0000000000000000 RBX: 00000000fffffffe RCX:
0000000000000000
[ 157.773440] RDX: 0000000000000001 RSI: 00000000ffffdfff RDI:
00000000ffffffff
[ 157.780528] RBP: ffff8880046ed200 R08: 0000000000000000 R09:
ffffc90000557ce8
[ 157.787610] R10: 0000000000000001 R11: 0000000000000003 R12:
0000000000011083
[ 157.794699] R13: 0000000000000400 R14: ffff8880046ed280 R15:
ffffffffffffffff
[ 157.801780] FS: 0000000000000000(0000) GS:ffff88807a580000(0000)
knlGS:0000000000000000
[ 157.809818] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 157.815516] CR2: 0000560cece6c0d8 CR3: 0000000002a0a000 CR4:
00000000001006a0
[ 157.822608] Call Trace:
[ 157.825018] <TASK>
[ 157.827104] ? _raw_spin_unlock+0xd/0x30
[ 157.830989] ? finish_task_switch+0x8a/0x260
[ 157.835221] ? __schedule+0x27c/0x670
[ 157.838855] ? 0xffffffffa004c000
[ 157.842131] cryptomgr_test+0x22/0x50 [cryptomgr]
[ 157.846804] kthread+0xd0/0x100
[ 157.849908] ? kthread_complete_and_exit+0x20/0x20
[ 157.854664] ret_from_fork+0x1f/0x30
[ 157.858209] </TASK>
[ 157.860346] ---[ end trace 0000000000000000 ]---
[ 157.908463] alg: hash: failed to allocate transform for
safexcel-xcbc-aes: -2
[ 157.914132] ------------[ cut here ]------------



2022-09-07 08:56:12

by Antoine Tenart

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] crypto: inside-secure - Select CRYPTO_AES config

Quoting Peter Harliman Liem (2022-09-07 08:46:32)
> On 6/9/2022 10:05 pm, Antoine Tenart wrote:
> >> CRYPTO_AES is needed for aes-related algo (e.g.
> >> safexcel-gcm-aes, safexcel-xcbc-aes, safexcel-cmac-aes).
> >> Without it, we observe failures when allocating transform
> >> for those algo.
> >>
> >> Fixes: 363a90c2d517 ("crypto: safexcel/aes - switch to library version of key expansion routine")
> >
> > The above commit explicitly switched crypto drivers to use the AES
> > library instead of the generic AES cipher one, which seems like a good
> > move. What are the issues you're encountering and why the AES lib makes
> > the driver to fail?
>
> If I load the kernel module (CONFIG_CRYPTO_MANAGER_DISABLE_TESTS is not
> set), I am getting failure messages below.
> IMHO this happens because some functions in the driver still rely on
> generic AES cipher (e.g. refer to safexcel_aead_gcm_cra_init() or
> safexcel_xcbcmac_cra_init()), therefore CONFIG_CRYPTO_AES is still needed.

That's possible, and the right fix might be what you proposed. I think
it would be nice to understand what is failing and where, so we have a
good argument for restoring the AES dependency (or not).

> Maybe the alternative is to switch all of them to use AES lib instead?
> Let me know if you prefer this.

If the AES lib can be used instead of the AES generic implementation
that would be great yes. If that's possible, depending on what is
actually failing, yes please go for this solution. Otherwise restoring
the AES dependency with a good explanation should work.

Thanks!
Antoine

2022-09-23 02:46:16

by Peter Harliman Liem

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] crypto: inside-secure - Select CRYPTO_AES config

On 7/9/2022 4:48 pm, Antoine Tenart wrote:
> Quoting Peter Harliman Liem (2022-09-07 08:46:32)
>> On 6/9/2022 10:05 pm, Antoine Tenart wrote:
>>>> CRYPTO_AES is needed for aes-related algo (e.g.
>>>> safexcel-gcm-aes, safexcel-xcbc-aes, safexcel-cmac-aes).
>>>> Without it, we observe failures when allocating transform
>>>> for those algo.
>>>>
>>>> Fixes: 363a90c2d517 ("crypto: safexcel/aes - switch to library version of key expansion routine")
>>>
>>> The above commit explicitly switched crypto drivers to use the AES
>>> library instead of the generic AES cipher one, which seems like a good
>>> move. What are the issues you're encountering and why the AES lib makes
>>> the driver to fail?
>>
>> If I load the kernel module (CONFIG_CRYPTO_MANAGER_DISABLE_TESTS is not
>> set), I am getting failure messages below.
>> IMHO this happens because some functions in the driver still rely on
>> generic AES cipher (e.g. refer to safexcel_aead_gcm_cra_init() or
>> safexcel_xcbcmac_cra_init()), therefore CONFIG_CRYPTO_AES is still needed.
>
> That's possible, and the right fix might be what you proposed. I think
> it would be nice to understand what is failing and where, so we have a
> good argument for restoring the AES dependency (or not).
>
>> Maybe the alternative is to switch all of them to use AES lib instead?
>> Let me know if you prefer this.
>
> If the AES lib can be used instead of the AES generic implementation
> that would be great yes. If that's possible, depending on what is
> actually failing, yes please go for this solution. Otherwise restoring
> the AES dependency with a good explanation should work.

I have replaced this commit in v3 with the change to AES lib instead.

Thanks!