[PATCH v7 00/12] crypto: qce - Fix crypto self-test failures

Bartosz Golaszewski posted 12 patches 2 weeks, 1 day ago
There is a newer version of this series
arch/arm64/configs/defconfig  |  1 +
drivers/crypto/Kconfig        |  4 +-
drivers/crypto/qce/aead.c     | 44 ++++++++++++++++----
drivers/crypto/qce/cipher.h   |  1 +
drivers/crypto/qce/common.h   |  1 -
drivers/crypto/qce/core.c     | 93 +++++++++++++++++++------------------------
drivers/crypto/qce/core.h     |  5 ++-
drivers/crypto/qce/sha.c      | 70 ++++++++++++++++++++++----------
drivers/crypto/qce/skcipher.c | 62 ++++++++++++++++++++++-------
drivers/soc/qcom/Kconfig      | 11 +++++
drivers/soc/qcom/Makefile     |  1 +
drivers/soc/qcom/qce-core.c   | 87 ++++++++++++++++++++++++++++++++++++++++
12 files changed, 283 insertions(+), 97 deletions(-)
[PATCH v7 00/12] crypto: qce - Fix crypto self-test failures
Posted by Bartosz Golaszewski 2 weeks, 1 day ago
This iteration - in addition to the previous fixes - proposes to split
the QCE driver into a core part necessary to bind to the QCE DT node and
enable runtime power management in order to allow to drop the
interconnect votes, and the crypto part registering the crypto
algorithms. The core module is then enabled in arm64 defconfig while the
crypto part stays disabled by default.

The QCE hardware crypto engine has several limitations that cause it to
produce incorrect results or stall on certain inputs. This series fixes
several bugs and adds workaround allowing the deiver to pass crypto
self-tests.

The failures addressed are:

- HMAC self-test failures for empty messages
- AES-XTS returning success on zero-length input (should be -EINVAL)
- AES-CTR: partial final block causes the engine to stall, output IV
  derivation was incorrect
- AES-XTS with key1 == key2 is not supported by the CE
- AES-CCM: partial final block and fragmented payload both stall the
  engine

All fixes were tested on an SM8650 QRD board with
CONFIG_CRYPTO_SELFTESTS=y and CONFIG_CRYPTO_SELFTESTS_FULL=y.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
Changes in v7:
- Add follow-up changes converting the QCE crypto driver to auxiliary
  bus, adding the core QCE driver under drivers/soc/ registering the
  auxiliary device and removing the BROKEN Kconfig label
- Link to v6: https://patch.msgid.link/20260717-qce-fix-self-tests-v6-0-455775fe5f6c@oss.qualcomm.com

Changes in v6:
- Handle all zero-length HMAC finalizations (like imported state with
  data already hashed), not only the genuinely empty message case
- Add an additional patch addressing the fragmented skcipher payload
  issue
- Link to v5: https://patch.msgid.link/20260706-qce-fix-self-tests-v5-0-86f461ff1829@oss.qualcomm.com

Changes in v5:
- Dropped patch 1/8 that's already queued
- Use the pre-allocated fallback ahash for HMAC transforms (Herbert)
- Link to v4: https://patch.msgid.link/20260622-qce-fix-self-tests-v4-0-4f82ffa716c6@oss.qualcomm.com

Changes in v4:
- Remove remaining ECB and DES3 bits
- Pick up tags
- Link to v3: https://patch.msgid.link/20260617-qce-fix-self-tests-v3-0-ecc2b4dedcfd@oss.qualcomm.com

Changes in v3:
- Remove even more algorithms and dead code in patch 1/8
- Link to v2: https://patch.msgid.link/20260615-qce-fix-self-tests-v2-0-dc911f1aad42@oss.qualcomm.com

Changes in v2:
- Add fixes for the full suite of crypto self-tests
- Add Fixes and Cc tags
- Link to v1: https://patch.msgid.link/20260610-qce_selftest_fix-v1-0-1b0504783a46@oss.qualcomm.com/

---
Bartosz Golaszewski (10):
      crypto: qce - Fix HMAC self-test failures for empty messages
      crypto: qce - Reject empty messages for AES-XTS
      crypto: qce - Use a fallback for AES-CTR with a partial final block
      crypto: qce - Use fallback for fragmented skcipher payloads
      crypto: qce - Use a fallback for CCM with a partial final block
      crypto: qce - Use fallback for CCM with a fragmented payload
      crypto: qce - remove the BROKEN label
      crypto: qce - convert to auxiliary bus
      soc: qcom: add core driver for the Qualcomm Crypto Engine
      arm64: defconfig: enable the Qualcomm Crypto Engine core driver

Kuldeep Singh (2):
      crypto: qce - Fix CTR-AES for partial block requests
      crypto: qce - Fix xts-aes-qce for weak keys

 arch/arm64/configs/defconfig  |  1 +
 drivers/crypto/Kconfig        |  4 +-
 drivers/crypto/qce/aead.c     | 44 ++++++++++++++++----
 drivers/crypto/qce/cipher.h   |  1 +
 drivers/crypto/qce/common.h   |  1 -
 drivers/crypto/qce/core.c     | 93 +++++++++++++++++++------------------------
 drivers/crypto/qce/core.h     |  5 ++-
 drivers/crypto/qce/sha.c      | 70 ++++++++++++++++++++++----------
 drivers/crypto/qce/skcipher.c | 62 ++++++++++++++++++++++-------
 drivers/soc/qcom/Kconfig      | 11 +++++
 drivers/soc/qcom/Makefile     |  1 +
 drivers/soc/qcom/qce-core.c   | 87 ++++++++++++++++++++++++++++++++++++++++
 12 files changed, 283 insertions(+), 97 deletions(-)
---
base-commit: c0b0cbecdd2bdd74c9e76409e6ae67b3e65d737a
change-id: 20260610-qce-fix-self-tests-492ffd2ef955

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Re: [PATCH v7 00/12] crypto: qce - Fix crypto self-test failures
Posted by Herbert Xu 1 week ago
On Thu, Sep 10, 2026 at 03:00:41PM +0200, Bartosz Golaszewski wrote:
> This iteration - in addition to the previous fixes - proposes to split
> the QCE driver into a core part necessary to bind to the QCE DT node and
> enable runtime power management in order to allow to drop the
> interconnect votes, and the crypto part registering the crypto
> algorithms. The core module is then enabled in arm64 defconfig while the
> crypto part stays disabled by default.
> 
> The QCE hardware crypto engine has several limitations that cause it to
> produce incorrect results or stall on certain inputs. This series fixes
> several bugs and adds workaround allowing the deiver to pass crypto
> self-tests.
> 
> The failures addressed are:
> 
> - HMAC self-test failures for empty messages
> - AES-XTS returning success on zero-length input (should be -EINVAL)
> - AES-CTR: partial final block causes the engine to stall, output IV
>   derivation was incorrect
> - AES-XTS with key1 == key2 is not supported by the CE
> - AES-CCM: partial final block and fragmented payload both stall the
>   engine
> 
> All fixes were tested on an SM8650 QRD board with
> CONFIG_CRYPTO_SELFTESTS=y and CONFIG_CRYPTO_SELFTESTS_FULL=y.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> ---
> Changes in v7:
> - Add follow-up changes converting the QCE crypto driver to auxiliary
>   bus, adding the core QCE driver under drivers/soc/ registering the
>   auxiliary device and removing the BROKEN Kconfig label
> - Link to v6: https://patch.msgid.link/20260717-qce-fix-self-tests-v6-0-455775fe5f6c@oss.qualcomm.com

For some reason your patches do not apply, please fix this and
resubmit:

https://sashiko.dev/#/patchset/20260910-qce-fix-self-tests-v7-0-cdbd2718af14%40oss.qualcomm.com

Thanks,
-- 
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
Re: [PATCH v7 00/12] crypto: qce - Fix crypto self-test failures
Posted by Bartosz Golaszewski 4 days, 4 hours ago
On Fri, 18 Sep 2026 11:03:21 +0200, Herbert Xu
<herbert@gondor.apana.org.au> said:
> On Thu, Sep 10, 2026 at 03:00:41PM +0200, Bartosz Golaszewski wrote:
>> This iteration - in addition to the previous fixes - proposes to split
>> the QCE driver into a core part necessary to bind to the QCE DT node and
>> enable runtime power management in order to allow to drop the
>> interconnect votes, and the crypto part registering the crypto
>> algorithms. The core module is then enabled in arm64 defconfig while the
>> crypto part stays disabled by default.
>>
>> The QCE hardware crypto engine has several limitations that cause it to
>> produce incorrect results or stall on certain inputs. This series fixes
>> several bugs and adds workaround allowing the deiver to pass crypto
>> self-tests.
>>
>> The failures addressed are:
>>
>> - HMAC self-test failures for empty messages
>> - AES-XTS returning success on zero-length input (should be -EINVAL)
>> - AES-CTR: partial final block causes the engine to stall, output IV
>>   derivation was incorrect
>> - AES-XTS with key1 == key2 is not supported by the CE
>> - AES-CCM: partial final block and fragmented payload both stall the
>>   engine
>>
>> All fixes were tested on an SM8650 QRD board with
>> CONFIG_CRYPTO_SELFTESTS=y and CONFIG_CRYPTO_SELFTESTS_FULL=y.
>>
>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
>> ---
>> Changes in v7:
>> - Add follow-up changes converting the QCE crypto driver to auxiliary
>>   bus, adding the core QCE driver under drivers/soc/ registering the
>>   auxiliary device and removing the BROKEN Kconfig label
>> - Link to v6: https://patch.msgid.link/20260717-qce-fix-self-tests-v6-0-455775fe5f6c@oss.qualcomm.com
>
> For some reason your patches do not apply, please fix this and
> resubmit:
>
> https://sashiko.dev/#/patchset/20260910-qce-fix-self-tests-v7-0-cdbd2718af14%40oss.qualcomm.com
>

Weird, I did base the series on current linux-next. I'll do the same for v8.

Bart
Re: [PATCH v7 00/12] crypto: qce - Fix crypto self-test failures
Posted by Demi Marie Obenour 1 week, 6 days ago
On 9/10/26 09:00, Bartosz Golaszewski wrote:
> This iteration - in addition to the previous fixes - proposes to split
> the QCE driver into a core part necessary to bind to the QCE DT node and
> enable runtime power management in order to allow to drop the
> interconnect votes, and the crypto part registering the crypto
> algorithms. The core module is then enabled in arm64 defconfig while the
> crypto part stays disabled by default.
> 
> The QCE hardware crypto engine has several limitations that cause it to
> produce incorrect results or stall on certain inputs. This series fixes
> several bugs and adds workaround allowing the deiver to pass crypto
> self-tests.
> 
> The failures addressed are:
> 
> - HMAC self-test failures for empty messages
> - AES-XTS returning success on zero-length input (should be -EINVAL)
> - AES-CTR: partial final block causes the engine to stall, output IV
>   derivation was incorrect
> - AES-XTS with key1 == key2 is not supported by the CE
> - AES-CCM: partial final block and fragmented payload both stall the
>   engine
> 
> All fixes were tested on an SM8650 QRD board with
> CONFIG_CRYPTO_SELFTESTS=y and CONFIG_CRYPTO_SELFTESTS_FULL=y.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
For stable, would it be better to not register the algorithms, thus
preventing their use?  Another option would be to replace the whole
driver with an always-on stub driver that just does power management.
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)
Re: [PATCH v7 00/12] crypto: qce - Fix crypto self-test failures
Posted by Bartosz Golaszewski 1 week, 3 days ago
On Fri, 11 Sep 2026 20:34:39 +0200, Demi Marie Obenour
<demiobenour@gmail.com> said:
> On 9/10/26 09:00, Bartosz Golaszewski wrote:
>> This iteration - in addition to the previous fixes - proposes to split
>> the QCE driver into a core part necessary to bind to the QCE DT node and
>> enable runtime power management in order to allow to drop the
>> interconnect votes, and the crypto part registering the crypto
>> algorithms. The core module is then enabled in arm64 defconfig while the
>> crypto part stays disabled by default.
>>
>> The QCE hardware crypto engine has several limitations that cause it to
>> produce incorrect results or stall on certain inputs. This series fixes
>> several bugs and adds workaround allowing the deiver to pass crypto
>> self-tests.
>>
>> The failures addressed are:
>>
>> - HMAC self-test failures for empty messages
>> - AES-XTS returning success on zero-length input (should be -EINVAL)
>> - AES-CTR: partial final block causes the engine to stall, output IV
>>   derivation was incorrect
>> - AES-XTS with key1 == key2 is not supported by the CE
>> - AES-CCM: partial final block and fragmented payload both stall the
>>   engine
>>
>> All fixes were tested on an SM8650 QRD board with
>> CONFIG_CRYPTO_SELFTESTS=y and CONFIG_CRYPTO_SELFTESTS_FULL=y.
>>
>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> For stable, would it be better to not register the algorithms, thus
> preventing their use?  Another option would be to replace the whole
> driver with an always-on stub driver that just does power management.

A module parameter called: really_register_algos that's disabled by default?

Bart
Re: [PATCH v7 00/12] crypto: qce - Fix crypto self-test failures
Posted by Demi Marie Obenour 1 week, 1 day ago
On 9/15/26 05:42, Bartosz Golaszewski wrote:
> On Fri, 11 Sep 2026 20:34:39 +0200, Demi Marie Obenour
> <demiobenour@gmail.com> said:
>> On 9/10/26 09:00, Bartosz Golaszewski wrote:
>>> This iteration - in addition to the previous fixes - proposes to split
>>> the QCE driver into a core part necessary to bind to the QCE DT node and
>>> enable runtime power management in order to allow to drop the
>>> interconnect votes, and the crypto part registering the crypto
>>> algorithms. The core module is then enabled in arm64 defconfig while the
>>> crypto part stays disabled by default.
>>>
>>> The QCE hardware crypto engine has several limitations that cause it to
>>> produce incorrect results or stall on certain inputs. This series fixes
>>> several bugs and adds workaround allowing the deiver to pass crypto
>>> self-tests.
>>>
>>> The failures addressed are:
>>>
>>> - HMAC self-test failures for empty messages
>>> - AES-XTS returning success on zero-length input (should be -EINVAL)
>>> - AES-CTR: partial final block causes the engine to stall, output IV
>>>   derivation was incorrect
>>> - AES-XTS with key1 == key2 is not supported by the CE
>>> - AES-CCM: partial final block and fragmented payload both stall the
>>>   engine
>>>
>>> All fixes were tested on an SM8650 QRD board with
>>> CONFIG_CRYPTO_SELFTESTS=y and CONFIG_CRYPTO_SELFTESTS_FULL=y.
>>>
>>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
>> For stable, would it be better to not register the algorithms, thus
>> preventing their use?  Another option would be to replace the whole
>> driver with an always-on stub driver that just does power management.
> 
> A module parameter called: really_register_algos that's disabled by default?
> 
> Bart

Works for me!  If you add that and fix the tests, go ahead and revert
my patch that marked it as BROKEN.  I suggest reverting the defconfig
changes too, for power management reasons.

While the driver is buggy, you might want to add a kernel taint if
the option is set.  There's precedent for that in other parts of the
kernel.
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)
Re: [PATCH v7 00/12] crypto: qce - Fix crypto self-test failures
Posted by Bartosz Golaszewski 4 days, 4 hours ago
On Wed, 16 Sep 2026 18:45:40 +0200, Demi Marie Obenour
<demiobenour@gmail.com> said:
> On 9/15/26 05:42, Bartosz Golaszewski wrote:
>> On Fri, 11 Sep 2026 20:34:39 +0200, Demi Marie Obenour
>> <demiobenour@gmail.com> said:
>>> On 9/10/26 09:00, Bartosz Golaszewski wrote:
>>>> This iteration - in addition to the previous fixes - proposes to split
>>>> the QCE driver into a core part necessary to bind to the QCE DT node and
>>>> enable runtime power management in order to allow to drop the
>>>> interconnect votes, and the crypto part registering the crypto
>>>> algorithms. The core module is then enabled in arm64 defconfig while the
>>>> crypto part stays disabled by default.
>>>>
>>>> The QCE hardware crypto engine has several limitations that cause it to
>>>> produce incorrect results or stall on certain inputs. This series fixes
>>>> several bugs and adds workaround allowing the deiver to pass crypto
>>>> self-tests.
>>>>
>>>> The failures addressed are:
>>>>
>>>> - HMAC self-test failures for empty messages
>>>> - AES-XTS returning success on zero-length input (should be -EINVAL)
>>>> - AES-CTR: partial final block causes the engine to stall, output IV
>>>>   derivation was incorrect
>>>> - AES-XTS with key1 == key2 is not supported by the CE
>>>> - AES-CCM: partial final block and fragmented payload both stall the
>>>>   engine
>>>>
>>>> All fixes were tested on an SM8650 QRD board with
>>>> CONFIG_CRYPTO_SELFTESTS=y and CONFIG_CRYPTO_SELFTESTS_FULL=y.
>>>>
>>>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
>>> For stable, would it be better to not register the algorithms, thus
>>> preventing their use?  Another option would be to replace the whole
>>> driver with an always-on stub driver that just does power management.
>>
>> A module parameter called: really_register_algos that's disabled by default?
>>
>> Bart
>
> Works for me!  If you add that and fix the tests, go ahead and revert
> my patch that marked it as BROKEN.  I suggest reverting the defconfig
> changes too, for power management reasons.
>
> While the driver is buggy, you might want to add a kernel taint if
> the option is set.  There's precedent for that in other parts of the
> kernel.

I'd prefer not to. I hope gating the algos behind a module parameter is enough.

Bart