* [PATCH] crypto: qat - fix restarting state leak on allocation failure
From: Ahsan Atta @ 2026-05-20 12:33 UTC (permalink / raw)
To: herbert
Cc: linux-crypto, qat-linux, Ahsan Atta, stable, Maksim Lukoshkov,
Giovanni Cabiddu
In adf_dev_aer_schedule_reset(), ADF_STATUS_RESTARTING is set before
allocating reset_data. If the allocation fails, the function returns
-ENOMEM without queuing reset work, so nothing ever clears the bit.
This leaves the device permanently stuck in the restarting state,
causing all subsequent reset attempts to be silently skipped.
Fix this by using test_and_set_bit() to atomically claim the
RESTARTING state, preventing duplicate reset scheduling races under
concurrent fatal error reporting. If the subsequent allocation fails,
clear the bit to restore clean state so future reset attempts can
proceed.
Cc: stable@vger.kernel.org
Fixes: d8cba25d2c68 ("crypto: qat - Intel(R) QAT driver framework")
Signed-off-by: Ahsan Atta <ahsan.atta@intel.com>
Co-developed-by: Maksim Lukoshkov <maksim.lukoshkov@intel.com>
Signed-off-by: Maksim Lukoshkov <maksim.lukoshkov@intel.com>
Reviewed-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
---
drivers/crypto/intel/qat/qat_common/adf_aer.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/crypto/intel/qat/qat_common/adf_aer.c b/drivers/crypto/intel/qat/qat_common/adf_aer.c
index af028488e660..3fc7d13e882c 100644
--- a/drivers/crypto/intel/qat/qat_common/adf_aer.c
+++ b/drivers/crypto/intel/qat/qat_common/adf_aer.c
@@ -207,13 +207,14 @@ static int adf_dev_aer_schedule_reset(struct adf_accel_dev *accel_dev,
struct adf_reset_dev_data *reset_data;
if (!adf_dev_started(accel_dev) ||
- test_bit(ADF_STATUS_RESTARTING, &accel_dev->status))
+ test_and_set_bit(ADF_STATUS_RESTARTING, &accel_dev->status))
return 0;
- set_bit(ADF_STATUS_RESTARTING, &accel_dev->status);
reset_data = kzalloc_obj(*reset_data);
- if (!reset_data)
+ if (!reset_data) {
+ clear_bit(ADF_STATUS_RESTARTING, &accel_dev->status);
return -ENOMEM;
+ }
reset_data->accel_dev = accel_dev;
init_completion(&reset_data->compl);
reset_data->mode = mode;
--
2.50.1
--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263
This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.
^ permalink raw reply related
* [PATCH] crypto: octeontx - use strscpy_pad in ucode_load_store
From: Thorsten Blum @ 2026-05-20 10:00 UTC (permalink / raw)
To: Srujana Challa, Bharat Bhushan, Herbert Xu, David S. Miller,
Thorsten Blum, Kees Cook
Cc: linux-crypto, linux-kernel
Instead of zero-initializing the temporary buffer and then copying into
it with strscpy(), use strscpy_pad() to copy the string and zero-pad any
trailing bytes. Drop the explicit size argument to further simplify the
code since strscpy_pad() can determine it automatically when the
destination buffer has a fixed length.
Also use strscpy_pad() to check for string truncation instead of the
hard-coded OTX_CPT_UCODE_NAME_LENGTH.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
drivers/crypto/marvell/octeontx/otx_cptpf_ucode.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/crypto/marvell/octeontx/otx_cptpf_ucode.c b/drivers/crypto/marvell/octeontx/otx_cptpf_ucode.c
index e0f38d32bc93..205579a6ba2b 100644
--- a/drivers/crypto/marvell/octeontx/otx_cptpf_ucode.c
+++ b/drivers/crypto/marvell/octeontx/otx_cptpf_ucode.c
@@ -1318,7 +1318,7 @@ static ssize_t ucode_load_store(struct device *dev,
{
struct otx_cpt_engines engs[OTX_CPT_MAX_ETYPES_PER_GRP] = { {0} };
char *ucode_filename[OTX_CPT_MAX_ETYPES_PER_GRP];
- char tmp_buf[OTX_CPT_UCODE_NAME_LENGTH] = { 0 };
+ char tmp_buf[OTX_CPT_UCODE_NAME_LENGTH];
char *start, *val, *err_msg, *tmp;
struct otx_cpt_eng_grps *eng_grps;
int grp_idx = 0, ret = -EINVAL;
@@ -1326,12 +1326,11 @@ static ssize_t ucode_load_store(struct device *dev,
int del_grp_idx = -1;
int ucode_idx = 0;
- if (count >= OTX_CPT_UCODE_NAME_LENGTH)
+ if (strscpy_pad(tmp_buf, buf) < 0)
return -EINVAL;
eng_grps = container_of(attr, struct otx_cpt_eng_grps, ucode_load_attr);
err_msg = "Invalid engine group format";
- strscpy(tmp_buf, buf, OTX_CPT_UCODE_NAME_LENGTH);
start = tmp_buf;
has_se = has_ie = has_ae = false;
^ permalink raw reply related
* Re: [PATCH] crypto: hisilicon/sec2 - lower priority for hisilicon crypto implementations
From: huangchenghai @ 2026-05-20 9:27 UTC (permalink / raw)
To: Eric Biggers, liulongfang
Cc: herbert, davem, linux-kernel, linux-crypto, fanghao11, qianweili,
wangzhou1
In-Reply-To: <20260520013240.GC1875993@google.com>
在 2026/5/20 9:32, Eric Biggers 写道:
> On Wed, May 20, 2026 at 09:22:49AM +0800, liulongfang wrote:
>> On 2026/5/11 8:49, Chenghai Huang wrote:
>>> From: lizhi <lizhi206@huawei.com>
>>>
>>> Lower the priority of HiSilicon's crypto implementations to allow more
>>> suitable alternatives to be selected. For example, certain kernel
>>> use-cases do not benefit from HiSilicon's symmetric crypto algorithms.
>>> This change ensures that more appropriate options are chosen first while
>>> retaining HiSilicon's implementations as alternatives.
>>>
>>> Signed-off-by: lizhi <lizhi206@huawei.com>
>>> Signed-off-by: Chenghai Huang <huangchenghai2@huawei.com>
>>> ---
>>> drivers/crypto/hisilicon/sec2/sec_crypto.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/crypto/hisilicon/sec2/sec_crypto.c b/drivers/crypto/hisilicon/sec2/sec_crypto.c
>>> index 2471a4dd0b50..77e0e03cbcab 100644
>>> --- a/drivers/crypto/hisilicon/sec2/sec_crypto.c
>>> +++ b/drivers/crypto/hisilicon/sec2/sec_crypto.c
>>> @@ -20,7 +20,7 @@
>>> #include "sec.h"
>>> #include "sec_crypto.h"
>>>
>>> -#define SEC_PRIORITY 4001
>>> +#define SEC_PRIORITY 80
>>> #define SEC_XTS_MIN_KEY_SIZE (2 * AES_MIN_KEY_SIZE)
>>> #define SEC_XTS_MID_KEY_SIZE (3 * AES_MIN_KEY_SIZE)
>>> #define SEC_XTS_MAX_KEY_SIZE (2 * AES_MAX_KEY_SIZE)
>>>
>> Reviewed-by: Longfang Liu <liulongfang@huawei.com>
> Makes sense, but perhaps this driver should just be removed entirely?
>
> - Eric
>
Hi Eric,
Thanks for the review.
We still have use cases to keep the driver.
1.Lowering the priority frees up hardware acceleration resources for
targeted commercial use cases like storage encryption.
2.On old version of Kunpeng storage server, crypto instruction
extensions may not be available, or the supported algorithm sets are
limited. In these environments, users still use the HiSilicon hardware
accelerator for encryption. Completely removing the driver would break
support for those deployments.
3.Make the driver an optional backup, like QAT.
Best regards,
Chenghai
^ permalink raw reply
* Re: [PATCH 01/19] btrfs: require at least 4 devices for RAID 6
From: Qu Wenruo @ 2026-05-20 8:41 UTC (permalink / raw)
To: Christoph Hellwig, H. Peter Anvin
Cc: kreijack, David Sterba, Andrew Morton, Catalin Marinas,
Will Deacon, Ard Biesheuvel, Huacai Chen, WANG Xuerui,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy (CS GROUP), Paul Walmsley, Palmer Dabbelt,
Albert Ou, Alexandre Ghiti, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Herbert Xu, Dan Williams, Chris Mason, David Sterba,
Arnd Bergmann, Song Liu, Yu Kuai, Li Nan, linux-kernel,
linux-arm-kernel, loongarch, linuxppc-dev, linux-riscv,
linux-s390, linux-crypto, linux-btrfs, linux-arch, linux-raid
In-Reply-To: <20260518051207.GB9374@lst.de>
在 2026/5/18 14:42, Christoph Hellwig 写道:
> On Fri, May 15, 2026 at 12:59:34PM -0700, H. Peter Anvin wrote:
>> I don't think this is a good idea. Error out; it is the btrfs maintainers' job to ensure user data isn't lost.
>>
>> The RAID-6 code has *never* supported only 3 units, and if it ever worked for *any* of the implementations it was purely by accident. Speaking as the original author I should know; this was deliberate as in some cases the degenerate case (3) would have required extra trays in the code to no user benefit.
>>
>> I would not be surprised if the kernel crashed or corrupted the page cache in that case.
>
> It does, that's why I wanted to exclude it. Anyway, for the about to be
> resent version I'll drop this btrfs patch over the stated objection and
> will otherwise not change anything. This means the (IMHO hypothetical)
> users of this configuration will get a WARN_ON_ONCE triggered, but
> otherwise keep working (or rather not working) as before.
>
For the btrfs part, I believe I can get the current 2-disk-raid5 and
3-disk-raid6 to fallback to raid1 inside btrfs.
I hope the btrfs part can be finished and reach the next merge window,
but I'm not 100% sure.
What is the planned cycle to merge this raid5/6 cleanup?
Thanks,
Qu
^ permalink raw reply
* Re: [PATCH v2 1/3] crypto: atmel-sha204a - Drop of_device_id data
From: Ard Biesheuvel @ 2026-05-20 7:49 UTC (permalink / raw)
To: Uwe Kleine-König (The Capable Hub), Thorsten Blum,
Herbert Xu, David S. Miller, Nicolas Ferre, Alexandre Belloni,
Claudiu Beznea
Cc: linux-crypto, linux-arm-kernel, linux-kernel
In-Reply-To: <d0fc3069860f9e31122c1af635a1114dd2c443cf.1779260113.git.u.kleine-koenig@baylibre.com>
On Wed, 20 May 2026, at 09:01, Uwe Kleine-König (The Capable Hub) wrote:
> The driver binds to i2c devices only and thus in the absence of an
> assignment for .data in the of_device_id array i2c_get_match_data()
> falls back to .driver_data from the i2c_device_id array. So only provide
> &atsha204_quality once to reduce duplication.
>
> Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
> ---
> drivers/crypto/atmel-sha204a.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/crypto/atmel-sha204a.c b/drivers/crypto/atmel-sha204a.c
> index 6e6ac4770416..f17e1f6af1a3 100644
> --- a/drivers/crypto/atmel-sha204a.c
> +++ b/drivers/crypto/atmel-sha204a.c
> @@ -208,8 +208,8 @@ static void atmel_sha204a_remove(struct i2c_client *client)
> }
>
> static const struct of_device_id atmel_sha204a_dt_ids[] = {
> - { .compatible = "atmel,atsha204", .data = &atsha204_quality },
> - { .compatible = "atmel,atsha204a", },
> + { .compatible = "atmel,atsha204" },
> + { .compatible = "atmel,atsha204a" },
> { }
> };
> MODULE_DEVICE_TABLE(of, atmel_sha204a_dt_ids);
Just trying to figure out how this is supposed to work:
i2c_get_match_data()
data = device_get_match_data(&client->dev);
... returns NULL ...
if (!data) {
match = i2c_match_id(driver->id_table, client);
... compares client->name with { "atsha204", "atsha204a" }
So we will be relying on client->name having been set to either
"atsha204" or "atsha204a" on the DT probe path before
i2c_match_data() is called, but I am struggling to see where
that might happen.
^ permalink raw reply
* Re: [PATCH] s390: crypto: add select CRYPTO_AEAD for aes
From: Harald Freudenberger @ 2026-05-20 7:47 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Herbert Xu, David S. Miller, Holger Dengler, Heiko Carstens,
Vasily Gorbik, Alexander Gordeev, Patrick Steuer, Arnd Bergmann,
Christian Borntraeger, Sven Schnelle, Eric Biggers,
Ard Biesheuvel, linux-crypto, linux-s390, linux-kernel
In-Reply-To: <20260520073911.843561-1-arnd@kernel.org>
On 2026-05-20 09:38, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> The aes driver registers both skcipher and aead algorithms,
> but when aead is not enabled this causes a link failure:
>
> s390-linux-ld: arch/s390/crypto/aes_s390.o: in function
> `aes_s390_fini':
> arch/s390/crypto/aes_s390.c:969:(.text+0x115e): undefined reference to
> `crypto_unregister_aead'
> s390-linux-ld: arch/s390/crypto/aes_s390.o: in function
> `aes_s390_init':
> arch/s390/crypto/aes_s390.c:1028:(.init.text+0x294): undefined
> reference to `crypto_register_aead'
>
> Add the missing 'select' statement.
>
> Fixes: bf7fa038707c ("s390/crypto: add s390 platform specific aes gcm
> support.")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> arch/s390/crypto/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/s390/crypto/Kconfig b/arch/s390/crypto/Kconfig
> index 00051d27db95..228570a1b233 100644
> --- a/arch/s390/crypto/Kconfig
> +++ b/arch/s390/crypto/Kconfig
> @@ -4,6 +4,7 @@ menu "Accelerated Cryptographic Algorithms for CPU
> (s390)"
>
> config CRYPTO_AES_S390
> tristate "Ciphers: AES, modes: ECB, CBC, CTR, XTS, GCM"
> + select CRYPTO_AEAD
> select CRYPTO_SKCIPHER
> help
> AEAD cipher: AES with GCM
Reviewed-by: Harald Freudenberger <freude@linux.ibm.com>
^ permalink raw reply
* [PATCH] s390: crypto: add select CRYPTO_AEAD for aes
From: Arnd Bergmann @ 2026-05-20 7:38 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, Harald Freudenberger, Holger Dengler,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Patrick Steuer
Cc: Arnd Bergmann, Christian Borntraeger, Sven Schnelle, Eric Biggers,
Ard Biesheuvel, linux-crypto, linux-s390, linux-kernel
From: Arnd Bergmann <arnd@arndb.de>
The aes driver registers both skcipher and aead algorithms,
but when aead is not enabled this causes a link failure:
s390-linux-ld: arch/s390/crypto/aes_s390.o: in function `aes_s390_fini':
arch/s390/crypto/aes_s390.c:969:(.text+0x115e): undefined reference to `crypto_unregister_aead'
s390-linux-ld: arch/s390/crypto/aes_s390.o: in function `aes_s390_init':
arch/s390/crypto/aes_s390.c:1028:(.init.text+0x294): undefined reference to `crypto_register_aead'
Add the missing 'select' statement.
Fixes: bf7fa038707c ("s390/crypto: add s390 platform specific aes gcm support.")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
arch/s390/crypto/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/s390/crypto/Kconfig b/arch/s390/crypto/Kconfig
index 00051d27db95..228570a1b233 100644
--- a/arch/s390/crypto/Kconfig
+++ b/arch/s390/crypto/Kconfig
@@ -4,6 +4,7 @@ menu "Accelerated Cryptographic Algorithms for CPU (s390)"
config CRYPTO_AES_S390
tristate "Ciphers: AES, modes: ECB, CBC, CTR, XTS, GCM"
+ select CRYPTO_AEAD
select CRYPTO_SKCIPHER
help
AEAD cipher: AES with GCM
--
2.39.5
^ permalink raw reply related
* [PATCH v2 3/3] crypto: atmel-ecc - Use named initializers for struct i2c_device_id
From: Uwe Kleine-König (The Capable Hub) @ 2026-05-20 7:01 UTC (permalink / raw)
To: Thorsten Blum, Herbert Xu, David S. Miller, Nicolas Ferre,
Alexandre Belloni, Claudiu Beznea
Cc: Ard Biesheuvel, linux-crypto, linux-arm-kernel, linux-kernel
In-Reply-To: <cover.1779260113.git.u.kleine-koenig@baylibre.com>
While being less compact, using named initializers allows to more easily
see which members of the structs are assigned which value without having
to lookup the declaration of the struct. And it's also more robust
against changes to the struct definition.
This patch doesn't modify the compiled array, only its representation in
source form benefits. The former was confirmed with x86 and arm64
builds.
Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
---
drivers/crypto/atmel-ecc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index 9660f6426a84..0ca02995a1de 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -376,8 +376,8 @@ static const struct of_device_id atmel_ecc_dt_ids[] = {
MODULE_DEVICE_TABLE(of, atmel_ecc_dt_ids);
static const struct i2c_device_id atmel_ecc_id[] = {
- { "atecc508a" },
- { "atecc608b" },
+ { .name = "atecc508a" },
+ { .name = "atecc608b" },
{ }
};
MODULE_DEVICE_TABLE(i2c, atmel_ecc_id);
--
2.47.3
^ permalink raw reply related
* [PATCH v2 2/3] crypto: atmel-sha204a - Use named initializers for struct i2c_device_id
From: Uwe Kleine-König (The Capable Hub) @ 2026-05-20 7:01 UTC (permalink / raw)
To: Thorsten Blum, Herbert Xu, David S. Miller, Nicolas Ferre,
Alexandre Belloni, Claudiu Beznea
Cc: Ard Biesheuvel, linux-crypto, linux-arm-kernel, linux-kernel
In-Reply-To: <cover.1779260113.git.u.kleine-koenig@baylibre.com>
While being less compact, using named initializers allows to more easily
see which members of the structs are assigned which value without having
to lookup the declaration of the struct. And it's also more robust
against changes to the struct definition.
This patch doesn't modify the compiled array, only its representation in
source form benefits. The former was confirmed with x86 and arm64
builds.
For consistency also assign .driver_data for the array item that the
driver relies on i2c_get_match_data() returning NULL for.
Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
---
drivers/crypto/atmel-sha204a.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/crypto/atmel-sha204a.c b/drivers/crypto/atmel-sha204a.c
index f17e1f6af1a3..f3bbe836778d 100644
--- a/drivers/crypto/atmel-sha204a.c
+++ b/drivers/crypto/atmel-sha204a.c
@@ -215,8 +215,8 @@ static const struct of_device_id atmel_sha204a_dt_ids[] = {
MODULE_DEVICE_TABLE(of, atmel_sha204a_dt_ids);
static const struct i2c_device_id atmel_sha204a_id[] = {
- { "atsha204", (kernel_ulong_t)&atsha204_quality },
- { "atsha204a" },
+ { .name = "atsha204", .driver_data = (kernel_ulong_t)&atsha204_quality },
+ { .name = "atsha204a", .driver_data = (kernel_ulong_t)NULL },
{ }
};
MODULE_DEVICE_TABLE(i2c, atmel_sha204a_id);
--
2.47.3
^ permalink raw reply related
* [PATCH v2 1/3] crypto: atmel-sha204a - Drop of_device_id data
From: Uwe Kleine-König (The Capable Hub) @ 2026-05-20 7:01 UTC (permalink / raw)
To: Thorsten Blum, Herbert Xu, David S. Miller, Nicolas Ferre,
Alexandre Belloni, Claudiu Beznea
Cc: Ard Biesheuvel, linux-crypto, linux-arm-kernel, linux-kernel
In-Reply-To: <cover.1779260113.git.u.kleine-koenig@baylibre.com>
The driver binds to i2c devices only and thus in the absence of an
assignment for .data in the of_device_id array i2c_get_match_data()
falls back to .driver_data from the i2c_device_id array. So only provide
&atsha204_quality once to reduce duplication.
Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
---
drivers/crypto/atmel-sha204a.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/crypto/atmel-sha204a.c b/drivers/crypto/atmel-sha204a.c
index 6e6ac4770416..f17e1f6af1a3 100644
--- a/drivers/crypto/atmel-sha204a.c
+++ b/drivers/crypto/atmel-sha204a.c
@@ -208,8 +208,8 @@ static void atmel_sha204a_remove(struct i2c_client *client)
}
static const struct of_device_id atmel_sha204a_dt_ids[] = {
- { .compatible = "atmel,atsha204", .data = &atsha204_quality },
- { .compatible = "atmel,atsha204a", },
+ { .compatible = "atmel,atsha204" },
+ { .compatible = "atmel,atsha204a" },
{ }
};
MODULE_DEVICE_TABLE(of, atmel_sha204a_dt_ids);
--
2.47.3
^ permalink raw reply related
* [PATCH v2 0/3] crypto - Rework i2c_device_id initialisation
From: Uwe Kleine-König (The Capable Hub) @ 2026-05-20 7:01 UTC (permalink / raw)
To: Thorsten Blum, Herbert Xu, David S. Miller, Nicolas Ferre,
Alexandre Belloni, Claudiu Beznea
Cc: Ard Biesheuvel, linux-crypto, linux-arm-kernel, linux-kernel
Hello,
this is v2 of the patch available at
https://lore.kernel.org/linux-crypto/20260519141033.1586036-2-u.kleine-koenig@baylibre.com.
Changes since v1 are:
- Rebase to next-20260519 to account for changes since v7.1-rc1 (= the
previous base)
- Patch #1 is new
- The adaption to atmel-sha204a is a bit less trivial, so split into a
separate patch (#2)
Best regards
Uwe
Uwe Kleine-König (The Capable Hub) (3):
crypto: atmel-sha204a - Drop of_device_id data
crypto: atmel-sha204a - Use named initializers for struct
i2c_device_id
crypto: atmel-ecc - Use named initializers for struct i2c_device_id
drivers/crypto/atmel-ecc.c | 4 ++--
drivers/crypto/atmel-sha204a.c | 8 ++++----
2 files changed, 6 insertions(+), 6 deletions(-)
base-commit: 6a50ba100ace43f43c87384367eb2d2605fcc16c
--
2.47.3
^ permalink raw reply
* Re: [PATCH v1] crypto: Use named initializers for struct i2c_device_id
From: Ard Biesheuvel @ 2026-05-20 6:43 UTC (permalink / raw)
To: Uwe Kleine-König (The Capable Hub), Thorsten Blum,
Herbert Xu, David S. Miller, Nicolas Ferre, Alexandre Belloni,
Claudiu Beznea
Cc: linux-crypto, linux-arm-kernel, linux-kernel
In-Reply-To: <20260519141033.1586036-2-u.kleine-koenig@baylibre.com>
On Tue, 19 May 2026, at 16:10, Uwe Kleine-König (The Capable Hub) wrote:
> While being less compact, using named initializers allows to more easily
> see which members of the structs are assigned which value without having
> to lookup the declaration of the struct. And it's also more robust
> against changes to the struct definition.
>
> This patch doesn't modify the compiled arrays, only their representation
> in source form benefits. The former was confirmed with x86 and arm64
> builds.
>
> Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
> ---
> Hello,
>
> this patch is part of a bigger quest to use named initializers for
> mainly struct i2c_device_id::driver_data to be able to modify
> i2c_device_id. See e.g.
> https://lore.kernel.org/all/20260518111203.639603-2-u.kleine-koenig@baylibre.com/
> for the details.
>
> This patch here isn't critical for this quest, as no driver makes use of
> .driver_data, so apart from the better readability this is only about
> consistency with other subsystems.
>
> Best regards
> Uwe
>
> drivers/crypto/atmel-ecc.c | 2 +-
> drivers/crypto/atmel-sha204a.c | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
Acked-by: Ard Biesheuvel <ardb@kernel.org>
> diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
> index 9c380351d2f9..56350454ac29 100644
> --- a/drivers/crypto/atmel-ecc.c
> +++ b/drivers/crypto/atmel-ecc.c
> @@ -380,7 +380,7 @@ MODULE_DEVICE_TABLE(of, atmel_ecc_dt_ids);
> #endif
>
> static const struct i2c_device_id atmel_ecc_id[] = {
> - { "atecc508a" },
> + { .name = "atecc508a" },
> { }
> };
> MODULE_DEVICE_TABLE(i2c, atmel_ecc_id);
> diff --git a/drivers/crypto/atmel-sha204a.c
> b/drivers/crypto/atmel-sha204a.c
> index dbb39ed0cea1..0fcb4692494f 100644
> --- a/drivers/crypto/atmel-sha204a.c
> +++ b/drivers/crypto/atmel-sha204a.c
> @@ -210,8 +210,8 @@ static const struct of_device_id
> atmel_sha204a_dt_ids[] __maybe_unused = {
> MODULE_DEVICE_TABLE(of, atmel_sha204a_dt_ids);
>
> static const struct i2c_device_id atmel_sha204a_id[] = {
> - { "atsha204" },
> - { "atsha204a" },
> + { .name = "atsha204" },
> + { .name = "atsha204a" },
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(i2c, atmel_sha204a_id);
>
> base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
> --
> 2.47.3
^ permalink raw reply
* [PATCH] crypto: tegra - Return ENOMEM when input buffer allocation fails for ccm
From: Herbert Xu @ 2026-05-20 2:51 UTC (permalink / raw)
To: Vladislav Dronov; +Cc: Linux Crypto Mailing List, Akhil R, Patrick Talbert
In-Reply-To: <CAMusb+T_q1Vu3MD+VjiPWfpe3NNpjJehXWQ9gePo=hM+NGm1zg@mail.gmail.com>
On Tue, May 19, 2026 at 01:42:59PM +0200, Vladislav Dronov wrote:
>
> sashiko.dev makes a point here (
> https://sashiko.dev/#/patchset/agvleqNqloWB6tpf%40gondor.apana.org.au )
> that the code does not set ret to an error value, as done in the other
> similar places, see:
Thanks Vladis. This is an existing bug and I'll fix it with a
separate patch:
---8<---
Ensure the ENOMEM error value is set when the input buffer allocation
fails in tegra_ccm_do_one_req.
Reported-by: Vladislav Dronov <vdronov@redhat.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/drivers/crypto/tegra/tegra-se-aes.c b/drivers/crypto/tegra/tegra-se-aes.c
index 30c78afe3dea..7ed39b40e4f5 100644
--- a/drivers/crypto/tegra/tegra-se-aes.c
+++ b/drivers/crypto/tegra/tegra-se-aes.c
@@ -1213,16 +1213,15 @@ static int tegra_ccm_do_one_req(struct crypto_engine *engine, void *areq)
rctx->inbuf.size = rctx->assoclen + rctx->authsize + rctx->cryptlen + 100;
rctx->inbuf.buf = dma_alloc_coherent(ctx->se->dev, rctx->inbuf.size,
&rctx->inbuf.addr, GFP_KERNEL);
+ ret = -ENOMEM;
if (!rctx->inbuf.buf)
goto out_finalize;
rctx->outbuf.size = rctx->assoclen + rctx->authsize + rctx->cryptlen + 100;
rctx->outbuf.buf = dma_alloc_coherent(ctx->se->dev, rctx->outbuf.size,
&rctx->outbuf.addr, GFP_KERNEL);
- if (!rctx->outbuf.buf) {
- ret = -ENOMEM;
+ if (!rctx->outbuf.buf)
goto out_free_inbuf;
- }
if (!ctx->key_id) {
ret = tegra_key_submit_reserved_aes(ctx->se, ctx->key,
--
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
* Re: [PATCH] crypto: hisilicon/sec2 - lower priority for hisilicon crypto implementations
From: Eric Biggers @ 2026-05-20 1:32 UTC (permalink / raw)
To: liulongfang
Cc: Chenghai Huang, herbert, davem, linux-kernel, linux-crypto,
fanghao11, qianweili, wangzhou1
In-Reply-To: <fe78b23b-37bb-5995-94b5-64fcf9578722@huawei.com>
On Wed, May 20, 2026 at 09:22:49AM +0800, liulongfang wrote:
> On 2026/5/11 8:49, Chenghai Huang wrote:
> > From: lizhi <lizhi206@huawei.com>
> >
> > Lower the priority of HiSilicon's crypto implementations to allow more
> > suitable alternatives to be selected. For example, certain kernel
> > use-cases do not benefit from HiSilicon's symmetric crypto algorithms.
> > This change ensures that more appropriate options are chosen first while
> > retaining HiSilicon's implementations as alternatives.
> >
> > Signed-off-by: lizhi <lizhi206@huawei.com>
> > Signed-off-by: Chenghai Huang <huangchenghai2@huawei.com>
> > ---
> > drivers/crypto/hisilicon/sec2/sec_crypto.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/crypto/hisilicon/sec2/sec_crypto.c b/drivers/crypto/hisilicon/sec2/sec_crypto.c
> > index 2471a4dd0b50..77e0e03cbcab 100644
> > --- a/drivers/crypto/hisilicon/sec2/sec_crypto.c
> > +++ b/drivers/crypto/hisilicon/sec2/sec_crypto.c
> > @@ -20,7 +20,7 @@
> > #include "sec.h"
> > #include "sec_crypto.h"
> >
> > -#define SEC_PRIORITY 4001
> > +#define SEC_PRIORITY 80
> > #define SEC_XTS_MIN_KEY_SIZE (2 * AES_MIN_KEY_SIZE)
> > #define SEC_XTS_MID_KEY_SIZE (3 * AES_MIN_KEY_SIZE)
> > #define SEC_XTS_MAX_KEY_SIZE (2 * AES_MAX_KEY_SIZE)
> >
>
> Reviewed-by: Longfang Liu <liulongfang@huawei.com>
Makes sense, but perhaps this driver should just be removed entirely?
- Eric
^ permalink raw reply
* Re: [PATCH] crypto: hisilicon/sec2 - lower priority for hisilicon crypto implementations
From: liulongfang @ 2026-05-20 1:22 UTC (permalink / raw)
To: Chenghai Huang, herbert, davem
Cc: linux-kernel, linux-crypto, fanghao11, qianweili, wangzhou1
In-Reply-To: <20260511004927.3469951-1-huangchenghai2@huawei.com>
On 2026/5/11 8:49, Chenghai Huang wrote:
> From: lizhi <lizhi206@huawei.com>
>
> Lower the priority of HiSilicon's crypto implementations to allow more
> suitable alternatives to be selected. For example, certain kernel
> use-cases do not benefit from HiSilicon's symmetric crypto algorithms.
> This change ensures that more appropriate options are chosen first while
> retaining HiSilicon's implementations as alternatives.
>
> Signed-off-by: lizhi <lizhi206@huawei.com>
> Signed-off-by: Chenghai Huang <huangchenghai2@huawei.com>
> ---
> drivers/crypto/hisilicon/sec2/sec_crypto.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/crypto/hisilicon/sec2/sec_crypto.c b/drivers/crypto/hisilicon/sec2/sec_crypto.c
> index 2471a4dd0b50..77e0e03cbcab 100644
> --- a/drivers/crypto/hisilicon/sec2/sec_crypto.c
> +++ b/drivers/crypto/hisilicon/sec2/sec_crypto.c
> @@ -20,7 +20,7 @@
> #include "sec.h"
> #include "sec_crypto.h"
>
> -#define SEC_PRIORITY 4001
> +#define SEC_PRIORITY 80
> #define SEC_XTS_MIN_KEY_SIZE (2 * AES_MIN_KEY_SIZE)
> #define SEC_XTS_MID_KEY_SIZE (3 * AES_MIN_KEY_SIZE)
> #define SEC_XTS_MAX_KEY_SIZE (2 * AES_MAX_KEY_SIZE)
>
Reviewed-by: Longfang Liu <liulongfang@huawei.com>
Thanks
Longfang.
^ permalink raw reply
* Re: [RFC] TID v2.0: kernel module for cache-line zeroization against Flush+Reload (CLFLUSHOPT + LFENCE + REP STOSQ)
From: Jann Horn @ 2026-05-19 21:41 UTC (permalink / raw)
To: Ahmad Hasan
Cc: linux-kernel, linux-security-module, linux-hardening,
kernel-hardening, linux-crypto, linux-mm, linux-api,
linux-kselftest
In-Reply-To: <CAAmtCfMHqdWbYh-Hc5sGbOhXSM-aCA9G0-s64G8FTM+rGEV5RA@mail.gmail.com>
On Tue, May 19, 2026 at 11:31 PM Ahmad Hasan
<ahmaaaaadbntaaaaa@gmail.com> wrote:
> Thank you for your questions. I'll address each one:
>
> == 1. Threat Model ==
>
> The target scenario is a same-machine attacker
> in multi-tenant/cloud environments where two
> processes share physical L3 cache.
>
> Example: a cryptographic service and a malicious
> process running on the same host. The attacker
> uses Flush+Reload to measure cache access timing
> after every encryption operation — no physical
> access required.
>
> This is documented with real measurements:
> - Without TID: 78 cycles (Cache HIT — key pattern visible)
> - With TID v2.0: 286 cycles (Cache MISS — attack defeated)
So you're assuming that the cryptographic code leaks secrets through a
cache-based side channel? That would be a vulnerability in the crypto
code.
> == 2. Why Kernel Module and not userspace? ==
>
> You are correct that CLFLUSHOPT does not require
> Ring 0. However, userspace execution can be
> interrupted by a Context Switch, which expands
> the timing window from 372ns to 36,640ns —
> making the attack significantly easier.
Why does it matter how many hundreds of nanoseconds it takes to wipe
the data from memory? You can also have a context switch directly
before you enter your cache-wiping syscall, or in the middle of a
crypto operation.
> == 3. Why not add this directly to libraries? ==
>
> No major security library implements CLFLUSHOPT
> after wiping — not OpenSSL, not libsodium, not
> glibc, not memzero_explicit. This gap has existed
> since Flush+Reload was published in 2014.
I don't think that's a gap, because the standard approach to
mitigating cache-based side channels such as FLUSH+RELOAD is to not
access memory at secret-dependent indices in the first place.
^ permalink raw reply
* Re: Which, if any, of the async crypto drivers are ever useful in the real world?
From: Eric Biggers @ 2026-05-19 21:07 UTC (permalink / raw)
To: Simon Richter; +Cc: Demi Marie Obenour, linux-crypto, Herbert Xu
In-Reply-To: <e07dc0ab-fcc3-4525-a758-f7b4808953c8@hogyros.de>
On Wed, May 20, 2026 at 05:36:04AM +0900, Simon Richter wrote:
> Hi,
>
> On 5/18/26 19:11, Demi Marie Obenour wrote:
>
> > Is it really *always* better to do the cryptography inline or on the
> > CPU?
>
> If there is an inline crypto engine, that is preferable, because we can
> submit a single async request and have the hardware mediate the async
> requests to the lower layers for us, reducing overhead.
>
> The CPU is a good choice if there is some acceleration built into it (like
> AES-NI or NEON), request sizes are small, there is no batching, the CPU is
> otherwise idle and total throughput per stream is manageable with a single
> core.
>
> That's a lot of conditions, but they happen to be fulfilled in a typical
> desktop PC use case, and usually there is no async offload option there
> anyway, so we end up on a CPU.
CPU is often preferable even when those conditions aren't met.
It's really the other way around. There's a long list of things that
would have to go right for a standalone symmetric crypto engine to be
worthwhile.
Here are the results of some real world tests:
- https://lore.kernel.org/linux-crypto/20250615184638.GA1480@sol/
- https://lore.kernel.org/linux-crypto/20250616164752.GB1373@sol/
- https://lore.kernel.org/linux-fscrypt/20250704070322.20692-1-ebiggers@kernel.org/
> fscrypt went the other direction, splitting requests from upper layers into
> individual data objects, submitting each separately and waiting for
> completion, which I can understand from a software complexity perspective,
> but it maximizes overhead for offloading.
Most kernel code that uses cryptography is synchronous. So this is the
norm, not the exception. Using the async callbacks is the exception,
and history has shown that it's very hard to implement correctly: it
typically results in lots of bug fixes being needed. It's also very
common for the async drivers themselves to have bugs, so anyone
prioritizing correctness can't really use them anyway.
> In general, if an offload engine with an async driver exists, I would expect
> that it provides a benefit over the CPU, in the worst case it frees up a CPU
> core even if there is no significant performance difference, and it uses
> less energy than a general-purpose core would.
For standalone symmetric crypto engines, real-world tests show
otherwise.
- Eric
^ permalink raw reply
* Re: [PATCH 00/12] crypto: atmel - refactor common i2c support and add SHA256 ahash support
From: Lothar Rubusch @ 2026-05-19 20:47 UTC (permalink / raw)
To: Thorsten Blum
Cc: herbert, davem, nicolas.ferre, alexandre.belloni, claudiu.beznea,
linux-crypto, linux-arm-kernel, linux-kernel
In-Reply-To: <agYnpsiG8bNeVw57@linux.dev>
Hi Thorsten,
(...)
>
> Sashiko [1] also reviewed the series and found potential regressions
> that might be helpful to consider.
>
> Thanks,
> Thorsten
>
> [1] https://sashiko.dev/#/patchset/20260512224349.64621-1-l.rubusch%40gmail.com
FYI
As you asked to consider the Sashiko output, I will paste here the sashiko
review feedback and inlined my answers down below. I may continue with this
still some more iterations against AI/sashiko bot, no problem. How eventually
can I signal you (guys), when I need someone more "human" / maintainer?
What exciting times, we're living in... - let me know, if I use this tool wrong.
[PATCH 01/12] crypto: atmel-ecc - rename driver_data before moving it
into atmel-i2c
> commit 0ed25c0f5044cd5112cafff9ff8ab8248d0e2676
> Author: Lothar Rubusch <l.rubusch@gmail.com>
> crypto: atmel-ecc - rename driver_data before moving it into atmel-i2c
> This commit renames the local driver_data instance to atmel_i2c_mgmt
> to prepare for moving shared I2C client management infrastructure into
> the atmel-i2c core driver. No functional changes are intended.
> > diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
> > index 9660f6426a849..c9f798ebf44fe 100644
> > --- a/drivers/crypto/atmel-ecc.c
> > +++ b/drivers/crypto/atmel-ecc.c
> [ ... ]
> > @@ -323,16 +323,16 @@ static int atmel_ecc_probe(struct i2c_client *client)
> >
> > i2c_priv = i2c_get_clientdata(client);
> >
> > - spin_lock(&driver_data.i2c_list_lock);
> > + spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
> > list_add_tail(&i2c_priv->i2c_client_list_node,
> > - &driver_data.i2c_client_list);
> > - spin_unlock(&driver_data.i2c_list_lock);
> > + &atmel_i2c_mgmt.i2c_client_list);
> > + spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
> >
> > ret = crypto_register_kpp(&atmel_ecdh_nist_p256);
> This isn't a bug introduced by this patch, but does adding i2c_priv to the
It is IMHO not very helpful to flag a(ll) particular patch(es) for regressions
in the existing code aside, but ok. Perhaps I can fix something.
> global list before crypto_register_kpp() create a use-after-free risk?
> If algorithm registration fails, the error path removes the client from the
> list and returns an error, leading the driver core to free the i2c_priv
> memory.
> However, during the brief window it was on the list, a concurrent
> atmel_ecc_i2c_client_alloc() could discover the client and assign it to an
> active crypto request.
> When the probe fails and memory is freed, would the concurrent crypto
> request operate on freed memory?
> Also, does calling crypto_register_kpp() with a single static global
> algorithm structure cause memory corruption if multiple devices are probed?
> The crypto core modifies the algorithm structure before taking locks, which
> would corrupt the state of the active algorithm registered by the first
> device.
> The second probe would also fail, breaking the multiple-device load-balancing
> design. Unregistering a single device also unconditionally unregisters the
> shared algorithm.
As soon as being registered, the struct `atmel_ecdh_nist_p256` will require to
access the `atmel_i2c_mgmt.i2c_client_list`. Being it for the selftests, or a
consumer. Thus I cannot simply change the order. I added a ready/state flag to
address the use-after-free risk.
> > if (ret) {
> > - spin_lock(&driver_data.i2c_list_lock);
> > + spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
> > list_del(&i2c_priv->i2c_client_list_node);
> > - spin_unlock(&driver_data.i2c_list_lock);
> > + spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
> [ ... ]
> > @@ -363,9 +363,9 @@ static void atmel_ecc_remove(struct i2c_client *client)
> >
> > crypto_unregister_kpp(&atmel_ecdh_nist_p256);
> >
> > - spin_lock(&driver_data.i2c_list_lock);
> > + spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
> > list_del(&i2c_priv->i2c_client_list_node);
> > - spin_unlock(&driver_data.i2c_list_lock);
> > + spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
> > }
> This isn't a bug introduced by this patch, but does the lockless check of
Sure, let's continue drive-by-reviewing...
> tfm_count earlier in atmel_ecc_remove() introduce a race condition?
> A concurrent atmel_ecc_i2c_client_alloc() could acquire the list lock and
> increment tfm_count immediately after the check, causing the device to be
> freed while a new session is starting.
> Additionally, if tfm_count is detected as greater than zero, the driver
> initially returns early. This leaves the freed i2c_priv on the global list
> which leads to list corruption and use-after-free on subsequent allocations.
> Returning from remove does not prevent the driver core from freeing the
> devres-managed memory. Should this synchronously block until tfm_count
> reaches zero to safely unbind?
> The dangling list pointer issue is fixed later in the series by commit
> "crypto: atmel-ecc - simplify remove path and relax busy handling", but the
> underlying active-user use-after-free remains unfixed.
I added a global lock with refcnting to address this.
[PATCH 02/12] crypto: atmel - rename atmel_ecc_driver_data to
atmel_i2c_client_mgmt
> Result:
> No issues found.
[PATCH 03/12] crypto: atmel - move i2c client management instance into
core driver
> commit 81ed08d3b64ea6591f1cadc2ccdca81d40c20205
> Author: Lothar Rubusch <l.rubusch@gmail.com>
> crypto: atmel - move i2c client management instance into core driver
> This commit moves the atmel_i2c client management instance from the ecc
> driver into the atmel-i2c core driver. This prepares for consolidating
> shared i2c client tracking infrastructure in the core.
> [ ... ]
> > diff --git a/drivers/crypto/atmel-i2c.c b/drivers/crypto/atmel-i2c.c
> > index 0e275dbdc8c55..db24f65ae90e8 100644
> > --- a/drivers/crypto/atmel-i2c.c
> > +++ b/drivers/crypto/atmel-i2c.c
> > @@ -21,6 +21,12 @@
> > #include <linux/workqueue.h>
> > #include "atmel-i2c.h"
> >
> > +struct atmel_i2c_client_mgmt atmel_i2c_mgmt = {
> > + .i2c_list_lock = __SPIN_LOCK_UNLOCKED(atmel_i2c_mgmt.i2c_list_lock),
> > + .i2c_client_list = LIST_HEAD_INIT(atmel_i2c_mgmt.i2c_client_list),
> > +};
> > +EXPORT_SYMBOL_GPL(atmel_i2c_mgmt);
> While this regression wasn't introduced by this patch, does exposing this
Sure..
> globally shared list allow for a time-of-check to time-of-use race in
> atmel_ecc_remove() that can result in a use-after-free?
> In atmel_ecc_remove(), the check if (atomic_read(&i2c_priv->tfm_count)) is
> performed without holding the i2c_list_lock.
> If the count is 0, the function proceeds to call crypto_unregister_kpp().
> During this algorithm unregistration window, the client remains accessible on
> the globally shared atmel_i2c_mgmt.i2c_client_list.
> If a concurrent thread calls crypto_alloc_kpp(), it triggers the .init callback
> atmel_ecdh_init_tfm(), which calls atmel_ecc_i2c_client_alloc().
> The allocator acquires the list lock, discovers the dying client still on the
> list, increments tfm_count, and returns it.
> Then, atmel_ecc_remove() acquires the list lock, removes the client from the
> list, and returns.
> Returning from the remove function causes the devres framework to automatically
> free i2c_priv, leaving the concurrent thread with a tfm bound to freed memory.
> Can this sequence lead to memory corruption?
The two additional patches will cover the remove function.
[PATCH 04/12] crypto: atmel-ecc - simplify probe error handling
> commit 3bca954bd7e39050b3fe5381aad35c915da883f3
> Author: Lothar Rubusch <l.rubusch@gmail.com>
> crypto: atmel-ecc - simplify probe error handling
> This commit refactors the error path in atmel_ecc_probe to use a goto
> statement instead of duplicating the list removal code. It also claims to
> add comments clarifying client list insertion and algorithm registration.
> > crypto: atmel-ecc - simplify probe error handling
> >
> > Replace early return in atmel_ecc_probe() with explicit error handling
> > using a goto-based cleanup path.
> >
> > Add comments to clarify client list insertion and algorithm registration
> > steps.
> This isn't a bug, but the commit message states it replaces an early
> return and adds comments, yet the diff doesn't show any comments being
> added or an early return being removed. Should the commit message be
> updated to reflect the actual changes?
> > diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
> > index 76fb1d0cf0753..696ab1d51fc67 100644
> > --- a/drivers/crypto/atmel-ecc.c
> > +++ b/drivers/crypto/atmel-ecc.c
> > @@ -328,17 +328,20 @@ static int atmel_ecc_probe(struct i2c_client *client)
> >
> > ret = crypto_register_kpp(&atmel_ecdh_nist_p256);
> Is it safe to register the static global atmel_ecdh_nist_p256 algorithm
> structure here directly from the probe function? If multiple devices are
> probed, a second call to crypto_register_kpp with the same static struct
> could cause crypto_check_alg to reset the active algorithm's reference
> count, and __crypto_register_alg to re-initialize the cra_users list,
> potentially causing a regression that corrupts the state for active
> crypto users.
> > if (ret) {
> > - spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
> > - list_del(&i2c_priv->i2c_client_list_node);
> > - spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
> > -
> > dev_err(&client->dev, "%s alg registration failed\n",
> > atmel_ecdh_nist_p256.base.cra_driver_name);
> > + goto err_list_del;
> By moving the list removal to a goto block after the dev_err call, does this
> code introduce a regression by widening the window for a race condition?
> Since the client is added to the global atmel_i2c_mgmt.i2c_client_list earlier
> in the probe, if registration fails, a concurrent task might find and use the
> failed client. Executing a potentially slow console log before removing the
> client from the list seems to worsen the window for a use-after-free when
> devres cleans up i2c_priv.
> [ ... ]
Patch replaced and merged
[PATCH 05/12] crypto: atmel - factor out i2c client unregistration helper
> commit 67148be9a0cfbd189723055fbc1cdfa19bd244ad
> Author: Lothar Rubusch <l.rubusch@gmail.com>
> crypto: atmel - factor out i2c client unregistration helper
> This commit moves the i2c client removal logic into a dedicated helper
> function in the atmel-i2c core. It also converts the ECC driver remove
> path to use this new helper.
> > Convert ECC driver remove path to use the new helper and
> > ensure queue flushing is performed after removing the device
> > from the shared client list.
> Is this description accurate? Looking at the implementation below, the queue
> flushing is performed before removing the device from the list.
> > diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
> > index 696ab1d51fc67..e5dd77008b97f 100644
> > --- a/drivers/crypto/atmel-ecc.c
> > +++ b/drivers/crypto/atmel-ecc.c
> [ ... ]
> > @@ -364,9 +364,8 @@ static void atmel_ecc_remove(struct i2c_client *client)
> >
> > crypto_unregister_kpp(&atmel_ecdh_nist_p256);
> >
> > - spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
> > - list_del(&i2c_priv->i2c_client_list_node);
> > - spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
> > + atmel_i2c_flush_queue();
> > + atmel_i2c_unregister_client(i2c_priv);
> > }
> While crypto_unregister_kpp() prevents new lookups, concurrent threads that
> already resolved the algorithm can still execute atmel_ecdh_init_tfm().
> Because atmel_i2c_flush_queue() is called first, it will sleep waiting for
> the workqueue to drain while the client is still present in the global
> atmel_i2c_mgmt.i2c_client_list.
> Could a concurrent atmel_ecdh_init_tfm() call atmel_i2c_client_alloc(),
> find the still-linked client, increment its tfm_count, and assign it to
> ctx->client?
> Once the flush completes, atmel_i2c_unregister_client() removes the client,
> the driver returns, and the device core frees the i2c_priv memory.
> Would this result in a use-after-free on subsequent crypto operations since
> ctx->client now points to freed memory?
Replaced by a more comprehensive dealing with the teardown, patch replaced.
[PATCH 06/12] crypto: atmel-sha204a - add i2c hw client list and
improve probe error handling
> commit 14fd691f797c32576c19b57c937cd6cf75be5ef8
> Author: Lothar Rubusch <l.rubusch@gmail.com>
> crypto: atmel-sha204a - add i2c hw client list and improve probe error handling
> This patch adds registration of the sha204a i2c client into the shared
> atmel_i2c client management list during probe. This allows the driver to
> participate in the common hardware selection infrastructure.
> > diff --git a/drivers/crypto/atmel-sha204a.c b/drivers/crypto/atmel-sha204a.c
> > index 6e6ac4770416e..cdfdcf2e43a7d 100644
> > --- a/drivers/crypto/atmel-sha204a.c
> > +++ b/drivers/crypto/atmel-sha204a.c
> > @@ -173,6 +173,13 @@ static int atmel_sha204a_probe(struct i2c_client *client)
> >
> > i2c_priv = i2c_get_clientdata(client);
> >
> > + /* add to client list */
> > + spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
> > + list_add_tail(&i2c_priv->i2c_client_list_node,
> > + &atmel_i2c_mgmt.i2c_client_list);
> > + spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
> Is it possible for another thread to discover this client before the rest of
> the probe completes?
> If another thread like atmel_i2c_client_alloc() grabs a reference to this
> client from the shared list, and then a subsequent initialization step fails,
> the error path will fail the probe. The devres cleanup will free i2c_priv,
> but the concurrent user will be left with a dangling pointer.
> Also, since the shared atmel_i2c_mgmt.i2c_client_list is used by the
> atmel-ecc driver for hardware selection, could adding the sha204a device
> here cause the atmel-ecc driver to incorrectly dispatch ecdh/ecdsa
> operations to this device?
> Since the selection logic picks the client with the lowest active workload,
> it might select this device and the hardware would reject unsupported
> commands. This was addressed later in the series by commit "crypto: atmel -
> add capability-based i2c client selection".
> Additionally, if the atmel-sha204a module is loaded first, could loading the
> atmel-ecc module later completely wipe out this shared list?
> If the atmel-ecc initialization blindly uses INIT_LIST_HEAD() on this list,
> it would orphan the sha204a clients. This was fixed later in the series by
> commit "crypto: atmel-ecc - switch to module_i2c_driver".
> > +
> > + /* register rng */
> > memset(&i2c_priv->hwrng, 0, sizeof(i2c_priv->hwrng));
> >
> > i2c_priv->hwrng.name = dev_name(&client->dev);
> > @@ -183,15 +190,24 @@ static int atmel_sha204a_probe(struct i2c_client *client)
> > i2c_priv->hwrng.quality = *quality;
> >
> > ret = devm_hwrng_register(&client->dev, &i2c_priv->hwrng);
> > - if (ret)
> > + if (ret) {
> > dev_warn(&client->dev, "failed to register rng (%d)\n", ret);
> > + goto err_list_del;
> > + }
> >
> > ret = sysfs_create_group(&client->dev.kobj, &atmel_sha204a_groups);
> > if (ret) {
> > dev_err(&client->dev, "failed to register sysfs entry\n");
> > - return ret;
> > + goto err_list_del;
> > }
> Is it possible for userspace to queue an asynchronous non-blocking read
> between devm_hwrng_register() and this sysfs_create_group() call?
> If an async rng read is queued to atmel_wq and work_data is allocated, and
> then sysfs_create_group() fails, devres will unwind allocations and free
> i2c_priv.
> Without calling atmel_i2c_flush_queue() in the error path, the pending work
> would execute after i2c_priv is freed. This would also leak the dynamically
> allocated work_data.
> >
> > + return ret;
> > +
> > +err_list_del:
> > + spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
> > + list_del(&i2c_priv->i2c_client_list_node);
> > + spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
> > +
> > return ret;
> > }
> Does the atmel_sha204a_remove() function need an update to remove the i2c
> client from the global list?
> Since the client is added during probe, it appears it would be left dangling
> in the list when the device is unbound and devres frees i2c_priv,
> potentially causing a crash if the list is traversed later.
> This was fixed later in the series by commit "crypto: atmel-sha204a -
> switch to module_i2c_driver".
Chronologically this is the wrong order. I moved this patch down to the end.
The issue should be fully covered. In this way, Atmel SHA204a devices are
added to the i2c list, when they can configure not being available for ECDH
operations. This is a preparation for future capabilities
(as in the initial roughly splitted series).
[PATCH 07/12] crypto: atmel-sha204a - switch to module_i2c_driver
> commit e46024ee80d35e3759c2e5365d027bec6fb2ea51
> Author: Lothar Rubusch <l.rubusch@gmail.com>
> crypto: atmel-sha204a - switch to module_i2c_driver
> Replace custom module init/exit functions with module_i2c_driver() for
> driver registration. Update remove path to unregister the client from the
> shared I2C management list before flushing pending work and cleaning up
> sysfs and hwrng resources.
> > Update remove path to unregister the client from the shared I2C management
> > list before flushing pending work and cleaning up sysfs and hwrng
> > resources.
> >
> > No functional change intended.
> This isn't a bug, but this patch appears to fix a list corruption regression.
> Before this patch, unloading the module would leave a freed i2c_priv structure
> linked into the shared atmel_i2c_mgmt.i2c_client_list. Does adding the
> atmel_i2c_unregister_client() call fix this dangling pointer issue, making
> the "No functional change intended" claim inaccurate?
> > diff --git a/drivers/crypto/atmel-sha204a.c b/drivers/crypto/atmel-sha204a.c
> > index cdfdcf2e43a7d..613ed5e7b3f61 100644
> > --- a/drivers/crypto/atmel-sha204a.c
> > +++ b/drivers/crypto/atmel-sha204a.c
> > @@ -216,6 +216,8 @@ static void atmel_sha204a_remove(struct i2c_client *client)
> > struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(client);
> >
> > devm_hwrng_unregister(&client->dev, &i2c_priv->hwrng);
> > +
> > + atmel_i2c_unregister_client(i2c_priv);
> The commit message states the intent to unregister the client from the shared
> list before cleaning up hwrng resources. Since devm_hwrng_unregister()
> blocks until active synchronous readers complete, does placing
> atmel_i2c_unregister_client() after it leave the client exposed in the shared
> list while the device is actively being torn down?
> Could another subsystem discover and bind to this partially removed client
> during the blocking window, resulting in memory corruption once the unbind
> completes?
> Should atmel_i2c_unregister_client() be moved to the beginning of the remove
> function to match the design described in the commit message?
> > atmel_i2c_flush_queue();
> >
> > sysfs_remove_group(&client->dev.kobj, &atmel_sha204a_groups);
> Additionally, I noticed a potential use-after-free regression in the probe
> error path.
> In atmel_sha204a_probe(), if sysfs_create_group() fails, the function
> returns an error:
> drivers/crypto/atmel-sha204a.c:atmel_sha204a_probe() {
> [ ... ]
> ret = devm_hwrng_register(&client->dev, &i2c_priv->hwrng);
> if (ret) {
> dev_warn(&client->dev, "failed to register RNG (%d)\n", ret);
> goto err_list_del;
> }
> ret = sysfs_create_group(&client->dev.kobj, &atmel_sha204a_groups);
> if (ret) {
> dev_err(&client->dev, "failed to register sysfs entry\n");
> goto err_list_del;
> }
> return ret;
> err_list_del:
> spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
> list_del(&i2c_priv->i2c_client_list_node);
> spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
> return ret;
> }
> At this point, the RNG has already been registered and exposed to user-space
> via devm_hwrng_register(). A concurrent user-space read could have initiated
> an asynchronous read, which queues a work item onto atmel_wq.
> When probe() returns an error, devres will immediately unregister the RNG and
> free the i2c_priv structure. Since the pending work on atmel_wq is never
> flushed or canceled, will it dereference the freed i2c_priv when it
> eventually executes?
> Should atmel_i2c_flush_queue() be explicitly called in the probe error path,
> just as it is in remove()?
> I also noticed a potential kernel heap memory leak to user-space in the
> asynchronous read path.
> In atmel_sha204a_rng_done(), if an asynchronous I2C transaction fails,
> the buffer is unconditionally cached:
> drivers/crypto/atmel-sha204a.c:atmel_sha204a_rng_done() {
> [ ... ]
> if (status)
> dev_warn_ratelimited(&i2c_priv->client->dev,
> "i2c transaction failed (%d)\n",
> status);
> rng->priv = (unsigned long)work_data;
> [ ... ]
> }
> If status != 0, the work_data->cmd.data buffer is left uninitialized or
> partially written.
> On the next call to atmel_sha204a_rng_read_nonblocking(), the code blindly
> copies up to 32 bytes of this buffer:
> drivers/crypto/atmel-sha204a.c:atmel_sha204a_rng_read_nonblocking() {
> [ ... ]
> if (rng->priv) {
> work_data = (struct atmel_i2c_work_data *)rng->priv;
> max = min(RANDOM_RSP_SIZE - CMD_OVERHEAD_SIZE, max);
> memcpy(data, &work_data->cmd.data[RSP_DATA_IDX], max);
> rng->priv = 0;
> } else {
> work_data = kmalloc_obj(*work_data, GFP_ATOMIC);
> [ ... ]
> }
> Because work_data is allocated without zeroing memory, does this copy
> uninitialized kernel heap memory into the hwrng output data buffer?
> Could this allow an attacker to leak kernel heap memory to user-space via
> /dev/hwrng by inducing I2C read failures?
Chronologically this is the wrong order. I moved this patch down to the end.
Add different error labels in probe to address failing sysfs_create_group().
Fix kernel heap information leak in a separate patch.
[PATCH 08/12] crypto: atmel-ecc - switch to module_i2c_driver
> commit 1156053e97f2cc049a62750ad595b29dc8a4e921
> Author: Lothar Rubusch <l.rubusch@gmail.com>
> Date: Sun May 17 18:06:35 2026 +0000
> crypto: atmel-ecc - switch to module_i2c_driver
>
> Replace custom module init/exit functions with module_i2c_driver() for
> simplified driver registration.
>
> Initialization of the shared I2C client management structure is handled by
> the core driver and no longer performed in the ECC module init path.
> > diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
> > index e5dd77008b97f..dcfc09d244973 100644
> > --- a/drivers/crypto/atmel-ecc.c
> > +++ b/drivers/crypto/atmel-ecc.c
> > @@ -392,21 +392,7 @@ static struct i2c_driver atmel_ecc_driver = {
> > .id_table = atmel_ecc_id,
> > };
> >
> > -static int __init atmel_ecc_init(void)
> > -{
> > - spin_lock_init(&atmel_i2c_mgmt.i2c_list_lock);
> > - INIT_LIST_HEAD(&atmel_i2c_mgmt.i2c_client_list);
> > - return i2c_add_driver(&atmel_ecc_driver);
> > -}
> > -
> > -static void __exit atmel_ecc_exit(void)
> > -{
> > - atmel_i2c_flush_queue();
> > - i2c_del_driver(&atmel_ecc_driver);
> > -}
> > -
> > -module_init(atmel_ecc_init);
> > -module_exit(atmel_ecc_exit);
> > +module_i2c_driver(atmel_ecc_driver);
> While this change simplifies driver registration, does this driver have a
> pre-existing regression involving a use-after-free in its remove path?
again, "pre-existing regression". I already addressed this topic above.
> Looking at the driver's remove path, it can return early if the client is
> busy:
> drivers/crypto/atmel-ecc.c:atmel_ecc_remove() {
> struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(client);
> if (atomic_read(&i2c_priv->tfm_count)) {
> dev_emerg(&client->dev, "Device is busy, expect memory corruption.\n");
> return;
> }
> ...
> }
> When atmel_ecc_remove() returns, the driver core unconditionally frees all
> devres-managed memory, including i2c_priv, which is allocated via
> devm_kmalloc() during probe.
> If there are open crypto transforms holding pointers to this freed memory,
> will subsequently closing a busy transform cause atmel_ecdh_exit_tfm() to
> access the freed i2c_priv->tfm_count and trigger a use-after-free of
> i2c_priv?
> Furthermore, does the early return leave the atmel_ecdh_nist_p256 algorithm
> permanently registered with dangling pointers to module text?
> Additionally, could a time-of-check to time-of-use race occur here?
> If atmel_ecc_remove() sees tfm_count == 0, a concurrent transform allocation
> might acquire the client just before it is removed from the global list,
> leading to the same devres use-after-free.
> Note that later commits like "crypto: atmel-ecc - simplify remove path and
> relax busy handling" remove the early return, but the fundamental
> use-after-free persists because devres still frees i2c_priv while active
> transforms hold references to it.
> Is there also a pre-existing regression in the probe error path that can lead
> to a use-after-free?
> The client is added to the global atmel_i2c_mgmt.i2c_client_list before
> crypto_register_kpp() is called:
> drivers/crypto/atmel-ecc.c:atmel_ecc_probe() {
> ...
> spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
> list_add_tail(&i2c_priv->i2c_client_list_node,
> &atmel_i2c_mgmt.i2c_client_list);
> spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
> ret = crypto_register_kpp(&atmel_ecdh_nist_p256);
> if (ret) {
> ...
> goto err_list_del;
> }
> ...
> }
> If registration fails (for example, because another device already registered
> the algorithm), the error path removes the client from the list and returns
> an error. This causes the driver core to free the devm-allocated i2c_priv.
> While the client was briefly in the global list, could a concurrent transform
> allocation acquire it via atmel_ecc_i2c_client_alloc()?
> This would leave the concurrent transform holding a dangling pointer to the
> freed i2c_priv memory, resulting in a use-after-free when cryptographic
> operations are submitted.
> >
> > MODULE_AUTHOR("Tudor Ambarus");
> > MODULE_DESCRIPTION("Microchip / Atmel ECC (I2C) driver");
All of the issues are supposed to be already addressed here. Let's have a look
into next Sahiko iteration. Patch updated.
[PATCH 09/12] crypto: atmel-ecc - simplify remove path and relax busy handling
> commit e814c428ad784badd861d6adbe0e2dbf99beae12
> Author: Lothar Rubusch <l.rubusch@gmail.com>
> crypto: atmel-ecc - simplify remove path and relax busy handling
> This commit simplifies the removal path of the atmel-ecc driver by removing
> the early exit logic when active transform users are detected. Instead, it
> issues a warning and continues with the full teardown of the crypto
> registration and device cleanup.
> > diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
> > index dcfc09d244973..ce7a2e750ba88 100644
> > --- a/drivers/crypto/atmel-ecc.c
> > +++ b/drivers/crypto/atmel-ecc.c
> > @@ -346,21 +346,14 @@ static int atmel_ecc_probe(struct i2c_client *client)
> >
> > static void atmel_ecc_remove(struct i2c_client *client)
> > {
> > - struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(client);
> > + struct atmel_i2c_client_priv *i2c_priv;
> >
> > - /* Return EBUSY if i2c client already allocated. */
> > - if (atomic_read(&i2c_priv->tfm_count)) {
> [ ... ]
> > - dev_emerg(&client->dev, "Device is busy, expect memory corruption.\n");
> > + i2c_priv = i2c_get_clientdata(client);
> > + if (WARN_ON(!i2c_priv))
> > return;
> > - }
> > +
> > + if (atomic_read(&i2c_priv->tfm_count))
> > + dev_warn(&client->dev, "Device is busy, remove it anyhow\n");
> Could this still lead to a use-after-free on i2c_priv?
> The i2c_priv structure is allocated using devres during probe. When
> atmel_ecc_remove() completes, the Linux device model automatically triggers
> devres cleanup, which unconditionally frees i2c_priv.
> If active crypto transforms (indicated by tfm_count > 0) retain pointers to
> the device context, won't they dereference the freed i2c_priv when enqueuing
> new crypto operations or when they are eventually closed?
> > crypto_unregister_kpp(&atmel_ecdh_nist_p256);
> Does unregistering the statically allocated crypto algorithm while busy
> trigger a warning and corrupt its reference count?
> Because the atmel_ecdh_nist_p256 structure is statically allocated, it has
> no cra_destroy callback. Calling crypto_unregister_alg() while active
> transforms exist (cra_refcnt > 1) deterministically hits the safety check
> WARN_ON(!alg->cra_destroy && refcount_read(&alg->cra_refcnt) != 1) in
> crypto/algapi.c.
> Additionally, if the device is subsequently re-probed, crypto_register_kpp()
> will overwrite the structure's cra_refcnt to 1. When the original, orphaned
> transforms finally close and call crypto_alg_put(), they will decrement the
> newly reset refcount to 0. Future allocations of the algorithm would then
> trigger refcount_inc() on a zeroed refcount, permanently corrupting the
> algorithm's state.
> >
> > - atmel_i2c_flush_queue();
Patch dropped.
[PATCH 10/12] crypto: atmel-sha204a - guard remove path against
missing client data
> commit 940da01227a9a17cd454a0d965228cb8cdc7880a
> Author: Lothar Rubusch <l.rubusch@gmail.com>
> crypto: atmel-sha204a - guard remove path against missing client data
> This commit attempts to prevent dereferencing invalid client state during
> driver removal by adding a NULL check for the I2C client private data.
> It also mentions retrieving the data only after sysfs cleanup.
> > Retrieve the I2C client private data in atmel_sha204a_remove() only
> > after sysfs cleanup and add a NULL check before continuing device
> > teardown.
> Is this description accurate? The patch places the i2c_get_clientdata()
> call at the top of the function and leaves sysfs_remove_group() at the
> bottom, which does not appear to reorder these operations.
> > diff --git a/drivers/crypto/atmel-sha204a.c b/drivers/crypto/atmel-sha204a.c
> > --- a/drivers/crypto/atmel-sha204a.c
> > +++ b/drivers/crypto/atmel-sha204a.c
> > @@ -213,7 +213,11 @@ static int atmel_sha204a_probe(struct i2c_client *client)
> >
> > static void atmel_sha204a_remove(struct i2c_client *client)
> > {
> > - struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(client);
> > + struct atmel_i2c_client_priv *i2c_priv;
> > +
> > + i2c_priv = i2c_get_clientdata(client);
> > + if (WARN_ON(!i2c_priv))
> > + return;
> Since the driver core guarantees atmel_sha204a_remove() is only called if
> atmel_sha204a_probe() completed successfully, can i2c_priv ever actually be
> NULL here?
> If this early return were to trigger, would it skip the rest of the teardown
> sequence?
> Bypassing mandatory cleanup like sysfs_remove_group() and
> atmel_i2c_flush_queue() would leave dangling sysfs files and uncancelled
> work queues.
> Could the crash this patch attempts to mitigate actually be caused by a
> pre-existing issue in the probe() error path?
> If sysfs_create_group() fails after devm_hwrng_register() succeeds in
> atmel_sha204a_probe(), the function returns an error and devres cleans up
> i2c_priv.
> However, if a concurrent non-blocking read queued asynchronous work to
> atmel_wq, this work is not flushed because atmel_i2c_flush_queue() is
> omitted from the probe error path.
> When atmel_i2c_work_handler() executes later, would it dereference the
> already freed i2c_priv structure?
Patch dropped.
[PATCH 11/12] crypto: atmel - move i2c client selection to core driver
> commit ec87e048a52b48d6d9799216ee15e9f3c0e86cbc
> Author: Lothar Rubusch <l.rubusch@gmail.com>
> crypto: atmel - move i2c client selection to core driver
> Moves the I2C client selection logic from the ECC-specific driver into the
> shared Atmel I2C core. This allows the load-balancing logic to be reused
> across other Atmel crypto drivers.
> > diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
> > index ce7a2e750ba88..f877f236552f7 100644
> > --- a/drivers/crypto/atmel-ecc.c
> > +++ b/drivers/crypto/atmel-ecc.c
> [ ... ]
> > -static struct i2c_client *atmel_ecc_i2c_client_alloc(void)
> > -{
> [ ... ]
> > - return client;
> > -}
> > -
> > static void atmel_ecc_i2c_client_free(struct i2c_client *client)
> > {
> > struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(client);
> > @@ -249,7 +214,7 @@ static int atmel_ecdh_init_tfm(struct crypto_kpp *tfm)
> This isn't a bug, but the patch exports atmel_i2c_client_alloc() to share
> allocation logic while leaving the corresponding free function static here.
> Since the goal is for other Atmel crypto drivers to use the new shared
> allocation API, won't they be forced to manually access i2c_priv->tfm_count
> to decrement the counter? Should we also move and export a shared free
> function to create a symmetric API lifecycle?
> > diff --git a/drivers/crypto/atmel-i2c.c b/drivers/crypto/atmel-i2c.c
> > index 861af52d7a88b..4beab68997c4f 100644
> > --- a/drivers/crypto/atmel-i2c.c
> > +++ b/drivers/crypto/atmel-i2c.c
> [ ... ]
> > +struct i2c_client *atmel_i2c_client_alloc(void)
> > +{
> > + struct atmel_i2c_client_priv *i2c_priv, *min_i2c_priv = NULL;
> > + struct i2c_client *client = ERR_PTR(-ENODEV);
> > + int min_tfm_cnt = INT_MAX;
> > + int tfm_cnt;
> > +
> > + spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
> > +
> > + if (list_empty(&atmel_i2c_mgmt.i2c_client_list)) {
> > + spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
> > + return ERR_PTR(-ENODEV);
> > + }
> > +
> > + list_for_each_entry(i2c_priv, &atmel_i2c_mgmt.i2c_client_list,
> > + i2c_client_list_node) {
> > + tfm_cnt = atomic_read(&i2c_priv->tfm_count);
> > + if (tfm_cnt < min_tfm_cnt) {
> > + min_tfm_cnt = tfm_cnt;
> > + min_i2c_priv = i2c_priv;
> > + }
> > + if (!min_tfm_cnt)
> > + break;
> > + }
> > +
> > + if (min_i2c_priv) {
> > + atomic_inc(&min_i2c_priv->tfm_count);
> > + client = min_i2c_priv->client;
> > + }
> > +
> > + spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
> > +
> > + return client;
> > +}
> Could this result in a use-after-free if the device is unbound?
> If the underlying I2C device is asynchronously unbound while a crypto
> transformation is active, the driver's remove callback executes. The remove
> callback unregisters the algorithm to prevent new transformations, but it
> does not block waiting for active ones to be freed.
> When remove returns, the I2C subsystem frees the i2c_client structure, and
> devres automatically frees the allocated i2c_priv structure. The active
> transformation would retain the dangling ctx->client pointer. Any subsequent
> crypto operation would enqueue a work item that dereferences the freed client
> and accesses freed memory.
> Should we take a reference on the device via get_device(&client->dev) before
> returning it here to ensure it stays alive?
No. atmel_ecc_remove() was reworked, this issue should be addressed now.
> > +EXPORT_SYMBOL(atmel_i2c_client_alloc);
> This isn't a bug, but this function internally acquires locks and iterates
> over atmel_i2c_mgmt, which is exported via EXPORT_SYMBOL_GPL.
> Does exporting this as a non-GPL symbol allow non-GPL modules to bypass the
> GPL restriction of the underlying shared data structure? Should this be
> exported using EXPORT_SYMBOL_GPL to maintain consistency?
Renamed patch title. I changed to EXPORT_SYMBOL_GPL. The alloc and free
functions are now migrated.
[PATCH 12/12] crypto: atmel - add capability-based I2C client selection
> commit 67dd11917ea1c6943f1088bb31e4aa5da0cb31f4
> Author: Lothar Rubusch <l.rubusch@gmail.com>
> crypto: atmel - add capability-based I2C client selection
> This commit introduces capability-based filtering for the I2C client
> allocator. Devices now advertise supported features during probe, and the
> allocator selects clients that match the requested capabilities. This
> prepares the driver for using the hardware beyond a single algorithm type.
> > diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
> > --- a/drivers/crypto/atmel-ecc.c
> > +++ b/drivers/crypto/atmel-ecc.c
> [ ... ]
> > @@ -286,6 +286,8 @@ static int atmel_ecc_probe(struct i2c_client *client)
> >
> > i2c_priv = i2c_get_clientdata(client);
> >
> > + i2c_priv->caps = BIT(ATMEL_CAP_ECDH);
> > +
> > spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
> > list_add_tail(&i2c_priv->i2c_client_list_node,
> > &atmel_i2c_mgmt.i2c_client_list);
> This problem wasn't introduced by this patch, but there appears to be a race
Sure
> condition if a device probe fails.
> In atmel_ecc_probe(), the i2c_priv object is added to the global list before
> all initialization steps are complete:
> atmel_ecc_probe()
> list_add_tail(&i2c_priv->i2c_client_list_node, ...)
> ...
> ret = crypto_register_kpp(&atmel_ecdh_nist_p256);
> if (ret) {
> goto err_list_del;
> If multiple Atmel ECC devices are present, the second device will fail to
> register the same static algorithm and return an error.
> During this window, could a concurrent call to atmel_i2c_client_alloc()
> select this incompletely probed device for a new crypto transformation?
> If so, when the probe function hits the error path, removes the device from
> the list, and devres frees i2c_priv, the crypto transformation would be left
> with a dangling pointer, potentially causing a use-after-free on subsequent
> operations. This same pattern also appears to be present in
> atmel_sha204a_probe().
> > diff --git a/drivers/crypto/atmel-i2c.c b/drivers/crypto/atmel-i2c.c
> > --- a/drivers/crypto/atmel-i2c.c
> > +++ b/drivers/crypto/atmel-i2c.c
> [ ... ]
> > @@ -73,6 +73,9 @@ struct i2c_client *atmel_i2c_client_alloc(void)
> >
> > list_for_each_entry(i2c_priv, &atmel_i2c_mgmt.i2c_client_list,
> > i2c_client_list_node) {
> > + if (!(i2c_priv->caps & BIT(cap)))
> > + continue;
> > +
> > tfm_cnt = atomic_read(&i2c_priv->tfm_count);
> > if (tfm_cnt < min_tfm_cnt) {
> This problem wasn't introduced by this patch, but it seems active
Already mentioned...
> cryptographic transformations might not properly secure the lifetime of the
> underlying I2C device.
> The atmel_i2c_client_alloc() function increments a local tfm_count to track
> active references, but does not pin the device structure itself or delay its
> destruction.
> If a device is hot-unplugged or manually unbound via sysfs, the driver's
> remove callback executes. Since i2c_priv is tied to the device's lifecycle
> via devres, it will be automatically freed upon returning from the remove
> function. The unbind process does not block waiting for tfm_count to drop to
> zero.
> Does this mean any concurrently active crypto TFM previously allocated via
> atmel_i2c_client_alloc() will maintain a dangling pointer to these freed
> structures?
> When the TFM is later destroyed or performs an operation, it might
> dereference the freed client and execute atomic_dec() on freed memory.
The time-of-check to time-of-use situation should be addressed, as also the
multiple hardware devices competing for the algo structure.
Best,
L
^ permalink raw reply
* [PATCH v2 12/12] crypto: atmel-sha204a - switch to module_i2c_driver
From: Lothar Rubusch @ 2026-05-19 20:48 UTC (permalink / raw)
To: thorsten.blum, herbert, davem, nicolas.ferre, alexandre.belloni,
claudiu.beznea
Cc: linux-crypto, linux-arm-kernel, linux-kernel, l.rubusch
In-Reply-To: <20260519204803.17034-1-l.rubusch@gmail.com>
Replace custom module init/exit functions with module_i2c_driver() for
driver registration.
Update remove path to unregister the client from the shared I2C management
list before flushing pending work and cleaning up sysfs and hwrng
resources.
No functional change intended.
Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
drivers/crypto/atmel-sha204a.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/crypto/atmel-sha204a.c b/drivers/crypto/atmel-sha204a.c
index 3d29543032cc..c65630a989a5 100644
--- a/drivers/crypto/atmel-sha204a.c
+++ b/drivers/crypto/atmel-sha204a.c
@@ -257,18 +257,7 @@ static struct i2c_driver atmel_sha204a_driver = {
.driver.of_match_table = atmel_sha204a_dt_ids,
};
-static int __init atmel_sha204a_init(void)
-{
- return i2c_add_driver(&atmel_sha204a_driver);
-}
-
-static void __exit atmel_sha204a_exit(void)
-{
- i2c_del_driver(&atmel_sha204a_driver);
-}
-
-module_init(atmel_sha204a_init);
-module_exit(atmel_sha204a_exit);
+module_i2c_driver(atmel_sha204a_driver);
MODULE_AUTHOR("Ard Biesheuvel <ard.biesheuvel@linaro.org>");
MODULE_DESCRIPTION("Microchip / Atmel SHA204A (I2C) driver");
--
2.39.5
^ permalink raw reply related
* [PATCH v2 11/12] crypto: atmel-sha204a - fix heap info leak on I2C transfer failure
From: Lothar Rubusch @ 2026-05-19 20:48 UTC (permalink / raw)
To: thorsten.blum, herbert, davem, nicolas.ferre, alexandre.belloni,
claudiu.beznea
Cc: linux-crypto, linux-arm-kernel, linux-kernel, l.rubusch
In-Reply-To: <20260519204803.17034-1-l.rubusch@gmail.com>
When a non-blocking read operation is requested, the driver dynamically
allocates memory to track asynchronous transfer status. If the underlying
I2C transmission fails, atmel_sha204a_rng_done() logs a rate-limited
warning but incorrectly proceeds to cache the pointer to this uninitialized
buffer inside the rng->priv data field anyway.
On subsequent execution passes, atmel_sha204a_rng_read_nonblocking()
detects the stale rng->priv value, skips executing a hardware data read,
and copies up to 32 bytes of uninitialized kernel heap data from this
garbage memory pool straight back into the system's hwrng data stream.
Fix this information disclosure vector by immediately releasing the
allocated asynchronous work data buffer and explicitly clearing the
tracking pointer context whenever an I2C transaction returns a non-zero
error status.
Additionally, ensure that tfm counter is decremented within the error path
to prevent reference counter stagnation, which would otherwise leave the
driver in a permanently busy state. Finding by a sashiko side-review.
Fixes: da001fb651b0 ("crypto: atmel-i2c - add support for SHA204A random number generator")
Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
drivers/crypto/atmel-sha204a.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/crypto/atmel-sha204a.c b/drivers/crypto/atmel-sha204a.c
index 38a269186e2a..3d29543032cc 100644
--- a/drivers/crypto/atmel-sha204a.c
+++ b/drivers/crypto/atmel-sha204a.c
@@ -31,10 +31,15 @@ static void atmel_sha204a_rng_done(struct atmel_i2c_work_data *work_data,
struct atmel_i2c_client_priv *i2c_priv = work_data->ctx;
struct hwrng *rng = areq;
- if (status)
+ if (status) {
dev_warn_ratelimited(&i2c_priv->client->dev,
"i2c transaction failed (%d)\n",
status);
+ kfree(work_data);
+ rng->priv = 0;
+ atomic_dec(&i2c_priv->tfm_count);
+ return;
+ }
rng->priv = (unsigned long)work_data;
atomic_dec(&i2c_priv->tfm_count);
--
2.39.5
^ permalink raw reply related
* [PATCH v2 10/12] crypto: atmel-sha204a - integrate into core management tracking
From: Lothar Rubusch @ 2026-05-19 20:48 UTC (permalink / raw)
To: thorsten.blum, herbert, davem, nicolas.ferre, alexandre.belloni,
claudiu.beznea
Cc: linux-crypto, linux-arm-kernel, linux-kernel, l.rubusch
In-Reply-To: <20260519204803.17034-1-l.rubusch@gmail.com>
Register the SHA204A I2C device instance into the shared atmel_i2c client
management tracking list during the probe phase. This allows the driver to
participate in the central hardware selection infrastructure.
Rework the error-unwind paths inside atmel_sha204a_probe() to prevent stale
entries from remaining in the global tracking structures if a partial
initialization failure occurs. If sysfs group creation fails, explicitly
trigger devm_hwrng_unregister() to preserve the strict lifecycle ordering
introduced in previous stability fixes.
Convert the removal path to use the core teardown helpers. Ensure the
device readiness state is deactivated using atmel_i2c_deactivate_client()
before any local hardware cleanup runs, and call
atmel_i2c_unregister_client() at the end of the sequence to safely drop the
node from global tracking.
No functional change intended beyond improved lifecycle handling.
Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
drivers/crypto/atmel-sha204a.c | 31 +++++++++++++++++++++++++------
1 file changed, 25 insertions(+), 6 deletions(-)
diff --git a/drivers/crypto/atmel-sha204a.c b/drivers/crypto/atmel-sha204a.c
index 3853d2b95449..38a269186e2a 100644
--- a/drivers/crypto/atmel-sha204a.c
+++ b/drivers/crypto/atmel-sha204a.c
@@ -172,9 +172,15 @@ static int atmel_sha204a_probe(struct i2c_client *client)
return ret;
i2c_priv = i2c_get_clientdata(client);
+ i2c_priv->ready = false;
i2c_priv->caps = 0;
+ spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
+ list_add_tail(&i2c_priv->i2c_client_list_node,
+ &atmel_i2c_mgmt.i2c_client_list);
+ spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
+
memset(&i2c_priv->hwrng, 0, sizeof(i2c_priv->hwrng));
i2c_priv->hwrng.name = dev_name(&client->dev);
@@ -185,15 +191,28 @@ static int atmel_sha204a_probe(struct i2c_client *client)
i2c_priv->hwrng.quality = *quality;
ret = devm_hwrng_register(&client->dev, &i2c_priv->hwrng);
- if (ret)
+ if (ret) {
dev_warn(&client->dev, "failed to register RNG (%d)\n", ret);
+ goto err_list_del;
+ }
ret = sysfs_create_group(&client->dev.kobj, &atmel_sha204a_groups);
if (ret) {
dev_err(&client->dev, "failed to register sysfs entry\n");
- return ret;
+ goto err_hwrng_unregister;
}
+ spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
+ i2c_priv->ready = true;
+ spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
+
+ return 0;
+
+err_hwrng_unregister:
+ devm_hwrng_unregister(&client->dev, &i2c_priv->hwrng);
+err_list_del:
+ atmel_i2c_unregister_client(i2c_priv);
+
return ret;
}
@@ -201,12 +220,13 @@ static void atmel_sha204a_remove(struct i2c_client *client)
{
struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(client);
- devm_hwrng_unregister(&client->dev, &i2c_priv->hwrng);
- atmel_i2c_flush_queue();
+ atmel_i2c_deactivate_client(i2c_priv);
+ devm_hwrng_unregister(&client->dev, &i2c_priv->hwrng);
sysfs_remove_group(&client->dev.kobj, &atmel_sha204a_groups);
-
kfree((void *)i2c_priv->hwrng.priv);
+
+ atmel_i2c_unregister_client(i2c_priv);
}
static const struct of_device_id atmel_sha204a_dt_ids[] = {
@@ -239,7 +259,6 @@ static int __init atmel_sha204a_init(void)
static void __exit atmel_sha204a_exit(void)
{
- atmel_i2c_flush_queue();
i2c_del_driver(&atmel_sha204a_driver);
}
--
2.39.5
^ permalink raw reply related
* [PATCH v2 08/12] crypto: atmel-i2c - move shared client allocation logic to core
From: Lothar Rubusch @ 2026-05-19 20:47 UTC (permalink / raw)
To: thorsten.blum, herbert, davem, nicolas.ferre, alexandre.belloni,
claudiu.beznea
Cc: linux-crypto, linux-arm-kernel, linux-kernel, l.rubusch
In-Reply-To: <20260519204803.17034-1-l.rubusch@gmail.com>
Migrate the I2C client allocation and runtime load-balancing routines out
of the ECC driver code and into the central atmel-i2c core library module.
Export the symmetric lifecycle helper interfaces atmel_i2c_client_alloc()
and atmel_i2c_client_free() using EXPORT_SYMBOL_GPL() to expose a unified
client management API. This consolidation enables the dynamic selection
subsystem (which chooses the least-loaded client device based on the active
transformation count) to be shared by both the ECC driver and upcoming
Atmel crypto modules.
Refactor the ECC driver's transformation context initialization (init_tfm)
and teardown (exit_tfm) paths to use this centralized core API.
No functional change is intended.
Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
drivers/crypto/atmel-ecc.c | 50 +++-----------------------------------
drivers/crypto/atmel-i2c.c | 46 +++++++++++++++++++++++++++++++++++
drivers/crypto/atmel-i2c.h | 3 +++
3 files changed, 52 insertions(+), 47 deletions(-)
diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index 4f27e1caf106..7d090c557330 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -203,50 +203,6 @@ static int atmel_ecdh_compute_shared_secret(struct kpp_request *req)
return ret;
}
-static struct i2c_client *atmel_ecc_i2c_client_alloc(void)
-{
- struct atmel_i2c_client_priv *i2c_priv, *min_i2c_priv = NULL;
- struct i2c_client *client = ERR_PTR(-ENODEV);
- int min_tfm_cnt = INT_MAX;
- int tfm_cnt;
-
- spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
-
- if (list_empty(&atmel_i2c_mgmt.i2c_client_list)) {
- spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
- return ERR_PTR(-ENODEV);
- }
-
- list_for_each_entry(i2c_priv, &atmel_i2c_mgmt.i2c_client_list,
- i2c_client_list_node) {
- if (!i2c_priv->ready)
- continue;
- tfm_cnt = atomic_read(&i2c_priv->tfm_count);
- if (tfm_cnt < min_tfm_cnt) {
- min_tfm_cnt = tfm_cnt;
- min_i2c_priv = i2c_priv;
- }
- if (!min_tfm_cnt)
- break;
- }
-
- if (min_i2c_priv) {
- atomic_inc(&min_i2c_priv->tfm_count);
- client = min_i2c_priv->client;
- }
-
- spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
-
- return client;
-}
-
-static void atmel_ecc_i2c_client_free(struct i2c_client *client)
-{
- struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(client);
-
- atomic_dec(&i2c_priv->tfm_count);
-}
-
static int atmel_ecdh_init_tfm(struct crypto_kpp *tfm)
{
const char *alg = kpp_alg_name(tfm);
@@ -254,7 +210,7 @@ static int atmel_ecdh_init_tfm(struct crypto_kpp *tfm)
struct atmel_ecdh_ctx *ctx = kpp_tfm_ctx(tfm);
ctx->curve_id = ECC_CURVE_NIST_P256;
- ctx->client = atmel_ecc_i2c_client_alloc();
+ ctx->client = atmel_i2c_client_alloc();
if (IS_ERR(ctx->client)) {
pr_err("tfm - i2c_client binding failed\n");
return PTR_ERR(ctx->client);
@@ -264,7 +220,7 @@ static int atmel_ecdh_init_tfm(struct crypto_kpp *tfm)
if (IS_ERR(fallback)) {
dev_err(&ctx->client->dev, "Failed to allocate transformation for '%s': %ld\n",
alg, PTR_ERR(fallback));
- atmel_ecc_i2c_client_free(ctx->client);
+ atmel_i2c_client_free(ctx->client);
return PTR_ERR(fallback);
}
@@ -280,7 +236,7 @@ static void atmel_ecdh_exit_tfm(struct crypto_kpp *tfm)
kfree(ctx->public_key);
crypto_free_kpp(ctx->fallback);
- atmel_ecc_i2c_client_free(ctx->client);
+ atmel_i2c_client_free(ctx->client);
}
static unsigned int atmel_ecdh_max_size(struct crypto_kpp *tfm)
diff --git a/drivers/crypto/atmel-i2c.c b/drivers/crypto/atmel-i2c.c
index c73ef3cadf0e..4621aa57833f 100644
--- a/drivers/crypto/atmel-i2c.c
+++ b/drivers/crypto/atmel-i2c.c
@@ -57,6 +57,52 @@ static void atmel_i2c_checksum(struct atmel_i2c_cmd *cmd)
*__crc16 = cpu_to_le16(bitrev16(crc16(0, data, len)));
}
+struct i2c_client *atmel_i2c_client_alloc(void)
+{
+ struct atmel_i2c_client_priv *i2c_priv, *min_i2c_priv = NULL;
+ struct i2c_client *client = ERR_PTR(-ENODEV);
+ int min_tfm_cnt = INT_MAX;
+ int tfm_cnt;
+
+ spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
+
+ if (list_empty(&atmel_i2c_mgmt.i2c_client_list)) {
+ spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
+ return ERR_PTR(-ENODEV);
+ }
+
+ list_for_each_entry(i2c_priv, &atmel_i2c_mgmt.i2c_client_list,
+ i2c_client_list_node) {
+ if (!i2c_priv->ready)
+ continue;
+ tfm_cnt = atomic_read(&i2c_priv->tfm_count);
+ if (tfm_cnt < min_tfm_cnt) {
+ min_tfm_cnt = tfm_cnt;
+ min_i2c_priv = i2c_priv;
+ }
+ if (!min_tfm_cnt)
+ break;
+ }
+
+ if (min_i2c_priv) {
+ atomic_inc(&min_i2c_priv->tfm_count);
+ client = min_i2c_priv->client;
+ }
+
+ spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
+
+ return client;
+}
+EXPORT_SYMBOL_GPL(atmel_i2c_client_alloc);
+
+void atmel_i2c_client_free(struct i2c_client *client)
+{
+ struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(client);
+
+ atomic_dec(&i2c_priv->tfm_count);
+}
+EXPORT_SYMBOL_GPL(atmel_i2c_client_free);
+
void atmel_i2c_init_read_config_cmd(struct atmel_i2c_cmd *cmd)
{
cmd->word_addr = COMMAND;
diff --git a/drivers/crypto/atmel-i2c.h b/drivers/crypto/atmel-i2c.h
index 351306c426aa..6c2d86fd9068 100644
--- a/drivers/crypto/atmel-i2c.h
+++ b/drivers/crypto/atmel-i2c.h
@@ -192,6 +192,9 @@ void atmel_i2c_init_genkey_cmd(struct atmel_i2c_cmd *cmd, u16 keyid);
int atmel_i2c_init_ecdh_cmd(struct atmel_i2c_cmd *cmd,
struct scatterlist *pubkey);
+struct i2c_client *atmel_i2c_client_alloc(void);
+void atmel_i2c_client_free(struct i2c_client *client);
+
void atmel_i2c_deactivate_client(struct atmel_i2c_client_priv *i2c_priv);
void atmel_i2c_unregister_client(struct atmel_i2c_client_priv *i2c_priv);
--
2.39.5
^ permalink raw reply related
* [PATCH v2 09/12] crypto: atmel-i2c - implement capability-based client selection
From: Lothar Rubusch @ 2026-05-19 20:48 UTC (permalink / raw)
To: thorsten.blum, herbert, davem, nicolas.ferre, alexandre.belloni,
claudiu.beznea
Cc: linux-crypto, linux-arm-kernel, linux-kernel, l.rubusch
In-Reply-To: <20260519204803.17034-1-l.rubusch@gmail.com>
Extend the shared I2C client allocation interface to support feature-aware
hardware selection by introducing capability filtering.
Add a 'caps' mask to 'struct atmel_i2c_client_priv' alongside an
'atmel_i2c_capability' enum. The allocator now explicitly filters hardware
nodes by a requested capability bit while retaining the least-loaded device
load-balancing scheme.
Update the ECC driver to advertise ATMEL_CAP_ECDH configuration capability
during probe, and adapt the tfm context setup execution path to request
this specific capability variant. Initialize the bitmask field to zero
inside the SHA204A driver context for now.
Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
drivers/crypto/atmel-ecc.c | 4 +++-
drivers/crypto/atmel-i2c.c | 6 +++++-
drivers/crypto/atmel-i2c.h | 8 +++++++-
drivers/crypto/atmel-sha204a.c | 2 ++
4 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index 7d090c557330..11ee8ef71b94 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -210,7 +210,7 @@ static int atmel_ecdh_init_tfm(struct crypto_kpp *tfm)
struct atmel_ecdh_ctx *ctx = kpp_tfm_ctx(tfm);
ctx->curve_id = ECC_CURVE_NIST_P256;
- ctx->client = atmel_i2c_client_alloc();
+ ctx->client = atmel_i2c_client_alloc(ATMEL_CAP_ECDH);
if (IS_ERR(ctx->client)) {
pr_err("tfm - i2c_client binding failed\n");
return PTR_ERR(ctx->client);
@@ -283,6 +283,8 @@ static int atmel_ecc_probe(struct i2c_client *client)
i2c_priv = i2c_get_clientdata(client);
i2c_priv->ready = false;
+ i2c_priv->caps = BIT(ATMEL_CAP_ECDH);
+
spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
list_add_tail(&i2c_priv->i2c_client_list_node,
&atmel_i2c_mgmt.i2c_client_list);
diff --git a/drivers/crypto/atmel-i2c.c b/drivers/crypto/atmel-i2c.c
index 4621aa57833f..e6eeba1f6554 100644
--- a/drivers/crypto/atmel-i2c.c
+++ b/drivers/crypto/atmel-i2c.c
@@ -57,7 +57,7 @@ static void atmel_i2c_checksum(struct atmel_i2c_cmd *cmd)
*__crc16 = cpu_to_le16(bitrev16(crc16(0, data, len)));
}
-struct i2c_client *atmel_i2c_client_alloc(void)
+struct i2c_client *atmel_i2c_client_alloc(enum atmel_i2c_capability cap)
{
struct atmel_i2c_client_priv *i2c_priv, *min_i2c_priv = NULL;
struct i2c_client *client = ERR_PTR(-ENODEV);
@@ -75,6 +75,10 @@ struct i2c_client *atmel_i2c_client_alloc(void)
i2c_client_list_node) {
if (!i2c_priv->ready)
continue;
+
+ if (!(i2c_priv->caps & BIT(cap)))
+ continue;
+
tfm_cnt = atomic_read(&i2c_priv->tfm_count);
if (tfm_cnt < min_tfm_cnt) {
min_tfm_cnt = tfm_cnt;
diff --git a/drivers/crypto/atmel-i2c.h b/drivers/crypto/atmel-i2c.h
index 6c2d86fd9068..636d21bd1348 100644
--- a/drivers/crypto/atmel-i2c.h
+++ b/drivers/crypto/atmel-i2c.h
@@ -115,6 +115,10 @@ struct atmel_i2c_cmd {
#define ECDH_PREFIX_MODE 0x00
/* Used for binding tfm objects to i2c clients. */
+enum atmel_i2c_capability {
+ ATMEL_CAP_ECDH = 0,
+};
+
struct atmel_i2c_client_mgmt {
struct list_head i2c_client_list;
spinlock_t i2c_list_lock;
@@ -131,6 +135,7 @@ extern struct atmel_i2c_client_mgmt atmel_i2c_mgmt;
* @tfm_count : number of active crypto transformations on i2c client
* @hwrng : hold the hardware generated rng
* @ready : hw client is ready to use
+ * @caps : feature capability of the particular driver
*
* Reads and writes from/to the i2c client are sequential. The first byte
* transmitted to the device is treated as the byte size. Any attempt to send
@@ -148,6 +153,7 @@ struct atmel_i2c_client_priv {
atomic_t tfm_count ____cacheline_aligned;
struct hwrng hwrng;
bool ready;
+ u32 caps;
};
/**
@@ -192,7 +198,7 @@ void atmel_i2c_init_genkey_cmd(struct atmel_i2c_cmd *cmd, u16 keyid);
int atmel_i2c_init_ecdh_cmd(struct atmel_i2c_cmd *cmd,
struct scatterlist *pubkey);
-struct i2c_client *atmel_i2c_client_alloc(void);
+struct i2c_client *atmel_i2c_client_alloc(enum atmel_i2c_capability cap);
void atmel_i2c_client_free(struct i2c_client *client);
void atmel_i2c_deactivate_client(struct atmel_i2c_client_priv *i2c_priv);
diff --git a/drivers/crypto/atmel-sha204a.c b/drivers/crypto/atmel-sha204a.c
index 6e6ac4770416..3853d2b95449 100644
--- a/drivers/crypto/atmel-sha204a.c
+++ b/drivers/crypto/atmel-sha204a.c
@@ -173,6 +173,8 @@ static int atmel_sha204a_probe(struct i2c_client *client)
i2c_priv = i2c_get_clientdata(client);
+ i2c_priv->caps = 0;
+
memset(&i2c_priv->hwrng, 0, sizeof(i2c_priv->hwrng));
i2c_priv->hwrng.name = dev_name(&client->dev);
--
2.39.5
^ permalink raw reply related
* [PATCH v2 07/12] crypto: atmel-ecc - switch to module_i2c_driver
From: Lothar Rubusch @ 2026-05-19 20:47 UTC (permalink / raw)
To: thorsten.blum, herbert, davem, nicolas.ferre, alexandre.belloni,
claudiu.beznea
Cc: linux-crypto, linux-arm-kernel, linux-kernel, l.rubusch
In-Reply-To: <20260519204803.17034-1-l.rubusch@gmail.com>
Remove custom boilerplate module configuration code and convert the module
init/exit paths to use the modern module_i2c_driver() helper macro.
This shortens and simplifies driver initialization. Custom structure setup
is no longer required here since management tracking context initialization
was already safely moved into the atmel-i2c core library module.
Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
drivers/crypto/atmel-ecc.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index 29706e4bfa04..4f27e1caf106 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -401,18 +401,7 @@ static struct i2c_driver atmel_ecc_driver = {
.id_table = atmel_ecc_id,
};
-static int __init atmel_ecc_init(void)
-{
- return i2c_add_driver(&atmel_ecc_driver);
-}
-
-static void __exit atmel_ecc_exit(void)
-{
- i2c_del_driver(&atmel_ecc_driver);
-}
-
-module_init(atmel_ecc_init);
-module_exit(atmel_ecc_exit);
+module_i2c_driver(atmel_ecc_driver);
MODULE_AUTHOR("Tudor Ambarus");
MODULE_DESCRIPTION("Microchip / Atmel ECC (I2C) driver");
--
2.39.5
^ permalink raw reply related
* [PATCH v2 06/12] crypto: atmel-i2c - introduce shared teardown helpers and fix queue flush
From: Lothar Rubusch @ 2026-05-19 20:47 UTC (permalink / raw)
To: thorsten.blum, herbert, davem, nicolas.ferre, alexandre.belloni,
claudiu.beznea
Cc: linux-crypto, linux-arm-kernel, linux-kernel, l.rubusch
In-Reply-To: <20260519204803.17034-1-l.rubusch@gmail.com>
Introduce atmel_i2c_deactivate_client() and atmel_i2c_unregister_client()
helpers in the atmel-i2c core library to modularize client teardown. This
encapsulates common client state tracking and list manipulation operations.
Convert the ECC driver's error recovery and device removal paths to utilize
these new helpers, ensuring consistent execution ordering when modifying
device-readiness states and deleting linked-list nodes.
Inside the unregistration helper, use list_empty_careful() to safely
validate the membership state of the individual node before triggering
list_del_init().
Additionally, migrate the atmel_i2c_flush_queue() call out of the module
exit path. It now runs inside the core unregistration helper under the
protection of the management spinlock. This configuration ensures the
shared workqueue is only flushed when the global client list becomes
completely empty, enabling proper scaling for multi-driver setups.
Export both new tracking symbols via EXPORT_SYMBOL_GPL() to match the
existing core driver licensing standard.
Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
drivers/crypto/atmel-ecc.c | 13 +++----------
drivers/crypto/atmel-i2c.c | 22 ++++++++++++++++++++++
drivers/crypto/atmel-i2c.h | 3 +++
3 files changed, 28 insertions(+), 10 deletions(-)
diff --git a/drivers/crypto/atmel-ecc.c b/drivers/crypto/atmel-ecc.c
index 33b90667c872..29706e4bfa04 100644
--- a/drivers/crypto/atmel-ecc.c
+++ b/drivers/crypto/atmel-ecc.c
@@ -336,9 +336,7 @@ static int atmel_ecc_probe(struct i2c_client *client)
if (atmel_ecc_kpp_refcnt == 0) {
ret = crypto_register_kpp(&atmel_ecdh_nist_p256);
if (ret) {
- spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
- list_del(&i2c_priv->i2c_client_list_node);
- spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
+ atmel_i2c_unregister_client(i2c_priv);
dev_err(&client->dev, "%s alg registration failed\n",
atmel_ecdh_nist_p256.base.cra_driver_name);
@@ -363,9 +361,7 @@ static void atmel_ecc_remove(struct i2c_client *client)
{
struct atmel_i2c_client_priv *i2c_priv = i2c_get_clientdata(client);
- spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
- i2c_priv->ready = false;
- spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
+ atmel_i2c_deactivate_client(i2c_priv);
/*
* Note, the Linux Crypto Core automatically blocks until all active
@@ -378,9 +374,7 @@ static void atmel_ecc_remove(struct i2c_client *client)
crypto_unregister_kpp(&atmel_ecdh_nist_p256);
mutex_unlock(&atmel_ecc_kpp_lock);
- spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
- list_del(&i2c_priv->i2c_client_list_node);
- spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
+ atmel_i2c_unregister_client(i2c_priv);
}
static const struct of_device_id atmel_ecc_dt_ids[] = {
@@ -414,7 +408,6 @@ static int __init atmel_ecc_init(void)
static void __exit atmel_ecc_exit(void)
{
- atmel_i2c_flush_queue();
i2c_del_driver(&atmel_ecc_driver);
}
diff --git a/drivers/crypto/atmel-i2c.c b/drivers/crypto/atmel-i2c.c
index db24f65ae90e..c73ef3cadf0e 100644
--- a/drivers/crypto/atmel-i2c.c
+++ b/drivers/crypto/atmel-i2c.c
@@ -354,6 +354,28 @@ static int device_sanity_check(struct i2c_client *client)
return ret;
}
+void atmel_i2c_deactivate_client(struct atmel_i2c_client_priv *i2c_priv)
+{
+ spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
+ i2c_priv->ready = false;
+ spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
+}
+EXPORT_SYMBOL_GPL(atmel_i2c_deactivate_client);
+
+void atmel_i2c_unregister_client(struct atmel_i2c_client_priv *i2c_priv)
+{
+ spin_lock(&atmel_i2c_mgmt.i2c_list_lock);
+
+ if (!list_empty_careful(&i2c_priv->i2c_client_list_node))
+ list_del_init(&i2c_priv->i2c_client_list_node);
+
+ if (list_empty(&atmel_i2c_mgmt.i2c_client_list))
+ atmel_i2c_flush_queue();
+
+ spin_unlock(&atmel_i2c_mgmt.i2c_list_lock);
+}
+EXPORT_SYMBOL_GPL(atmel_i2c_unregister_client);
+
int atmel_i2c_probe(struct i2c_client *client)
{
struct atmel_i2c_client_priv *i2c_priv;
diff --git a/drivers/crypto/atmel-i2c.h b/drivers/crypto/atmel-i2c.h
index d54bd836e0f5..351306c426aa 100644
--- a/drivers/crypto/atmel-i2c.h
+++ b/drivers/crypto/atmel-i2c.h
@@ -192,4 +192,7 @@ void atmel_i2c_init_genkey_cmd(struct atmel_i2c_cmd *cmd, u16 keyid);
int atmel_i2c_init_ecdh_cmd(struct atmel_i2c_cmd *cmd,
struct scatterlist *pubkey);
+void atmel_i2c_deactivate_client(struct atmel_i2c_client_priv *i2c_priv);
+void atmel_i2c_unregister_client(struct atmel_i2c_client_priv *i2c_priv);
+
#endif /* __ATMEL_I2C_H__ */
--
2.39.5
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox