Clean and improve SA2UL driver
v2 cover Clean and improve SA2UL driver
The following series focuses on some cleanups to the sa2ul driver, it starts with very trivial cleanups then goes to fix some bigger cleanups and migration to crypto_engine which help fix the broader race condition problems with the driver, thereafter it adds the support for AES CM as per SAxUL 3.1 IP. KASAN is also run alongside to fix the issues caught by them.
https://gist.github.com/manorit2001/0c005603f1a2da0e82c760e209ab0c4a
Signed-off-by Manorit Chawdhry <[email protected]>
---
Changes in v2:
* Sashiko (visible web review)
- Improve some commit descriptions
- Map contexts at allocation and remove the redundant mempool allocation check
- Prime AES-CM before registering algorithms; fail probe if priming fails
- Wipe the temporary AES-CM round key
* Sashiko (API-only concerns) [0]
- Make the probe register mapping local
- Validate context IDs and AES keys
- Check scatterlist errors and handle unaligned IVs
- Synchronize DMA using the recorded mapping direction
* Pratham
- Use fixed hash/skcipher request sizes
- Migrate to init_tfm/exit_tfm
* Self
- Rebase onto linux-next
- Remove Assisted-by tags
- Drop few patches that are irrelevant after rebase
- Remove redundant algorithm IDs while preserving digest sizes
- Remove unused SA_MAX_AUX_DATA_WORDS
- Preserve full authentication-key lengths
- Propagate DMA completion errors in a separate fix
- Unwind fallback transforms and security contexts on init failure
- Stop the engine and quiesce DMA once before teardown
Link to v1: https://lore.kernel.org/all/[email protected]
[0] https://sashiko.dev/api/patchset?id=20260915-b4-upstream-sa2ul-cleanup-v1-0-57ab34e97162%40ti.com
---
Manorit Chawdhry (30):
crypto: sa2ul - remove dead code
crypto: sa2ul - remove totally unused structure fields
crypto: sa2ul - remove unused fields from sa_cmdl_cfg
crypto: sa2ul - remove unused context fields
crypto: sa2ul - remove unused macro definitions
crypto: sa2ul - remove unused base register pointer
crypto: sa2ul - remove unused includes and defines
crypto: sa2ul - remove unused line
crypto: sa2ul - remove redundant sa_sha_digest() wrapper
crypto: sa2ul - fix struct documentation for match_data
crypto: sa2ul - keep the probe register mapping local
crypto: sa2ul - zero out security context on free
crypto: sa2ul - preserve full authentication key lengths
crypto: sa2ul - validate AES key sizes before indexing
crypto: sa2ul - remove unused SC ID range tracking
crypto: sa2ul - copy command-label inputs locally
crypto: sa2ul - consolidate encryption offset definitions
crypto: sa2ul - handle unaligned IV buffers
crypto: sa2ul - remove redundant algorithm ID fields
crypto: sa2ul - handle transform initialization failures
crypto: sa2ul - reject exhausted context IDs
crypto: sa2ul - validate scatterlist setup and unwind DMA mappings
crypto: sa2ul - propagate DMA transfer errors
crypto: sa2ul - calculate DMA metadata length from the command label
crypto: sa2ul - correct scatterlist DMA ownership transitions
crypto: sa2ul - use a mempool for security contexts
crypto: sa2ul - use synchronous fallbacks and fixed request sizes
crypto: sa2ul - route requests through crypto_engine
crypto: sa2ul - report SA engine hardware revision
crypto: sa2ul - add SA3UL_CM AES support
drivers/crypto/Kconfig | 1 + drivers/crypto/sa2ul.c | 1265 +++++++++++++++++++++++++++++++----------------- drivers/crypto/sa2ul.h | 172 ++----- 3 files changed, 854 insertions(+), 584 deletions(-)
---
base-commit: 7649ba73f9e7e1e04d0b46213e39c79ae09dd55c
change-id: 20251106-b4-upstream-sa2ul-cleanup-ce85d40c7eb8
Best regards,
Signature · 1 line
Manorit Chawdhry <[email protected]>
PATCH v2 1/30 crypto: sa2ul - remove dead code
The following code path is not used at all, as SA_CMDL_UPD_AUTH_IV and SA_CMDL_UPD_AUX_KEY are never set. Remove it.
Signed-off-by Manorit Chawdhry <[email protected]>
--- drivers/crypto/sa2ul.c | 24 ------------------------ 1 file changed, 24 deletions(-)
drivers/crypto/sa2ul.c +0 -24
@@ -41,8 +41,6 @@#define SA_CMDL_UPD_ENC 0x0001#define SA_CMDL_UPD_AUTH 0x0002#define SA_CMDL_UPD_ENC_IV 0x0004 -#define SA_CMDL_UPD_AUTH_IV 0x0008 -#define SA_CMDL_UPD_AUX_KEY 0x0010#define SA_AUTH_SUBKEY_LEN 16#define SA_CMDL_PAYLOAD_LENGTH_MASK 0xFFFF
@@ -496,17 +494,6 @@ static void sa_set_sc_auth(struct algo_data *ad, const u8 *key, u16 key_sz, } } -static inline void sa_copy_iv (__be32 *out,const u8 *iv,bool size16) -{ -int j; - -for (j =0 ; j < ((size16) ?4 :2 ); j++) { - *out =cpu_to_be32 (*((u32 *)iv)); - iv +=4 ; - out++; - } -} -/* Format general command label */ static int sa_format_cmdl_gen (struct sa_cmdl_cfg *cfg,u8 *cmdl,struct sa_cmdl_upd_info *upd_info)
@@ -615,17 +602,6 @@ static inline void sa_update_cmdl(struct sa_req *req, u32 *cmdl, cmdl[upd_info->auth_offset.index] |=FIELD_PREP (SA_CMDL_SOP_BYPASS_LEN_MASK , req->auth_offset); -if (upd_info->flags &SA_CMDL_UPD_AUTH_IV ) { -sa_copy_iv ((void *)&cmdl[upd_info->auth_iv.index], - req->auth_iv, - (upd_info->auth_iv.size >8 )); - } -if (upd_info->flags &SA_CMDL_UPD_AUX_KEY ) { -int offset = (req->auth_size &0xF ) ?4 :0 ; - -memcpy (&cmdl[upd_info->aux_key_info.index], - &upd_info->aux_key[offset],16 ); - } } }
Signature · 1 line
2.43.0
PATCH v2 2/30 crypto: sa2ul - remove totally unused structure fields
In reply to Manorit ChawdhryThe following have never been used in code. Remove them.
Signed-off-by Manorit Chawdhry <[email protected]>
--- drivers/crypto/sa2ul.h | 46 ---------------------------------------------- 1 file changed, 46 deletions(-)
drivers/crypto/sa2ul.h +0 -46
@@ -222,43 +222,24 @@ struct sa_cmdl_param_info {u16 size; }; -/* Maximum length of Auxiliary data in 32bit words */ -#define SA_MAX_AUX_DATA_WORDS 8 -/** * struct sa_cmdl_upd_info: Command label updation info * @flags: flags in command label * @submode: Encryption submodes * @enc_size: Size of first pass encryption size -* @enc_size2: Size of second pass encryption size * @enc_offset: Encryption payload offset in the packet * @enc_iv: Encryption initialization vector for pass2 -* @enc_iv2: Encryption initialization vector for pass2 -* @aad: Associated data -* @payload: Payload info * @auth_size: Authentication size for pass 1 -* @auth_size2: Authentication size for pass 2 * @auth_offset: Authentication payload offset -* @auth_iv: Authentication initialization vector -* @aux_key_info: Authentication aux key information -* @aux_key: Aux key for authentication */ struct sa_cmdl_upd_info {u16 flags;u16 submode;struct sa_cmdl_param_info enc_size; -struct sa_cmdl_param_info enc_size2;struct sa_cmdl_param_info enc_offset;struct sa_cmdl_param_info enc_iv; -struct sa_cmdl_param_info enc_iv2; -struct sa_cmdl_param_info aad; -struct sa_cmdl_param_info payload;struct sa_cmdl_param_info auth_size; -struct sa_cmdl_param_info auth_size2;struct sa_cmdl_param_info auth_offset; -struct sa_cmdl_param_info auth_iv; -struct sa_cmdl_param_info aux_key_info; -u32 aux_key[SA_MAX_AUX_DATA_WORDS ]; };/*
@@ -330,13 +311,6 @@ struct sa_sha_req_ctx {struct ahash_request fallback_req; }; -enum sa_submode { -SA_MODE_GEN =0 , -SA_MODE_CCM , -SA_MODE_GCM , -SA_MODE_GMAC -}; -/* Encryption algorithms */ enum sa_ealg_id {SA_EALG_ID_NONE =0 ,/* No encryption */
@@ -371,26 +345,6 @@ enum sa_aalg_id { SA_AALG_ID_AES_XCBC /* AES Extended Cipher Block Chaining */ }; -/* -* Mode control engine algorithms used to index the -* mode control instruction tables -*/ -enum sa_eng_algo_id { -SA_ENG_ALGO_ECB =0 , -SA_ENG_ALGO_CBC , -SA_ENG_ALGO_CFB , -SA_ENG_ALGO_OFB , -SA_ENG_ALGO_CTR , -SA_ENG_ALGO_F8 , -SA_ENG_ALGO_F8F9 , -SA_ENG_ALGO_GCM , -SA_ENG_ALGO_GMAC , -SA_ENG_ALGO_CCM , -SA_ENG_ALGO_CMAC , -SA_ENG_ALGO_CBCMAC , -SA_NUM_ENG_ALGOS -}; -/** * struct sa_eng_info: Security accelerator engine info * @eng_id: Engine ID
Signature · 1 line
2.43.0
PATCH v2 3/30 crypto: sa2ul - remove unused fields from sa_cmdl_cfg
In reply to Manorit ChawdhryThese are assigned but never used. Remove them.
Signed-off-by Manorit Chawdhry <[email protected]>
--- drivers/crypto/sa2ul.c | 8 -------- 1 file changed, 8 deletions(-)
drivers/crypto/sa2ul.c +0 -8
@@ -96,8 +96,6 @@ static struct device *sa_k3_dev;* @enc_eng_id: Encryption Engine ID supported by the SA hardware * @auth_eng_id: Authentication Engine ID * @iv_size: Initialization Vector size -* @akey: Authentication key -* @akey_len: Authentication key length * @enc: True, if this is an encode request */ struct sa_cmdl_cfg {
@@ -105,8 +103,6 @@ struct sa_cmdl_cfg {u8 enc_eng_id;u8 auth_eng_id;u8 iv_size; -const u8 *akey; -u16 akey_len;bool enc; };
@@ -1376,8 +1372,6 @@ static int sa_sha_setup(struct sa_tfm_ctx *ctx, struct algo_data *ad) cfg.enc_eng_id = ad->enc_eng.eng_id; cfg.auth_eng_id = ad->auth_eng.eng_id; cfg.iv_size =0 ; - cfg.akey =NULL ; - cfg.akey_len =0 ; ctx->dev_data =dev_get_drvdata (sa_k3_dev);/* Setup Encryption Security Context & Command label template */
@@ -1703,8 +1697,6 @@ static int sa_aead_setkey(struct crypto_aead *authenc, cfg.enc_eng_id = ad->enc_eng.eng_id; cfg.auth_eng_id = ad->auth_eng.eng_id; cfg.iv_size =crypto_aead_ivsize (authenc); - cfg.akey = keys.authkey; - cfg.akey_len = keys.authkeylen;/* Setup Encryption Security Context & Command label template */ if (sa_init_sc(&ctx->enc, ctx->dev_data->match_data, keys.enckey,
Signature · 1 line
2.43.0
PATCH v2 4/30 crypto: sa2ul - remove unused context fields
In reply to Manorit ChawdhryRemove unused context fields, their assignments and kernel-doc.
Signed-off-by Manorit Chawdhry <[email protected]>
--- drivers/crypto/sa2ul.h | 4 ---- 1 file changed, 4 deletions(-)
drivers/crypto/sa2ul.h +0 -4
@@ -279,18 +279,14 @@ struct sa_ctx_info {* @dev_data: struct sa_crypto_data pointer * @enc: struct sa_ctx_info for encryption * @dec: struct sa_ctx_info for decryption -* @keylen: encrption/decryption keylength * @iv_idx: Initialization vector index -* @key: encryption key * @fallback: SW fallback algorithm */ struct sa_tfm_ctx {struct sa_crypto_data *dev_data;struct sa_ctx_info enc;struct sa_ctx_info dec; -int keylen;int iv_idx; -u32 key[AES_KEYSIZE_256 /sizeof (u32)];/* for fallback */ union {struct crypto_skcipher *skcipher;
Signature · 1 line
2.43.0
PATCH v2 5/30 crypto: sa2ul - remove unused macro definitions
In reply to Manorit ChawdhrySA_AUTH_SUBKEY_LEN and NKEY_SZ have no users. Remove the definitions.
Signed-off-by Manorit Chawdhry <[email protected]>
--- drivers/crypto/sa2ul.c | 1 - drivers/crypto/sa2ul.h | 1 - 2 files changed, 2 deletions(-)
drivers/crypto/sa2ul.c +0 -1
@@ -42,7 +42,6 @@#define SA_CMDL_UPD_AUTH 0x0002#define SA_CMDL_UPD_ENC_IV 0x0004 -#define SA_AUTH_SUBKEY_LEN 16#define SA_CMDL_PAYLOAD_LENGTH_MASK 0xFFFF#define SA_CMDL_SOP_BYPASS_LEN_MASK 0xFF000000
drivers/crypto/sa2ul.h +0 -1
@@ -46,7 +46,6 @@ struct sa_tfm_ctx;/* Number of 32 bit words in PS data */ #define SA_DMA_NUM_PS_WORDS 16 -#define NKEY_SZ 3#define MCI_SZ 27/*
Signature · 1 line
2.43.0
PATCH v2 6/30 crypto: sa2ul - remove unused base register pointer
In reply to Manorit ChawdhryProbe accesses registers through saul_base. Remove the unused device-context base member and its assignment.
Signed-off-by Manorit Chawdhry <[email protected]>
--- drivers/crypto/sa2ul.c | 1 - drivers/crypto/sa2ul.h | 2 -- 2 files changed, 3 deletions(-)
drivers/crypto/sa2ul.c +0 -1
@@ -2275,7 +2275,6 @@ static int sa_ul_probe(struct platform_device *pdev) sa_k3_dev = dev; dev_data->dev = dev; dev_data->pdev = pdev; - dev_data->base = saul_base;platform_set_drvdata (pdev, dev_data);dev_set_drvdata (sa_k3_dev, dev_data);
drivers/crypto/sa2ul.h +0 -2
@@ -175,7 +175,6 @@ struct sa_match_data;/** * struct sa_crypto_data - Crypto driver instance data -* @base: Base address of the register space * @soc_data: Pointer to SoC specific data * @pdev: Platform device pointer * @sc_pool: security context pool
@@ -191,7 +190,6 @@ struct sa_match_data;* @dma_tx: Pointer to DMA TX channel */ struct sa_crypto_data { -void __iomem *base;const struct sa_match_data *match_data;struct platform_device *pdev;struct dma_pool *sc_pool;
Signature · 1 line
2.43.0
PATCH v2 7/30 crypto: sa2ul - remove unused includes and defines
In reply to Manorit ChawdhryRemove unused includes and define statements that are not referenced anywhere in the driver.
Signed-off-by Manorit Chawdhry <[email protected]>
--- drivers/crypto/sa2ul.c | 13 ------------- drivers/crypto/sa2ul.h | 10 ---------- 2 files changed, 23 deletions(-)
drivers/crypto/sa2ul.c +0 -13
@@ -9,7 +9,6 @@* Tero Kristo */ #include <linux/bitfield.h> -#include <linux/clk.h> #include <linux/dma-mapping.h> #include <linux/dmaengine.h> #include <linux/dmapool.h>
@@ -58,14 +57,6 @@#define SA_SW2_EGRESS_LENGTH 0xFF000000#define SA_BASIC_HASH 0x10 -#define SHA256_DIGEST_WORDS 8 -/* Make 32-bit word from 4 bytes */ -#define SA_MK_U32 (b0, b1, b2, b3) (((b0) << 24) | ((b1) << 16) | \ - ((b2) << 8) | (b3)) - -/* size of SCCTL structure in bytes */ -#define SA_SCCTL_SZ 16 -/* Max Authentication tag size */ #define SA_MAX_AUTH_TAG_SZ 64
@@ -188,14 +179,12 @@ struct sa_rx_data {/** * struct sa_req: SA request definition -* @dev: device for the request * @size: total data to the xmitted via DMA * @enc_offset: offset of cipher data * @enc_size: data to be passed to cipher engine * @enc_iv: cipher IV * @auth_offset: offset of the authentication data * @auth_size: size of the authentication data -* @auth_iv: authentication IV * @type: algorithm type for the request * @cmdl: command label pointer * @base: pointer to the base request
@@ -207,14 +196,12 @@ struct sa_rx_data {* @mdata_size: metadata size passed to DMA */ struct sa_req { -struct device *dev;u16 size;u8 enc_offset;u16 enc_size;u8 *enc_iv;u8 auth_offset;u16 auth_size; -u8 *auth_iv;u32 type;u32 *cmdl;struct crypto_async_request *base;
drivers/crypto/sa2ul.h +0 -10
@@ -184,7 +184,6 @@ struct sa_match_data;* @sc_id_end: Ending index for SC ID * @sc_id: Security Context ID * @ctx_bm: Bitmap to keep track of Security context ID's -* @ctx: SA tfm context pointer * @dma_rx1: Pointer to DMA rx channel for sizes < 256 Bytes * @dma_rx2: Pointer to DMA rx channel for sizes > 256 Bytes * @dma_tx: Pointer to DMA TX channel
@@ -201,7 +200,6 @@ struct sa_crypto_data {u16 sc_id;unsigned long ctx_bm[DIV_ROUND_UP (SA_MAX_NUM_CTX ,BITS_PER_LONG )]; -struct sa_tfm_ctx *ctx;struct dma_chan *dma_rx1;struct dma_chan *dma_rx2;struct dma_chan *dma_tx;
@@ -210,19 +208,16 @@ struct sa_crypto_data {/** * struct sa_cmdl_param_info: Command label parameters info * @index: Index of the parameter in the command label format -* @offset: the offset of the parameter * @size: Size of the parameter */ struct sa_cmdl_param_info {u16 index; -u16 offset;u16 size; };/** * struct sa_cmdl_upd_info: Command label updation info * @flags: flags in command label -* @submode: Encryption submodes * @enc_size: Size of first pass encryption size * @enc_offset: Encryption payload offset in the packet * @enc_iv: Encryption initialization vector for pass2
@@ -231,7 +226,6 @@ struct sa_cmdl_param_info {*/ struct sa_cmdl_upd_info {u16 flags; -u16 submode;struct sa_cmdl_param_info enc_size;struct sa_cmdl_param_info enc_offset;struct sa_cmdl_param_info enc_iv;
@@ -294,13 +288,9 @@ struct sa_tfm_ctx {/** * struct sa_sha_req_ctx: Structure used for sha request -* @dev_data: struct sa_crypto_data pointer -* @cmdl: Complete command label with psdata and epib included * @fallback_req: SW fallback request container */ struct sa_sha_req_ctx { -struct sa_crypto_data *dev_data; -u32 cmdl[SA_MAX_CMDL_WORDS +SA_PSDATA_CTX_WORDS ];struct ahash_request fallback_req; };
Signature · 1 line
2.43.0
[email protected]
AI reviewRe: [PATCH v2 07/30] crypto: sa2ul - remove unused includes and defines
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] crypto: sa2ul - commit message omits struct field removals and leaves dead declarations --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] crypto: sa2ul - commit message omits struct field removals and leaves dead declarations --
commit ffe272048a90096ed2fcdf326cdc814515cd3d94
Author: Manorit Chawdhry <[email protected]>
crypto: sa2ul - remove unused includes and defines
This commit removes an unused header include, unused macro definitions, and unused structure fields in the SA2UL crypto driver.
Quoted 11 lines
crypto: sa2ul - remove unused includes and defines
Remove unused includes and define statements that are not referenced anywhere in the driver.
Signed-off-by Manorit Chawdhry <[email protected]>drivers/crypto/sa2ul.c +0
-0
[ ... ]
Quoted 19 lines
drivers/crypto/sa2ul.c +0
-2@@ -207,14 +196,12 @@ struct sa_rx_data {* @mdata_size: metadata size passed to DMA */ struct sa_req { -struct device *dev;u16 size;u8 enc_offset;u16 enc_size;u8 *enc_iv;u8 auth_offset;u16 auth_size; -u8 *auth_iv;u32 type;u32 *cmdl;struct crypto_async_request *base;drivers/crypto/sa2ul.h +0
-0
[ ... ]
Quoted 39 lines
drivers/crypto/sa2ul.h +0
-5@@ -201,7 +200,6 @@ struct sa_crypto_data {u16 sc_id;unsigned long ctx_bm[DIV_ROUND_UP (SA_MAX_NUM_CTX ,BITS_PER_LONG )]; -struct sa_tfm_ctx *ctx;struct dma_chan *dma_rx1;struct dma_chan *dma_rx2;struct dma_chan *dma_tx;@@ -210,19 +208,16 @@ struct sa_crypto_data {/** * struct sa_cmdl_param_info: Command label parameters info * @index: Index of the parameter in the command label format -* @offset: the offset of the parameter * @size: Size of the parameter */ struct sa_cmdl_param_info {u16 index; -u16 offset;u16 size; };/** * struct sa_cmdl_upd_info: Command label updation info * @flags: flags in command label -* @submode: Encryption submodes * @enc_size: Size of first pass encryption size * @enc_offset: Encryption payload offset in the packet * @enc_iv: Encryption initialization vector for pass2 @@ -231,7 +226,6 @@ struct sa_cmdl_param_info {*/ struct sa_cmdl_upd_info {u16 flags; -u16 submode;struct sa_cmdl_param_info enc_size;struct sa_cmdl_param_info enc_offset;struct sa_cmdl_param_info enc_iv;@@ -294,13 +288,9 @@ struct sa_tfm_ctx {/**
drivers/crypto/sa2ul.h struct sa_tfm_ctx { · line 295
Quoted 10 lines
drivers/crypto/sa2ul.h +0
-4* struct sa_sha_req_ctx: Structure used for sha request -* @dev_data: struct sa_crypto_data pointer -* @cmdl: Complete command label with psdata and epib included * @fallback_req: SW fallback request container */ struct sa_sha_req_ctx { -struct sa_crypto_data *dev_data; -u32 cmdl[SA_MAX_CMDL_WORDS +SA_PSDATA_CTX_WORDS ];struct ahash_request fallback_req; };
[Severity: Low]
This isn't a bug, but the commit title and message only mention removing unused includes and define statements, whereas all of the changes in drivers/crypto/sa2ul.h and part of drivers/crypto/sa2ul.c remove unused structure members from struct sa_req, struct sa_crypto_data, struct sa_cmdl_param_info, struct sa_cmdl_upd_info, and struct sa_sha_req_ctx.
Also, now that struct sa_tfm_ctx *ctx is removed from struct sa_crypto_data, the forward declaration near the top of drivers/crypto/sa2ul.h is no longer needed:
#define SA_ENGINE_STATUS 0x0008
#define SA_ENGINE_ENABLE_CONTROL 0x1000
struct sa_tfm_ctx;And in struct sa_req in drivers/crypto/sa2ul.c, the cmdl member is also never read or written anywhere in the driver.
Could the commit message be updated to mention removing unused struct members, and should the unused struct sa_tfm_ctx forward declaration and sa_req::cmdl field be removed as well?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7
PATCH v2 8/30 crypto: sa2ul - remove unused line
In reply to Manorit ChawdhryRemove an unused line from the driver.
Signed-off-by Manorit Chawdhry <[email protected]>
--- drivers/crypto/sa2ul.c | 1 - 1 file changed, 1 deletion(-)
drivers/crypto/sa2ul.c +0 -1
@@ -1181,7 +1181,6 @@ static int sa_run(struct sa_req *req) sizeof(u32))), cmdl, sizeof(sa_ctx->epib), sa_ctx->epib); - ml = sa_ctx->cmdl_size + (SA_PSDATA_CTX_WORDS *sizeof (u32));dmaengine_desc_set_metadata_len (tx_out, req->mdata_size);dmaengine_submit (tx_out);
Signature · 1 line
2.43.0
PATCH v2 9/30 crypto: sa2ul - remove redundant sa_sha_digest() wrapper
In reply to Manorit Chawdhrysa_sha_digest() only calls sa_sha_run(). Use sa_sha_run() directly.
Signed-off-by Manorit Chawdhry <[email protected]>
--- drivers/crypto/sa2ul.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)
drivers/crypto/sa2ul.c +3 -8
@@ -1413,11 +1413,6 @@ static int sa_sha_cra_init_alg(struct crypto_tfm *tfm, const char *alg_base)return 0 ; } -static int sa_sha_digest (struct ahash_request *req) -{ -return sa_sha_run (req); -} -static int sa_sha_init (struct ahash_request *req) {struct crypto_ahash *tfm =crypto_ahash_reqtfm (req);
@@ -1930,7 +1925,7 @@ static struct sa_alg_tmpl sa_algs[] = { .update = sa_sha_update, .final = sa_sha_final, .finup = sa_sha_finup, - .digest = sa_sha_digest, + .digest = sa_sha_run, .export = sa_sha_export, .import = sa_sha_import, },
@@ -1959,7 +1954,7 @@ static struct sa_alg_tmpl sa_algs[] = { .update = sa_sha_update, .final = sa_sha_final, .finup = sa_sha_finup, - .digest = sa_sha_digest, + .digest = sa_sha_run, .export = sa_sha_export, .import = sa_sha_import, },
@@ -1988,7 +1983,7 @@ static struct sa_alg_tmpl sa_algs[] = { .update = sa_sha_update, .final = sa_sha_final, .finup = sa_sha_finup, - .digest = sa_sha_digest, + .digest = sa_sha_run, .export = sa_sha_export, .import = sa_sha_import, },
Signature · 1 line
2.43.0
PATCH v2 10/30 crypto: sa2ul - fix struct documentation for match_data
In reply to Manorit ChawdhryThe device context stores SoC-specific information in match_data, but its kernel-doc still describes soc_data. Match the documentation to the existing member name.
Signed-off-by Manorit Chawdhry <[email protected]>
--- drivers/crypto/sa2ul.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/crypto/sa2ul.h +1 -1
@@ -175,7 +175,7 @@ struct sa_match_data;/** * struct sa_crypto_data - Crypto driver instance data -* @soc_data: Pointer to SoC specific data +* @match_data: Pointer to match data * @pdev: Platform device pointer * @sc_pool: security context pool * @dev: Device pointer
Signature · 1 line
2.43.0
PATCH v2 11/30 crypto: sa2ul - keep the probe register mapping local
In reply to Manorit ChawdhryA static mapping pointer is shared by concurrent probe calls, which can redirect register accesses to another device. Fix it even though there aren't multiple devices getting probed today in Linux.
Fixes 7694b6ca649f ("crypto: sa2ul - Add crypto driver")
Signed-off-by Manorit Chawdhry <[email protected]>
--- drivers/crypto/sa2ul.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/crypto/sa2ul.c +1 -1
@@ -2236,7 +2236,7 @@ static int sa_ul_probe(struct platform_device *pdev) {struct device *dev = &pdev->dev;struct device_node *node = dev->of_node; -static void __iomem *saul_base; +void __iomem *saul_base;struct sa_crypto_data *dev_data;u32 status, val;int ret;
Signature · 1 line
2.43.0
PATCH v2 12/30 crypto: sa2ul - zero out security context on free
In reply to Manorit ChawdhryZero out the security context memory before freeing to prevent potential information leakage of cryptographic keys and state.
Signed-off-by Manorit Chawdhry <[email protected]>
--- drivers/crypto/sa2ul.c | 1 + 1 file changed, 1 insertion(+)
drivers/crypto/sa2ul.c +1 -0
@@ -697,6 +697,7 @@ static void sa_free_ctx_info(struct sa_ctx_info *ctx,spin_unlock (&data->scid_lock);if (ctx->sc) { +memzero_explicit (ctx->sc,SA_CTX_MAX_SZ );dma_pool_free (data->sc_pool, ctx->sc, ctx->sc_phys); ctx->sc =NULL ; }
Signature · 1 line
2.43.0
PATCH v2 13/30 crypto: sa2ul - preserve full authentication key lengths
In reply to Manorit ChawdhryAuthentication keys come from the Crypto API with an unsigned-int length, but narrowing that length to u16 before HMAC preparation silently changes keys longer than 65535 bytes. Carry the full length through security context setup and ipad/opad preparation so long keys are hashed intact.
Fixes d2c8ac187fc9 ("crypto: sa2ul - Add AEAD algorithm support")
Signed-off-by Manorit Chawdhry <[email protected]>
--- drivers/crypto/sa2ul.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/crypto/sa2ul.c +5 -5
@@ -356,8 +356,8 @@ static void sa_swiz_128(u8 *in, u16 len) } } -static void sa_prepare_iopads (struct algo_data *data,const u8 *key,u16 key_sz, -__be32 *ipad,__be32 *opad) +static void sa_prepare_iopads (struct algo_data *data,const u8 *key, +unsigned int key_sz,__be32 *ipad,__be32 *opad) {union {struct hmac_sha1_key sha1;
@@ -455,8 +455,8 @@ static int sa_set_sc_enc(struct algo_data *ad, const u8 *key, u16 key_sz, }/* Set Security context for the authentication engine */ -static void sa_set_sc_auth (struct algo_data *ad,const u8 *key,u16 key_sz, -u8 *sc_buf) +static void sa_set_sc_auth (struct algo_data *ad,const u8 *key, +unsigned int key_sz,u8 *sc_buf) {__be32 *ipad = (void *)(sc_buf +32 );__be32 *opad = (void *)(sc_buf +64 );
@@ -619,7 +619,7 @@ static void sa_dump_sc(u8 *buf, dma_addr_t dma_addr)static int sa_init_sc (struct sa_ctx_info *ctx,const struct sa_match_data *match_data,const u8 *enc_key,u16 enc_key_sz, -const u8 *auth_key,u16 auth_key_sz, +const u8 *auth_key,unsigned int auth_key_sz,struct algo_data *ad,u8 enc,u32 *swinfo) {int enc_sc_offset =0 ;
Signature · 1 line
2.43.0
PATCH v2 14/30 crypto: sa2ul - validate AES key sizes before indexing
In reply to Manorit ChawdhryShort or malformed AES keys can produce a negative MCI table index. In the AEAD path this access happens before the common late AES-key check, so reject anything other than the supported AES key lengths before selecting the mode-control instruction.
Fixes 7694b6ca649f ("crypto: sa2ul - Add crypto driver")
Fixes d2c8ac187fc9 ("crypto: sa2ul - Add AEAD algorithm support")
Signed-off-by Manorit Chawdhry <[email protected]>
--- drivers/crypto/sa2ul.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
drivers/crypto/sa2ul.c +7 -4
@@ -856,7 +856,7 @@ static int sa_aes_cbc_setkey(struct crypto_skcipher *tfm, const u8 *key,/* Convert the key size (16/24/32) to the key size index (0/1/2) */ int key_idx = (keylen >>3 ) -2 ; -if (key_idx >=3 ) +if (key_idx <0 || key_idx >=3 )return -EINVAL ; ad.mci_enc = mci_cbc_enc_array[key_idx];
@@ -876,7 +876,7 @@ static int sa_aes_ecb_setkey(struct crypto_skcipher *tfm, const u8 *key,/* Convert the key size (16/24/32) to the key size index (0/1/2) */ int key_idx = (keylen >>3 ) -2 ; -if (key_idx >=3 ) +if (key_idx <0 || key_idx >=3 )return -EINVAL ; ad.mci_enc = mci_ecb_enc_array[key_idx];
@@ -1658,10 +1658,13 @@ static int sa_aead_setkey(struct crypto_aead *authenc,return -EINVAL ;/* Convert the key size (16/24/32) to the key size index (0/1/2) */ - key_idx = (keys.enckeylen >>3 ) -2 ; -if (key_idx >=3 ) +if (keys.enckeylen !=AES_KEYSIZE_128 && + keys.enckeylen !=AES_KEYSIZE_192 && + keys.enckeylen !=AES_KEYSIZE_256 )return -EINVAL ; + key_idx = (keys.enckeylen >>3 ) -2 ; + ad->enc_eng.eng_id =SA_ENG_ID_EM1 ; ad->enc_eng.sc_size =SA_CTX_ENC_TYPE1_SZ ; ad->auth_eng.eng_id =SA_ENG_ID_AM1 ;
Signature · 1 line
2.43.0
PATCH v2 15/30 crypto: sa2ul - remove unused SC ID range tracking
In reply to Manorit ChawdhryRemove unused sc_id_start and sc_id_end fields from sa_dev_data. These were intended for security context ID management but were never used in the driver.
Signed-off-by Manorit Chawdhry <[email protected]>
--- drivers/crypto/sa2ul.c | 4 ++-- drivers/crypto/sa2ul.h | 4 ---- 2 files changed, 2 insertions(+), 6 deletions(-)
drivers/crypto/sa2ul.c +2 -2
@@ -690,7 +690,7 @@ static void sa_free_ctx_info(struct sa_ctx_info *ctx, {unsigned long bn; - bn = ctx->sc_id - data->sc_id_start; + bn = ctx->sc_id;spin_lock (&data->scid_lock);__clear_bit (bn, data->ctx_bm); data->sc_id--;
@@ -715,7 +715,7 @@ static int sa_init_ctx_info(struct sa_ctx_info *ctx, data->sc_id++;spin_unlock (&data->scid_lock); - ctx->sc_id = (u16)(data->sc_id_start + bn); + ctx->sc_id = (u16 )bn; ctx->sc =dma_pool_alloc (data->sc_pool,GFP_KERNEL , &ctx->sc_phys);if (!ctx->sc) {
drivers/crypto/sa2ul.h +0 -4
@@ -180,8 +180,6 @@ struct sa_match_data;* @sc_pool: security context pool * @dev: Device pointer * @scid_lock: secure context ID lock -* @sc_id_start: starting index for SC ID -* @sc_id_end: Ending index for SC ID * @sc_id: Security Context ID * @ctx_bm: Bitmap to keep track of Security context ID's * @dma_rx1: Pointer to DMA rx channel for sizes < 256 Bytes
@@ -195,8 +193,6 @@ struct sa_crypto_data {struct device *dev;spinlock_t scid_lock;/* lock for SC-ID allocation */ /* Security context data */ -u16 sc_id_start; -u16 sc_id_end;u16 sc_id;unsigned long ctx_bm[DIV_ROUND_UP (SA_MAX_NUM_CTX ,BITS_PER_LONG )];
Signature · 1 line
2.43.0
PATCH v2 16/30 crypto: sa2ul - copy command-label inputs locally
In reply to Manorit ChawdhryKeep the command-label size and update information alongside the request-local template while formatting DMA metadata.
Signed-off-by Manorit Chawdhry <[email protected]>
--- drivers/crypto/sa2ul.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
drivers/crypto/sa2ul.c +8 -3
@@ -987,6 +987,8 @@ static int sa_run(struct sa_req *req)struct sa_rx_data *rxd;gfp_t gfp_flags;u32 cmdl[SA_MAX_CMDL_WORDS ]; +u16 cmdl_size; +struct sa_cmdl_upd_info cmdl_upd_info;struct sa_crypto_data *pdata =dev_get_drvdata (sa_k3_dev);struct device *ddev;struct dma_chan *dma_rx;
@@ -1032,9 +1034,12 @@ static int sa_run(struct sa_req *req) ddev =dmaengine_get_dma_device (pdata->dma_tx); rxd->ddev = ddev; -memcpy (cmdl, sa_ctx->cmdl, sa_ctx->cmdl_size); +/* Keep command-label formatting inputs in local storage. */ + cmdl_size = sa_ctx->cmdl_size; + cmdl_upd_info = sa_ctx->cmdl_upd_info; +memcpy (cmdl, sa_ctx->cmdl, cmdl_size); -sa_update_cmdl (req, cmdl, &sa_ctx->cmdl_upd_info); +sa_update_cmdl (req, cmdl, &cmdl_upd_info);if (req->type !=CRYPTO_ALG_TYPE_AHASH ) {if (req->enc)
@@ -1158,7 +1163,7 @@ static int sa_run(struct sa_req *req) rxd->req = (void *)req->base; rxd->enc = req->enc; rxd->iv_idx = req->ctx->iv_idx; - rxd->enc_iv_size = sa_ctx->cmdl_upd_info.enc_iv.size; + rxd->enc_iv_size = cmdl_upd_info.enc_iv.size; rxd->tx_in->callback = req->callback; rxd->tx_in->callback_param = rxd;
Signature · 1 line
2.43.0
PATCH v2 17/30 crypto: sa2ul - consolidate encryption offset definitions
In reply to Manorit ChawdhryMove all SA2UL encryption offset macro definitions to a single location for better maintainability. No functional change.
Signed-off-by Manorit Chawdhry <[email protected]>
--- drivers/crypto/sa2ul.c | 44 ++++++++++++++++++++++++++++---------------- drivers/crypto/sa2ul.h | 13 ------------- 2 files changed, 28 insertions(+), 29 deletions(-)
drivers/crypto/sa2ul.c +28 -16
@@ -32,10 +32,23 @@#include "sa2ul.h" -/* Byte offset for key in encryption security context */ -#define SC_ENC_KEY_OFFSET (1 + 27 + 4) -/* Byte offset for Aux-1 in encryption security context */ -#define SC_ENC_AUX1_OFFSET (1 + 27 + 4 + 32) +#define SC_ENC_MODESEL_OFFSET (0) +#define SC_ENC_MODESEL_SIZE (1) + +#define SC_ENC_MCI_OFFSET (SC_ENC_MODESEL_SIZE) +#define SC_ENC_MCI_SIZE (27) + +#define SC_ENC_KEY_OFFSET (SC_ENC_MCI_OFFSET + SC_ENC_MCI_SIZE + 4) +#define SC_ENC_KEY_SIZE 32 + +#define SC_ENC_AUX1_OFFSET (SC_ENC_KEY_OFFSET + SC_ENC_KEY_SIZE) +#define SC_ENC_AUX1_SIZE 32 + +#define SC_ENC_AUX2_OFFSET (SC_ENC_AUX1_OFFSET + SC_ENC_AUX1_SIZE) +#define SC_ENC_AUX2_SIZE 16 + +#define SC_ENC_AUX3_OFFSET (SC_ENC_AUX2_OFFSET + SC_ENC_AUX2_SIZE) +#define SC_ENC_AUX3_SIZE 16#define SA_CMDL_UPD_ENC 0x0001#define SA_CMDL_UPD_AUTH 0x0002
@@ -44,7 +57,6 @@#define SA_CMDL_PAYLOAD_LENGTH_MASK 0xFFFF#define SA_CMDL_SOP_BYPASS_LEN_MASK 0xFF000000 -#define MODE_CONTROL_BYTES 27#define SA_HASH_PROCESSING 0#define SA_CRYPTO_PROCESSING 0#define SA_UPLOAD_HASH_TO_TLR BIT(6)
@@ -217,7 +229,7 @@ struct sa_req {* Mode Control Instructions for various Key lengths 128, 192, 256 * For CBC (Cipher Block Chaining) mode for encryption */ -static u8 mci_cbc_enc_array[3 ][MODE_CONTROL_BYTES ] = { +static u8 mci_cbc_enc_array[3 ][SC_ENC_MCI_SIZE ] = { {0x61 ,0x00 ,0x00 ,0x18 ,0x88 ,0x0a ,0xaa ,0x4b ,0x7e ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 },
@@ -233,7 +245,7 @@ static u8 mci_cbc_enc_array[3][MODE_CONTROL_BYTES] = {* Mode Control Instructions for various Key lengths 128, 192, 256 * For CBC (Cipher Block Chaining) mode for decryption */ -static u8 mci_cbc_dec_array[3 ][MODE_CONTROL_BYTES ] = { +static u8 mci_cbc_dec_array[3 ][SC_ENC_MCI_SIZE ] = { {0x71 ,0x00 ,0x00 ,0x80 ,0x8a ,0xca ,0x98 ,0xf4 ,0x40 ,0xc0 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 },
@@ -249,7 +261,7 @@ static u8 mci_cbc_dec_array[3][MODE_CONTROL_BYTES] = {* Mode Control Instructions for various Key lengths 128, 192, 256 * For CBC (Cipher Block Chaining) mode for encryption */ -static u8 mci_cbc_enc_no_iv_array[3 ][MODE_CONTROL_BYTES ] = { +static u8 mci_cbc_enc_no_iv_array[3 ][SC_ENC_MCI_SIZE ] = { {0x21 ,0x00 ,0x00 ,0x18 ,0x88 ,0x0a ,0xaa ,0x4b ,0x7e ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 },
@@ -265,7 +277,7 @@ static u8 mci_cbc_enc_no_iv_array[3][MODE_CONTROL_BYTES] = {* Mode Control Instructions for various Key lengths 128, 192, 256 * For CBC (Cipher Block Chaining) mode for decryption */ -static u8 mci_cbc_dec_no_iv_array[3 ][MODE_CONTROL_BYTES ] = { +static u8 mci_cbc_dec_no_iv_array[3 ][SC_ENC_MCI_SIZE ] = { {0x31 ,0x00 ,0x00 ,0x80 ,0x8a ,0xca ,0x98 ,0xf4 ,0x40 ,0xc0 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 },
@@ -314,25 +326,25 @@ static u8 mci_ecb_dec_array[3][27] = {* For CBC (Cipher Block Chaining) mode and ECB mode * encryption and for decryption respectively */ -static u8 mci_cbc_3des_enc_array[MODE_CONTROL_BYTES ] = { +static u8 mci_cbc_3des_enc_array[SC_ENC_MCI_SIZE ] = {0x60 ,0x00 ,0x00 ,0x18 ,0x88 ,0x52 ,0xaa ,0x4b ,0x7e ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 , }; -static u8 mci_cbc_3des_dec_array[MODE_CONTROL_BYTES ] = { +static u8 mci_cbc_3des_dec_array[SC_ENC_MCI_SIZE ] = {0x70 ,0x00 ,0x00 ,0x85 ,0x0a ,0xca ,0x98 ,0xf4 ,0x40 ,0xc0 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 , }; -static u8 mci_ecb_3des_enc_array[MODE_CONTROL_BYTES ] = { +static u8 mci_ecb_3des_enc_array[SC_ENC_MCI_SIZE ] = {0x20 ,0x00 ,0x00 ,0x85 ,0x0a ,0x04 ,0xb7 ,0x90 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 , }; -static u8 mci_ecb_3des_dec_array[MODE_CONTROL_BYTES ] = { +static u8 mci_ecb_3des_dec_array[SC_ENC_MCI_SIZE ] = {0x30 ,0x00 ,0x00 ,0x85 ,0x0a ,0x04 ,0xb7 ,0x90 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,
@@ -432,7 +444,7 @@ static int sa_set_sc_enc(struct algo_data *ad, const u8 *key, u16 key_sz,const u8 *mci =NULL ;/* Set Encryption mode selector to crypto processing */ - sc_buf[0 ] =SA_CRYPTO_PROCESSING ; + sc_buf[SC_ENC_MODESEL_OFFSET ] =SA_CRYPTO_PROCESSING ;if (enc) mci = ad->mci_enc;
@@ -440,7 +452,7 @@ static int sa_set_sc_enc(struct algo_data *ad, const u8 *key, u16 key_sz, mci = ad->mci_dec;/* Set the mode control instructions in security context */ if (mci) -memcpy (&sc_buf[1 ], mci,MODE_CONTROL_BYTES ); +memcpy (&sc_buf[SC_ENC_MCI_OFFSET ], mci,SC_ENC_MCI_SIZE );/* For AES-CBC decryption get the inverse key */ if (ad->inv_key && !enc) {
@@ -523,7 +535,7 @@ static int sa_format_cmdl_gen(struct sa_cmdl_cfg *cfg, u8 *cmdl,SA_CMDL_HEADER_SIZE_BYTES + cfg->iv_size; cmdl[enc_offset +SA_CMDL_OFFSET_OPTION_CTRL1 ] = - (SA_CTX_ENC_AUX2_OFFSET | (cfg->iv_size >>3 )); + (SC_ENC_AUX2_OFFSET | (cfg->iv_size >>3 )); total +=SA_CMDL_HEADER_SIZE_BYTES + cfg->iv_size; }else { cmdl[enc_offset + SA_CMDL_OFFSET_LABEL_LEN] =
drivers/crypto/sa2ul.h +0 -13
@@ -46,7 +46,6 @@ struct sa_tfm_ctx;/* Number of 32 bit words in PS data */ #define SA_DMA_NUM_PS_WORDS 16 -#define MCI_SZ 27/* * Maximum number of simultaeneous security contexts
@@ -60,12 +59,6 @@ struct sa_tfm_ctx;#define SA_CTX_SIZE_TO_DMA_SIZE (ctx_sz) \ ((ctx_sz) ? ((ctx_sz) / 32 - 1) : 0) -#define SA_CTX_ENC_KEY_OFFSET 32 -#define SA_CTX_ENC_AUX1_OFFSET 64 -#define SA_CTX_ENC_AUX2_OFFSET 96 -#define SA_CTX_ENC_AUX3_OFFSET 112 -#define SA_CTX_ENC_AUX4_OFFSET 128 -/* Next Engine Select code in CP_ACE */ #define SA_ENG_ID_EM1 2/* Enc/Dec engine with AES/DEC core */ #define SA_ENG_ID_EM2 3/* Encryption/Decryption enginefor pass 2 */
@@ -142,12 +135,6 @@ struct sa_tfm_ctx;*/ #define SA_CTX_SCCTL_OWNER_OFFSET 0 -#define SA_CTX_ENC_KEY_OFFSET 32 -#define SA_CTX_ENC_AUX1_OFFSET 64 -#define SA_CTX_ENC_AUX2_OFFSET 96 -#define SA_CTX_ENC_AUX3_OFFSET 112 -#define SA_CTX_ENC_AUX4_OFFSET 128 -#define SA_SCCTL_FE_AUTH_ENC 0x65#define SA_SCCTL_FE_ENC 0x8D
Signature · 1 line
2.43.0
PATCH v2 18/30 crypto: sa2ul - handle unaligned IV buffers
In reply to Manorit ChawdhryRequest IV buffers need not be word aligned. Use unaligned accesses when formatting and returning IV words, preserving their byte order without risking alignment faults.
Fixes 7694b6ca649f ("crypto: sa2ul - Add crypto driver")
Fixes d2c8ac187fc9 ("crypto: sa2ul - Add AEAD algorithm support")
Signed-off-by Manorit Chawdhry <[email protected]>
--- drivers/crypto/sa2ul.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
drivers/crypto/sa2ul.c +8 -6
@@ -14,6 +14,7 @@#include <linux/dmapool.h> #include <linux/kernel.h> #include <linux/module.h> +#include <linux/unaligned.h> #include <linux/of.h> #include <linux/of_platform.h> #include <linux/platform_device.h>
@@ -579,11 +580,11 @@ static inline void sa_update_cmdl(struct sa_req *req, u32 *cmdl,if (likely (upd_info->flags &SA_CMDL_UPD_ENC_IV )) {__be32 *data = (__be32 *)&cmdl[upd_info->enc_iv.index]; -u32 *enc_iv = (u32 *)req->enc_iv; +const u8 *enc_iv = req->enc_iv;for (j =0 ; i < upd_info->enc_iv.size; i +=4 , j++) { - data[j] =cpu_to_be32 (*enc_iv); - enc_iv++; + data[j] =cpu_to_be32 (get_unaligned ((const u32 *)enc_iv)); + enc_iv +=sizeof (u32); } } }
@@ -957,7 +958,7 @@ static void sa_aes_dma_in_callback(void *data) {struct sa_rx_data *rxd = data;struct skcipher_request *req; -u32 *result; +u8 *result;__be32 *mdptr;size_t ml, pl;int i;
@@ -968,10 +969,11 @@ static void sa_aes_dma_in_callback(void *data)if (req->iv) { mdptr = (__be32 *)dmaengine_desc_get_metadata_ptr (rxd->tx_in, &pl, &ml); - result = (u32 *)req->iv; + result = req->iv;for (i =0 ; i < (rxd->enc_iv_size /4 ); i++) - result[i] =be32_to_cpu (mdptr[i + rxd->iv_idx]); +put_unaligned (be32_to_cpu (mdptr[i + rxd->iv_idx]), + (u32 *)(result + i *sizeof (u32))); }sa_free_sa_rx_data (rxd);
Signature · 1 line
2.43.0
PATCH v2 19/30 crypto: sa2ul - remove redundant algorithm ID fields
In reply to Manorit ChawdhryUse the full digest size to select HMAC-SHA1 or HMAC-SHA256 when preparing ipad and opad. The size is already populated for the SHA and AEAD paths, so a separate software algorithm ID is unnecessary.
Keep the digest size unchanged during context setup and round only the hardware egress length, so SHA1 still selects its 20-byte digest after both encryption and decryption contexts are prepared.
The encryption algorithm ID and command-label algorithm field are unused, so remove them and their assignments along with the legacy authentication and encryption ID enums.
Signed-off-by Manorit Chawdhry <[email protected]>
--- drivers/crypto/sa2ul.c | 32 ++++++-------------------------- drivers/crypto/sa2ul.h | 34 ---------------------------------- 2 files changed, 6 insertions(+), 60 deletions(-)
drivers/crypto/sa2ul.c +6 -26
@@ -95,14 +95,12 @@ static struct device *sa_k3_dev;/** * struct sa_cmdl_cfg - Command label configuration descriptor -* @aalg: authentication algorithm ID * @enc_eng_id: Encryption Engine ID supported by the SA hardware * @auth_eng_id: Authentication Engine ID * @iv_size: Initialization Vector size * @enc: True, if this is an encode request */ struct sa_cmdl_cfg { -int aalg;u8 enc_eng_id;u8 auth_eng_id;u8 iv_size;
@@ -117,8 +115,6 @@ struct sa_cmdl_cfg {* @hash_size: Size of digest * @iv_idx: iv index in psdata * @iv_out_size: iv out size -* @ealg_id: Encryption Algorithm ID -* @aalg_id: Authentication algorithm ID * @mci_enc: Mode Control Instruction for Encryption algorithm * @mci_dec: Mode Control Instruction for Decryption * @inv_key: Whether the encryption algorithm demands key inversion
@@ -131,8 +127,6 @@ struct algo_data {u8 hash_size;u8 iv_idx;u8 iv_out_size; -u8 ealg_id; -u8 aalg_id;u8 *mci_enc;u8 *mci_dec;bool inv_key;
@@ -377,15 +371,15 @@ static void sa_prepare_iopads(struct algo_data *data, const u8 *key,struct hmac_sha256_key sha256; } k; -switch (data->aalg_id) { -case SA_AALG_ID_HMAC_SHA1 : +switch (data->hash_size) { +case SHA1_DIGEST_SIZE :hmac_sha1_preparekey (&k.sha1, key, key_sz);cpu_to_be32_array (ipad, k.sha1.istate.h,ARRAY_SIZE (k.sha1.istate.h));cpu_to_be32_array (opad, k.sha1.ostate.h,ARRAY_SIZE (k.sha1.ostate.h));break ; -case SA_AALG_ID_HMAC_SHA2_256 : +case SHA256_DIGEST_SIZE :hmac_sha256_preparekey (&k.sha256, key, key_sz);cpu_to_be32_array (ipad, k.sha256.key.istate.h,ARRAY_SIZE (k.sha256.key.istate.h));
@@ -393,8 +387,8 @@ static void sa_prepare_iopads(struct algo_data *data, const u8 *key, ARRAY_SIZE(k.sha256.key.ostate.h));break ;default : -dev_err (sa_k3_dev,"%s: bad aalg_id=%d\n" , __func__, - data->aalg_id); +dev_err (sa_k3_dev,"%s: bad hash_size=%u\n" , __func__, + data->hash_size);return ; }
@@ -654,7 +648,6 @@ int sa_init_sc(struct sa_ctx_info *ctx, const struct sa_match_data *match_data, sc_buf[1 ] =SA_SCCTL_FE_AUTH_ENC ;if (!ad->hash_size)return -EINVAL ; - ad->hash_size =roundup (ad->hash_size,8 ); }else if (ad->enc_eng.eng_id && !ad->auth_eng.eng_id) { enc_sc_offset =SA_CTX_PHP_PE_CTX_SZ ;
@@ -690,7 +683,7 @@ int sa_init_sc(struct sa_ctx_info *ctx, const struct sa_match_data *match_data,sa_swiz_128 (sc_buf,SA_CTX_MAX_SZ );sa_set_swinfo (first_engine, ctx->sc_id, ctx->sc_phys,1 ,0 , -SA_SW_INFO_FLAG_EVICT , ad->hash_size, swinfo); +SA_SW_INFO_FLAG_EVICT ,roundup (ad->hash_size,8 ), swinfo);sa_dump_sc (sc_buf, ctx->sc_phys);
@@ -875,7 +868,6 @@ static int sa_aes_cbc_setkey(struct crypto_skcipher *tfm, const u8 *key, ad.mci_enc = mci_cbc_enc_array[key_idx]; ad.mci_dec = mci_cbc_dec_array[key_idx]; ad.inv_key = true; - ad.ealg_id =SA_EALG_ID_AES_CBC ; ad.iv_idx =4 ; ad.iv_out_size =16 ;
@@ -895,7 +887,6 @@ static int sa_aes_ecb_setkey(struct crypto_skcipher *tfm, const u8 *key, ad.mci_enc = mci_ecb_enc_array[key_idx]; ad.mci_dec = mci_ecb_dec_array[key_idx]; ad.inv_key = true; - ad.ealg_id =SA_EALG_ID_AES_ECB ;return sa_cipher_setkey (tfm, key, keylen, &ad); }
@@ -907,7 +898,6 @@ static int sa_3des_cbc_setkey(struct crypto_skcipher *tfm, const u8 *key, ad.mci_enc = mci_cbc_3des_enc_array; ad.mci_dec = mci_cbc_3des_dec_array; - ad.ealg_id =SA_EALG_ID_3DES_CBC ; ad.iv_idx =6 ; ad.iv_out_size =8 ;
@@ -1373,7 +1363,6 @@ static int sa_sha_setup(struct sa_tfm_ctx *ctx, struct algo_data *ad) ad->auth_eng.sc_size =SA_CTX_AUTH_TYPE2_SZ ;memset (&cfg,0 ,sizeof (cfg)); - cfg.aalg = ad->aalg_id; cfg.enc_eng_id = ad->enc_eng.eng_id; cfg.auth_eng_id = ad->auth_eng.eng_id; cfg.iv_size =0 ;
@@ -1508,7 +1497,6 @@ static int sa_sha1_cra_init(struct crypto_tfm *tfm)sa_sha_cra_init_alg (tfm,"sha1" ); - ad.aalg_id =SA_AALG_ID_SHA1 ; ad.hash_size =SHA1_DIGEST_SIZE ; ad.auth_ctrl =SA_AUTH_SW_CTRL_SHA1 ;
@@ -1524,7 +1512,6 @@ static int sa_sha256_cra_init(struct crypto_tfm *tfm)sa_sha_cra_init_alg (tfm,"sha256" ); - ad.aalg_id =SA_AALG_ID_SHA2_256 ; ad.hash_size =SHA256_DIGEST_SIZE ; ad.auth_ctrl =SA_AUTH_SW_CTRL_SHA256 ;
@@ -1540,7 +1527,6 @@ static int sa_sha512_cra_init(struct crypto_tfm *tfm)sa_sha_cra_init_alg (tfm,"sha512" ); - ad.aalg_id =SA_AALG_ID_SHA2_512 ; ad.hash_size =SHA512_DIGEST_SIZE ; ad.auth_ctrl =SA_AUTH_SW_CTRL_SHA512 ;
@@ -1692,11 +1678,9 @@ static int sa_aead_setkey(struct crypto_aead *authenc, ad->mci_dec = mci_cbc_dec_no_iv_array[key_idx]; ad->inv_key = true; ad->keyed_mac = true; - ad->ealg_id =SA_EALG_ID_AES_CBC ;memset (&cfg,0 ,sizeof (cfg)); cfg.enc = true; - cfg.aalg = ad->aalg_id; cfg.enc_eng_id = ad->enc_eng.eng_id; cfg.auth_eng_id = ad->auth_eng.eng_id; cfg.iv_size =crypto_aead_ivsize (authenc);
@@ -1750,8 +1734,6 @@ static int sa_aead_cbc_sha1_setkey(struct crypto_aead *authenc, {struct algo_data ad = {0 }; - ad.ealg_id =SA_EALG_ID_AES_CBC ; - ad.aalg_id =SA_AALG_ID_HMAC_SHA1 ; ad.hash_size =SHA1_DIGEST_SIZE ; ad.auth_ctrl =SA_AUTH_SW_CTRL_SHA1 ;
@@ -1763,8 +1745,6 @@ static int sa_aead_cbc_sha256_setkey(struct crypto_aead *authenc, {struct algo_data ad = {0 }; - ad.ealg_id =SA_EALG_ID_AES_CBC ; - ad.aalg_id =SA_AALG_ID_HMAC_SHA2_256 ; ad.hash_size =SHA256_DIGEST_SIZE ; ad.auth_ctrl =SA_AUTH_SW_CTRL_SHA256 ;
drivers/crypto/sa2ul.h +0 -34
@@ -277,40 +277,6 @@ struct sa_sha_req_ctx {struct ahash_request fallback_req; }; -/* Encryption algorithms */ -enum sa_ealg_id { -SA_EALG_ID_NONE =0 ,/* No encryption */ -SA_EALG_ID_NULL ,/* NULL encryption */ -SA_EALG_ID_AES_CTR ,/* AES Counter mode */ -SA_EALG_ID_AES_F8 ,/* AES F8 mode */ -SA_EALG_ID_AES_CBC ,/* AES CBC mode */ -SA_EALG_ID_DES_CBC ,/* DES CBC mode */ -SA_EALG_ID_3DES_CBC ,/* 3DES CBC mode */ -SA_EALG_ID_CCM ,/* Counter with CBC-MAC mode */ -SA_EALG_ID_GCM ,/* Galois Counter mode */ -SA_EALG_ID_AES_ECB , -SA_EALG_ID_LAST -}; - -/* Authentication algorithms */ -enum sa_aalg_id { -SA_AALG_ID_NONE =0 ,/* No Authentication */ -SA_AALG_ID_NULL =SA_EALG_ID_LAST ,/* NULL Authentication */ -SA_AALG_ID_MD5 ,/* MD5 mode */ -SA_AALG_ID_SHA1 ,/* SHA1 mode */ -SA_AALG_ID_SHA2_224 ,/* 224-bit SHA2 mode */ -SA_AALG_ID_SHA2_256 ,/* 256-bit SHA2 mode */ -SA_AALG_ID_SHA2_512 ,/* 512-bit SHA2 mode */ -SA_AALG_ID_HMAC_MD5 ,/* HMAC with MD5 mode */ -SA_AALG_ID_HMAC_SHA1 ,/* HMAC with SHA1 mode */ -SA_AALG_ID_HMAC_SHA2_224 ,/* HMAC with 224-bit SHA2 mode */ -SA_AALG_ID_HMAC_SHA2_256 ,/* HMAC with 256-bit SHA2 mode */ -SA_AALG_ID_GMAC ,/* Galois Message Auth. Code mode */ -SA_AALG_ID_CMAC ,/* Cipher-based Mes. Auth. Code mode */ -SA_AALG_ID_CBC_MAC ,/* Cipher Block Chaining */ -SA_AALG_ID_AES_XCBC /* AES Extended Cipher Block Chaining */ -}; -/** * struct sa_eng_info: Security accelerator engine info * @eng_id: Engine ID
Signature · 1 line
2.43.0
PATCH v2 20/30 crypto: sa2ul - handle transform initialization failures
In reply to Manorit ChawdhryStop initialization when security-context or fallback allocation fails, and propagate hash setup errors instead of exposing a partial transform.
Unwind acquired fallback transforms and security contexts in reverse order. Release the hash context on fallback allocation failure, and use the hash teardown path when command-label setup fails.
Fixes 7694b6ca649f ("crypto: sa2ul - Add crypto driver")
Fixes 2dc53d004745 ("crypto: sa2ul - add sha1/sha256/sha512 support")
Fixes d2c8ac187fc9 ("crypto: sa2ul - Add AEAD algorithm support")
Signed-off-by Manorit Chawdhry <[email protected]>
--- drivers/crypto/sa2ul.c | 70 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 50 insertions(+), 20 deletions(-)
drivers/crypto/sa2ul.c +50 -20
@@ -771,16 +771,14 @@ static int sa_cipher_cra_init(struct crypto_skcipher *tfm)if (ret)return ret; ret =sa_init_ctx_info (&ctx->dec, data); -if (ret) { -sa_free_ctx_info (&ctx->enc, data); -return ret; - } +if (ret) + goto ctx_dec_err; child =crypto_alloc_skcipher (name,0 ,CRYPTO_ALG_NEED_FALLBACK ); -if (IS_ERR (child)) {dev_err (sa_k3_dev,"Error allocating fallback algo %s\n" , name); -return PTR_ERR (child); + ret =PTR_ERR (child); + goto alloc_err; } ctx->fallback.skcipher = child;
@@ -791,6 +789,11 @@ static int sa_cipher_cra_init(struct crypto_skcipher *tfm) __func__, tfm, ctx->enc.sc_id, &ctx->enc.sc_phys, ctx->dec.sc_id, &ctx->dec.sc_phys);return 0 ; +alloc_err: +sa_free_ctx_info (&ctx->dec, data); +ctx_dec_err: +sa_free_ctx_info (&ctx->enc, data); +return ret; } static int sa_cipher_setkey(struct crypto_skcipher *tfm, const u8 *key,
@@ -1407,7 +1410,9 @@ static int sa_sha_cra_init_alg(struct crypto_tfm *tfm, const char *alg_base)if (IS_ERR (ctx->fallback.ahash)) {dev_err (ctx->dev_data->dev,"Could not load fallback driver\n" ); -return PTR_ERR (ctx->fallback.ahash); + ret =PTR_ERR (ctx->fallback.ahash); + ctx->fallback.ahash =NULL ; + goto err_free_ctx; } }
@@ -1420,8 +1425,14 @@ static int sa_sha_cra_init_alg(struct crypto_tfm *tfm, const char *alg_base) crypto_ahash_reqsize(ctx->fallback.ahash));return 0 ; + +err_free_ctx: +sa_free_ctx_info (&ctx->enc, data); +return ret; } +static void sa_sha_cra_exit (struct crypto_tfm *tfm); +static int sa_sha_init (struct ahash_request *req) {struct crypto_ahash *tfm =crypto_ahash_reqtfm (req);
@@ -1494,45 +1505,60 @@ static int sa_sha1_cra_init(struct crypto_tfm *tfm) {struct algo_data ad = {0 };struct sa_tfm_ctx *ctx =crypto_tfm_ctx (tfm); +int ret =0 ; -sa_sha_cra_init_alg (tfm,"sha1" ); + ret =sa_sha_cra_init_alg (tfm,"sha1" ); +if (ret) +return ret; ad.hash_size =SHA1_DIGEST_SIZE ; ad.auth_ctrl =SA_AUTH_SW_CTRL_SHA1 ; -sa_sha_setup (ctx, &ad); + ret =sa_sha_setup (ctx, &ad); +if (ret) +sa_sha_cra_exit (tfm); -return 0 ; +return ret; }static int sa_sha256_cra_init (struct crypto_tfm *tfm) {struct algo_data ad = {0 };struct sa_tfm_ctx *ctx =crypto_tfm_ctx (tfm); +int ret =0 ; -sa_sha_cra_init_alg (tfm,"sha256" ); + ret =sa_sha_cra_init_alg (tfm,"sha256" ); +if (ret) +return ret; ad.hash_size =SHA256_DIGEST_SIZE ; ad.auth_ctrl =SA_AUTH_SW_CTRL_SHA256 ; -sa_sha_setup (ctx, &ad); + ret =sa_sha_setup (ctx, &ad); +if (ret) +sa_sha_cra_exit (tfm); -return 0 ; +return ret; }static int sa_sha512_cra_init (struct crypto_tfm *tfm) {struct algo_data ad = {0 };struct sa_tfm_ctx *ctx =crypto_tfm_ctx (tfm); +int ret =0 ; -sa_sha_cra_init_alg (tfm,"sha512" ); + ret =sa_sha_cra_init_alg (tfm,"sha512" ); +if (ret) +return ret; ad.hash_size =SHA512_DIGEST_SIZE ; ad.auth_ctrl =SA_AUTH_SW_CTRL_SHA512 ; -sa_sha_setup (ctx, &ad); + ret =sa_sha_setup (ctx, &ad); +if (ret) +sa_sha_cra_exit (tfm); -return 0 ; +return ret; }static void sa_sha_cra_exit (struct crypto_tfm *tfm)
@@ -1544,10 +1570,10 @@ static void sa_sha_cra_exit(struct crypto_tfm *tfm) __func__, tfm, ctx->enc.sc_id, &ctx->enc.sc_phys, ctx->dec.sc_id, &ctx->dec.sc_phys); +crypto_free_ahash (ctx->fallback.ahash); +if (crypto_tfm_alg_type (tfm) ==CRYPTO_ALG_TYPE_AHASH )sa_free_ctx_info (&ctx->enc, data); - -crypto_free_ahash (ctx->fallback.ahash); }static void sa_aead_dma_in_callback (void *data)
@@ -1612,12 +1638,12 @@ static int sa_cra_init_aead(struct crypto_aead *tfm, const char *fallback) ret =sa_init_ctx_info (&ctx->enc, data);if (ret) -return ret; + goto err_free_fallback; ret =sa_init_ctx_info (&ctx->dec, data);if (ret) {sa_free_ctx_info (&ctx->enc, data); -return ret; + goto err_free_fallback; } dev_dbg(sa_k3_dev, "%s(0x%p) sc-ids(0x%x(0x%pad), 0x%x(0x%pad))\n",
@@ -1625,6 +1651,10 @@ static int sa_cra_init_aead(struct crypto_aead *tfm, const char *fallback) ctx->dec.sc_id, &ctx->dec.sc_phys);return ret; + +err_free_fallback: +crypto_free_aead (ctx->fallback.aead); +return ret; }static int sa_cra_init_aead_sha1 (struct crypto_aead *tfm)
Signature · 1 line
2.43.0
PATCH v2 21/30 crypto: sa2ul - reject exhausted context IDs
In reply to Manorit ChawdhryWhen every security-context ID is allocated, the bitmap search returns SA_MAX_NUM_CTX. Setting that out-of-range bit can corrupt adjacent state in the device structure. Reject allocation while still holding the lock so no bitmap or context count is changed.
Fixes 7694b6ca649f ("crypto: sa2ul - Add crypto driver")
Signed-off-by Manorit Chawdhry <[email protected]>
--- drivers/crypto/sa2ul.c | 4 ++++ 1 file changed, 4 insertions(+)
drivers/crypto/sa2ul.c +4 -0
@@ -717,6 +717,10 @@ static int sa_init_ctx_info(struct sa_ctx_info *ctx,spin_lock (&data->scid_lock); bn =find_first_zero_bit (data->ctx_bm,SA_MAX_NUM_CTX ); +if (bn >=SA_MAX_NUM_CTX ) { +spin_unlock (&data->scid_lock); +return -ENOSPC ; + }__set_bit (bn, data->ctx_bm); data->sc_id++;spin_unlock (&data->scid_lock);
Signature · 1 line
2.43.0
Re: [PATCH v2 21/30] crypto: sa2ul - reject exhausted context IDs
Quoted 16 lines · Manorit Chawdhry
When every security-context ID is allocated, the bitmap search returns
SA_MAX_NUM_CTX. Setting that out-of-range bit can corrupt adjacent state in the device structure. Reject allocation while still holding the lock so no bitmap or context count is changed.Fixes 7694b6ca649f ("crypto: sa2ul - Add crypto driver")Signed-off-by Manorit Chawdhry <[email protected]>--- drivers/crypto/sa2ul.c | 4 ++++ 1 file changed, 4 insertions(+)drivers/crypto/sa2ul.c +0
-0
drivers/crypto/sa2ul.c static int sa_init_ctx_info(struct sa_ctx_info *ctx, · line 726
Quoted 10 lines
drivers/crypto/sa2ul.c +4
-0@@ -717,6 +717,10 @@ static int sa_init_ctx_info(struct sa_ctx_info *ctx,spin_lock (&data->scid_lock); bn =find_first_zero_bit (data->ctx_bm,SA_MAX_NUM_CTX ); +if (bn >=SA_MAX_NUM_CTX ) { +spin_unlock (&data->scid_lock); +return -ENOSPC ; + }__set_bit (bn, data->ctx_bm); data->sc_id++;spin_unlock (&data->scid_lock);
Reviewed-by Thorsten Blum <[email protected]>
PATCH v2 22/30 crypto: sa2ul - validate scatterlist setup and unwind DMA mappings
In reply to Manorit ChawdhryDo not continue with an unsplit scatterlist or pass invalid entry counts to the DMA API when request preparation fails. Check source and destination lengths before mapping, stop on split failures, and route partial setup through common cleanup so all mappings and split lists are released before returning an error.
Fixes 7694b6ca649f ("crypto: sa2ul - Add crypto driver")
Signed-off-by Manorit Chawdhry <[email protected]>
--- drivers/crypto/sa2ul.c | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-)
drivers/crypto/sa2ul.c +18 -14
@@ -1071,6 +1071,10 @@ static int sa_run(struct sa_req *req)*/ src = req->src; sg_nents =sg_nents_for_len (src, req->size); +if (sg_nents <0 ) { + ret = sg_nents; + goto err_cleanup; + } split_size = req->size;
@@ -1086,7 +1090,7 @@ static int sa_run(struct sa_req *req) mapped_sg->sgt.orig_nents = src_nents; ret =dma_map_sgtable (ddev, &mapped_sg->sgt, dir_src,0 );if (ret) { -kfree (rxd); +sa_free_sa_rx_data (rxd);return ret; }
@@ -1097,7 +1101,7 @@ static int sa_run(struct sa_req *req) mapped_sg->sgt.orig_nents = sg_nents; ret =dma_map_sgtable (ddev, &mapped_sg->sgt, dir_src,0 );if (ret) { -kfree (rxd); +sa_free_sa_rx_data (rxd);return ret; }
@@ -1106,12 +1110,10 @@ static int sa_run(struct sa_req *req) ret =sg_split (mapped_sg->sgt.sgl, mapped_sg->sgt.nents,0 ,1 , &split_size, &src, &src_nents, gfp_flags); -if (ret) { - src_nents = mapped_sg->sgt.nents; - src = mapped_sg->sgt.sgl; - }else { - mapped_sg->split_sg = src; - } +if (ret) + goto err_cleanup; + + mapped_sg->split_sg = src; }dma_sync_sgtable_for_device (ddev, &mapped_sg->sgt,DMA_TO_DEVICE );
@@ -1121,6 +1123,10 @@ static int sa_run(struct sa_req *req) dst = src; }else { dst_nents =sg_nents_for_len (req->dst, req->size); +if (dst_nents <0 ) { + ret = dst_nents; + goto err_cleanup; + } mapped_sg = &rxd->mapped_sg[1 ];if (dst_nents ==1 && split_size <= req->dst->length) {
@@ -1153,12 +1159,10 @@ static int sa_run(struct sa_req *req) ret =sg_split (mapped_sg->sgt.sgl, mapped_sg->sgt.nents,0 ,1 , &split_size, &dst, &dst_nents, gfp_flags); -if (ret) { - dst_nents = mapped_sg->sgt.nents; - dst = mapped_sg->sgt.sgl; - }else { - mapped_sg->split_sg = dst; - } +if (ret) + goto err_cleanup; + + mapped_sg->split_sg = dst; } }
Signature · 1 line
2.43.0
[email protected]
AI reviewRe: [PATCH v2 22/30] crypto: sa2ul - validate scatterlist setup and unwind DMA mappings
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] crypto: sa2ul: sa_run() passes 0 nents to dma_map_sgtable() when size is 0 --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] crypto: sa2ul: sa_run() passes 0 nents to dma_map_sgtable() when size is 0 --
commit 615706b20f3b90ba270a289c49acc68499b974e6
Author: Manorit Chawdhry <[email protected]>
crypto: sa2ul - validate scatterlist setup and unwind DMA mappings
This commit validates source and destination scatterlist lengths with sg_nents_for_len(), aborts on sg_split() failures, and routes partial setup errors in sa_run() through sa_free_sa_rx_data().
Quoted 51 lines
drivers/crypto/sa2ul.c +10
-8@@ -1071,6 +1071,10 @@ static int sa_run(struct sa_req *req)*/ src = req->src; sg_nents =sg_nents_for_len (src, req->size); +if (sg_nents <0 ) { + ret = sg_nents; + goto err_cleanup; + } split_size = req->size;@@ -1086,7 +1090,7 @@ static int sa_run(struct sa_req *req) mapped_sg->sgt.orig_nents = src_nents; ret =dma_map_sgtable (ddev, &mapped_sg->sgt, dir_src,0 );if (ret) { -kfree (rxd); +sa_free_sa_rx_data (rxd);return ret; }@@ -1097,7 +1101,7 @@ static int sa_run(struct sa_req *req) mapped_sg->sgt.orig_nents = sg_nents; ret =dma_map_sgtable (ddev, &mapped_sg->sgt, dir_src,0 );if (ret) { -kfree (rxd); +sa_free_sa_rx_data (rxd);return ret; }@@ -1106,12 +1110,10 @@ static int sa_run(struct sa_req *req) ret =sg_split (mapped_sg->sgt.sgl, mapped_sg->sgt.nents,0 ,1 , &split_size, &src, &src_nents, gfp_flags); -if (ret) { - src_nents = mapped_sg->sgt.nents; - src = mapped_sg->sgt.sgl; - }else { - mapped_sg->split_sg = src; - } +if (ret) + goto err_cleanup; + + mapped_sg->split_sg = src; }dma_sync_sgtable_for_device (ddev, &mapped_sg->sgt,DMA_TO_DEVICE );
drivers/crypto/sa2ul.c static int sa_run(struct sa_req *req) · line 1129
Quoted 7 lines
drivers/crypto/sa2ul.c +4
-0@@ -1121,6 +1123,10 @@ static int sa_run(struct sa_req *req) dst = src; }else { dst_nents =sg_nents_for_len (req->dst, req->size); +if (dst_nents <0 ) { + ret = dst_nents; + goto err_cleanup; + }
[Severity: High]
What happens in sa_run() when req->size is 0?
In sa_aead_run(), an AEAD request with req->assoclen == 0 and zero plaintext length (req->cryptlen == 0 on encryption, or req->cryptlen == crypto_aead_authsize(tfm) on decryption) results in auth_size == 0. Unlike sa_cipher_run() and sa_sha_run(), sa_aead_run() does not divert zero-length requests to the software fallback:
drivers/crypto/sa2ul.c:sa_aead_run() {
...
if (auth_size > SA_MAX_DATA_SZ ||
(auth_size >= SA_UNSAFE_DATA_SZ_MIN &&
auth_size <= SA_UNSAFE_DATA_SZ_MAX)) {
...
}
...
sa_req.size = auth_size;
...
return sa_run(&sa_req);
}When sa_run() calls sg_nents_for_len() with a length of 0, sg_nents_for_len() returns 0. Because the new checks only test sg_nents < 0 and dst_nents < 0, a return value of 0 bypasses the error check, skips the sg_nents == 1 branch, and enters the else branch:
} else {
mapped_sg->sgt.sgl = req->src;
mapped_sg->sgt.orig_nents = sg_nents;
ret = dma_map_sgtable(ddev, &mapped_sg->sgt, dir_src, 0);Passing orig_nents == 0 to dma_map_sgtable() triggers the WARN_ON_ONCE(nents <= 0) in __dma_map_sg_attrs() and fails the request with -EINVAL.
Should sa_run() reject sg_nents <= 0 and dst_nents <= 0 (or should sa_aead_run() route auth_size == 0 to the software fallback)?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=22
PATCH v2 23/30 crypto: sa2ul - propagate DMA transfer errors
In reply to Manorit ChawdhryDMA completion callbacks discard the transfer result and process output metadata even when a transfer fails. This can report failed cipher or hash requests as successful, or an AEAD transfer failure as a bad tag.
Use result-aware callbacks and return -EIO for failed or incomplete transfers. Skip IV, digest and tag processing on error while preserving DMA cleanup and completing each request once.
Signed-off-by Manorit Chawdhry <[email protected]>
--- drivers/crypto/sa2ul.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-)
drivers/crypto/sa2ul.c +21 -10
@@ -199,7 +199,7 @@ struct sa_rx_data {* @enc: true if this is an encode request * @src: source data * @dst: destination data -* @callback: DMA callback for the request +* @callback: DMA callback receiving the transfer result * @mdata_size: metadata size passed to DMA */ struct sa_req {
@@ -216,7 +216,7 @@ struct sa_req {bool enc;struct scatterlist *src;struct scatterlist *dst; -dma_async_tx_callback callback; +dma_async_tx_callback_result callback;u16 mdata_size; };
@@ -951,19 +951,21 @@ static void sa_free_sa_rx_data(struct sa_rx_data *rxd)kfree (rxd); } -static void sa_aes_dma_in_callback (void *data) +static void sa_aes_dma_in_callback (void *data, +const struct dmaengine_result *dma_result) {struct sa_rx_data *rxd = data;struct skcipher_request *req;u8 *result;__be32 *mdptr;size_t ml, pl; +int err = dma_result->result ==DMA_TRANS_NOERROR && !dma_result->residue ?0 : -EIO ;int i;sa_sync_from_device (rxd); req = container_of(rxd->req, struct skcipher_request, base); -if (req->iv) { +if (!err && req->iv) { mdptr = (__be32 *)dmaengine_desc_get_metadata_ptr (rxd->tx_in, &pl, &ml); result = req->iv;
@@ -975,7 +977,7 @@ static void sa_aes_dma_in_callback(void *data)sa_free_sa_rx_data (rxd); -skcipher_request_complete (req,0 ); +skcipher_request_complete (req, err); } static void
@@ -1179,7 +1181,7 @@ static int sa_run(struct sa_req *req) rxd->enc = req->enc; rxd->iv_idx = req->ctx->iv_idx; rxd->enc_iv_size = cmdl_upd_info.enc_iv.size; - rxd->tx_in->callback = req->callback; + rxd->tx_in->callback_result = req->callback; rxd->tx_in->callback_param = rxd; tx_out = dmaengine_prep_slave_sg(pdata->dma_tx, src,
@@ -1274,7 +1276,8 @@ static int sa_decrypt(struct skcipher_request *req)return sa_cipher_run (req, req->iv,0 ); } -static void sa_sha_dma_in_callback (void *data) +static void sa_sha_dma_in_callback (void *data, +const struct dmaengine_result *dma_result) {struct sa_rx_data *rxd = data;struct ahash_request *req;
@@ -1284,9 +1287,12 @@ static void sa_sha_dma_in_callback(void *data)size_t ml, pl;u32 *result;__be32 *mdptr; +int err = dma_result->result ==DMA_TRANS_NOERROR && !dma_result->residue ?0 : -EIO ;sa_sync_from_device (rxd); req = container_of(rxd->req, struct ahash_request, base); +if (err) + goto out; tfm =crypto_ahash_reqtfm (req); authsize =crypto_ahash_digestsize (tfm);
@@ -1296,9 +1302,10 @@ static void sa_sha_dma_in_callback(void *data)for (i =0 ; i < (authsize /4 ); i++) result[i] =be32_to_cpu (mdptr[i +4 ]); +out:sa_free_sa_rx_data (rxd); -ahash_request_complete (req,0 ); +ahash_request_complete (req, err); }static int zero_message_process (struct ahash_request *req)
@@ -1584,7 +1591,8 @@ static void sa_sha_cra_exit(struct crypto_tfm *tfm)sa_free_ctx_info (&ctx->enc, data); } -static void sa_aead_dma_in_callback (void *data) +static void sa_aead_dma_in_callback (void *data, +const struct dmaengine_result *dma_result) {struct sa_rx_data *rxd = data;struct aead_request *req;
@@ -1594,11 +1602,13 @@ static void sa_aead_dma_in_callback(void *data)u8 auth_tag[SA_MAX_AUTH_TAG_SZ ];size_t pl, ml;int i; -int err =0 ; +int err = dma_result->result ==DMA_TRANS_NOERROR && !dma_result->residue ?0 : -EIO ;u32 *mdptr;sa_sync_from_device (rxd); req = container_of(rxd->req, struct aead_request, base); +if (err) + goto out; tfm =crypto_aead_reqtfm (req); start = req->assoclen + req->cryptlen; authsize =crypto_aead_authsize (tfm);
@@ -1618,6 +1628,7 @@ static void sa_aead_dma_in_callback(void *data) err =crypto_memneq (&mdptr[4 ], auth_tag, authsize) ? -EBADMSG :0 ; } +out:sa_free_sa_rx_data (rxd);aead_request_complete (req, err);
Signature · 1 line
2.43.0
PATCH v2 24/30 crypto: sa2ul - calculate DMA metadata length from the command label
In reply to Manorit ChawdhryRequest type and subtype are software bookkeeping, not command-label data. Remove that trailer and calculate the DMA metadata length from the EPIB, protocol-specific header and actual command-label size.
With those extra bytes no longer sent through DMA, update the 3DES IV index from 6 to 4 to match the new offsets.
Fixes 7694b6ca649f ("crypto: sa2ul - Add crypto driver")
Fixes d2c8ac187fc9 ("crypto: sa2ul - Add AEAD algorithm support")
Signed-off-by Manorit Chawdhry <[email protected]>
--- drivers/crypto/sa2ul.c | 25 +++++++------------------ drivers/crypto/sa2ul.h | 23 +---------------------- 2 files changed, 8 insertions(+), 40 deletions(-)
drivers/crypto/sa2ul.c +7 -18
@@ -905,7 +905,7 @@ static int sa_3des_cbc_setkey(struct crypto_skcipher *tfm, const u8 *key, ad.mci_enc = mci_cbc_3des_enc_array; ad.mci_dec = mci_cbc_3des_dec_array; - ad.iv_idx =6 ; + ad.iv_idx =4 ; ad.iv_out_size =8 ;return sa_cipher_setkey (tfm, key, keylen, &ad);
@@ -980,7 +980,7 @@ static void sa_aes_dma_in_callback(void *data,skcipher_request_complete (req, err); } -static void +static int sa_prepare_tx_desc (u32 *mdptr,u32 pslen,u32 *psdata,u32 epiblen,u32 *epib) {u32 *out, *in;
@@ -993,6 +993,8 @@ sa_prepare_tx_desc(u32 *mdptr, u32 pslen, u32 *psdata, u32 epiblen, u32 *epib)for (out = &mdptr[5 ], in = psdata, i =0 ; i < pslen /sizeof (u32); i++) *out++ = *in++; + +return epiblen +sizeof (u32) + pslen; }static int sa_run (struct sa_req *req)
@@ -1054,16 +1056,6 @@ static int sa_run(struct sa_req *req)sa_update_cmdl (req, cmdl, &cmdl_upd_info); -if (req->type !=CRYPTO_ALG_TYPE_AHASH ) { -if (req->enc) - req->type |= - (SA_REQ_SUBTYPE_ENC <<SA_REQ_SUBTYPE_SHIFT ); -else - req->type |= - (SA_REQ_SUBTYPE_DEC <<SA_REQ_SUBTYPE_SHIFT ); - } - - cmdl[sa_ctx->cmdl_size /sizeof (u32)] = req->type;/* * Map the packets, first we check if the data fits into a single
@@ -1200,9 +1192,9 @@ static int sa_run(struct sa_req *req)*/ mdptr = (u32 *)dmaengine_desc_get_metadata_ptr (tx_out, &pl, &ml); -sa_prepare_tx_desc (mdptr, (sa_ctx->cmdl_size + (SA_PSDATA_CTX_WORDS * -sizeof (u32))), cmdl,sizeof (sa_ctx->epib), - sa_ctx->epib); + req->mdata_size =sa_prepare_tx_desc (mdptr, cmdl_size, + cmdl,sizeof (sa_ctx->epib), + sa_ctx->epib);dmaengine_desc_set_metadata_len (tx_out, req->mdata_size);
@@ -1259,7 +1251,6 @@ static int sa_cipher_run(struct skcipher_request *req, u8 *iv, int enc) sa_req.type =CRYPTO_ALG_TYPE_SKCIPHER ; sa_req.enc = enc; sa_req.callback = sa_aes_dma_in_callback; - sa_req.mdata_size =44 ; sa_req.base = &req->base; sa_req.ctx = ctx;
@@ -1364,7 +1355,6 @@ static int sa_sha_run(struct ahash_request *req) sa_req.enc = true; sa_req.type =CRYPTO_ALG_TYPE_AHASH ; sa_req.callback = sa_sha_dma_in_callback; - sa_req.mdata_size =28 ; sa_req.ctx = ctx; sa_req.base = &req->base;
@@ -1841,7 +1831,6 @@ static int sa_aead_run(struct aead_request *req, u8 *iv, int enc) sa_req.type =CRYPTO_ALG_TYPE_AEAD ; sa_req.enc = enc; sa_req.callback = sa_aead_dma_in_callback; - sa_req.mdata_size =52 ; sa_req.base = &req->base; sa_req.ctx = ctx; sa_req.src = req->src;
drivers/crypto/sa2ul.h +1 -22
@@ -31,22 +31,9 @@ struct sa_tfm_ctx;#define SA_EEC_CPPI_PORT_IN_EN 0x00000200#define SA_EEC_CPPI_PORT_OUT_EN 0x00000800 -/* -* Encoding used to identify the typo of crypto operation -* performed on the packet when the packet is returned -* by SA -*/ -#define SA_REQ_SUBTYPE_ENC 0x0001 -#define SA_REQ_SUBTYPE_DEC 0x0002 -#define SA_REQ_SUBTYPE_SHIFT 16 -#define SA_REQ_SUBTYPE_MASK 0xffff -/* Number of 32 bit words in EPIB */ #define SA_DMA_NUM_EPIB_WORDS 4 -/* Number of 32 bit words in PS data */ -#define SA_DMA_NUM_PS_WORDS 16 -/* * Maximum number of simultaeneous security contexts * supported by the driver
@@ -216,16 +203,8 @@ struct sa_cmdl_upd_info {struct sa_cmdl_param_info auth_offset; }; -/* -* Number of 32bit words appended after the command label -* in PSDATA to identify the crypto request context. -* word-0: Request type -* word-1: pointer to request -*/ -#define SA_PSDATA_CTX_WORDS 4 -/* Maximum size of Command label in 32 words */ -#define SA_MAX_CMDL_WORDS (SA_DMA_NUM_PS_WORDS - SA_PSDATA_CTX_WORDS) +#define SA_MAX_CMDL_WORDS 24/** * struct sa_ctx_info: SA context information
Signature · 1 line
2.43.0
PATCH v2 25/30 crypto: sa2ul - correct scatterlist DMA ownership transitions
In reply to Manorit ChawdhryUse each scatterlist's recorded mapping direction for CPU/device ownership transitions, including bidirectional in-place buffers.
Completion callbacks modify results after synchronizing them for CPU access. Skip the implicit synchronization on unmap so cache maintenance or bounce-buffer copyback cannot overwrite those CPU modifications.
Fixes 00c9211f60db ("crypto: sa2ul - Fix DMA mapping API usage")
Signed-off-by Manorit Chawdhry <[email protected]>
--- drivers/crypto/sa2ul.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
drivers/crypto/sa2ul.c +9 -5
@@ -925,13 +925,17 @@ static int sa_3des_ecb_setkey(struct crypto_skcipher *tfm, const u8 *key,static void sa_sync_from_device (struct sa_rx_data *rxd) {struct sg_table *sgt; +u32 dir; -if (rxd->mapped_sg[0 ].dir ==DMA_BIDIRECTIONAL ) +if (rxd->mapped_sg[0 ].dir ==DMA_BIDIRECTIONAL ) { sgt = &rxd->mapped_sg[0 ].sgt; -else + dir = rxd->mapped_sg[0 ].dir; + }else { sgt = &rxd->mapped_sg[1 ].sgt; + dir = rxd->mapped_sg[1 ].dir; + } -dma_sync_sgtable_for_cpu (rxd->ddev, sgt,DMA_FROM_DEVICE ); +dma_sync_sgtable_for_cpu (rxd->ddev, sgt, dir); }static void sa_free_sa_rx_data (struct sa_rx_data *rxd)
@@ -943,7 +947,7 @@ static void sa_free_sa_rx_data(struct sa_rx_data *rxd)if (mapped_sg->mapped) {dma_unmap_sgtable (rxd->ddev, &mapped_sg->sgt, - mapped_sg->dir,0 ); + mapped_sg->dir,DMA_ATTR_SKIP_CPU_SYNC );kfree (mapped_sg->split_sg); } }
@@ -1110,7 +1114,7 @@ static int sa_run(struct sa_req *req) mapped_sg->split_sg = src; } -dma_sync_sgtable_for_device (ddev, &mapped_sg->sgt,DMA_TO_DEVICE ); +dma_sync_sgtable_for_device (ddev, &mapped_sg->sgt, dir_src);if (!diff_dst) { dst_nents = src_nents;
Signature · 1 line
2.43.0
PATCH v2 26/30 crypto: sa2ul - use a mempool for security contexts
In reply to Manorit ChawdhryThe SA2UL engine is a DMA master, so any memory it accesses must either be allocated coherent or be explicitly synced with the CPU cache around each access.
The security context buffer is currently allocated from a DMA pool via dma_pool_alloc(), which assumes coherent memory, which is untrue at a bus level for many of our SoCs. This causes any fetch issues from SA2UL to end up getting some stale or unsynced data, causing failures for any crypto operations.
To remove this, allocate the security context from a kmalloc-backed mempool instead. Since this memory is now ordinary cacheable kernel memory, explicitly call dma_map_single()/dma_unmap_single() for DMA-related operations.
Signed-off-by Manorit Chawdhry <[email protected]>
--- drivers/crypto/sa2ul.c | 36 +++++++++++++++++++++++++----------- drivers/crypto/sa2ul.h | 2 +- 2 files changed, 26 insertions(+), 12 deletions(-)
drivers/crypto/sa2ul.c +25 -11
@@ -13,6 +13,7 @@#include <linux/dmaengine.h> #include <linux/dmapool.h> #include <linux/kernel.h> +#include <linux/mempool.h> #include <linux/module.h> #include <linux/unaligned.h> #include <linux/of.h>
@@ -629,12 +630,16 @@ int sa_init_sc(struct sa_ctx_info *ctx, const struct sa_match_data *match_data, const u8 *auth_key, unsigned int auth_key_sz, struct algo_data *ad, u8 enc, u32 *swinfo) { + struct sa_crypto_data *data = dev_get_drvdata(sa_k3_dev); +struct device *dev = &data->pdev->dev; +int enc_sc_offset =0 ;int auth_sc_offset =0 ;u8 *sc_buf = ctx->sc;u16 sc_id = ctx->sc_id;u8 first_engine =0 ; +dma_sync_single_for_cpu (dev, ctx->sc_phys,SA_CTX_MAX_SZ ,DMA_BIDIRECTIONAL );memzero_explicit (sc_buf,SA_CTX_MAX_SZ );if (ad->auth_eng.eng_id) {
@@ -682,11 +687,13 @@ int sa_init_sc(struct sa_ctx_info *ctx, const struct sa_match_data *match_data,/* swizzle the security context */ sa_swiz_128 (sc_buf,SA_CTX_MAX_SZ ); +sa_dump_sc (sc_buf, ctx->sc_phys); + +dma_sync_single_for_device (dev, ctx->sc_phys,SA_CTX_MAX_SZ ,DMA_BIDIRECTIONAL ); +sa_set_swinfo (first_engine, ctx->sc_id, ctx->sc_phys,1 ,0 ,SA_SW_INFO_FLAG_EVICT ,roundup (ad->hash_size,8 ), swinfo); -sa_dump_sc (sc_buf, ctx->sc_phys); -return 0 ; }
@@ -694,6 +701,7 @@ int sa_init_sc(struct sa_ctx_info *ctx, const struct sa_match_data *match_data,static void sa_free_ctx_info (struct sa_ctx_info *ctx,struct sa_crypto_data *data) { +struct device *dev = &data->pdev->dev;unsigned long bn; bn = ctx->sc_id;
@@ -703,8 +711,9 @@ static void sa_free_ctx_info(struct sa_ctx_info *ctx,spin_unlock (&data->scid_lock);if (ctx->sc) { +dma_unmap_single (dev, ctx->sc_phys,SA_CTX_MAX_SZ ,DMA_BIDIRECTIONAL );memzero_explicit (ctx->sc,SA_CTX_MAX_SZ ); -dma_pool_free (data->sc_pool, ctx->sc, ctx->sc_phys); +mempool_free (ctx->sc, data->sc_pool); ctx->sc =NULL ; } }
@@ -713,7 +722,8 @@ static int sa_init_ctx_info(struct sa_ctx_info *ctx, struct sa_crypto_data *data) {unsigned long bn; -int err; +int err =0 ; +struct device *dev = &data->pdev->dev;spin_lock (&data->scid_lock); bn =find_first_zero_bit (data->ctx_bm,SA_MAX_NUM_CTX );
@@ -726,10 +736,15 @@ static int sa_init_ctx_info(struct sa_ctx_info *ctx,spin_unlock (&data->scid_lock); ctx->sc_id = (u16 )bn; + ctx->sc_phys =0 ; - ctx->sc =dma_pool_alloc (data->sc_pool,GFP_KERNEL , &ctx->sc_phys); -if (!ctx->sc) { -dev_err (&data->pdev->dev,"Failed to allocate SC memory\n" ); + ctx->sc =mempool_alloc (data->sc_pool,GFP_KERNEL ); + + ctx->sc_phys =dma_map_single (dev, ctx->sc,SA_CTX_MAX_SZ ,DMA_BIDIRECTIONAL ); +if (dma_mapping_error (dev, ctx->sc_phys)) { +dev_err (dev,"Failed to map SC DMA\n" ); +mempool_free (ctx->sc, data->sc_pool); + ctx->sc =NULL ; err = -ENOMEM ; goto scid_rollback; }
@@ -2153,8 +2168,7 @@ static int sa_init_mem(struct sa_crypto_data *dev_data) {struct device *dev = &dev_data->pdev->dev;/* Setup dma pool for security context buffers */ - dev_data->sc_pool =dma_pool_create ("keystone-sc" , dev, -SA_CTX_MAX_SZ ,64 ,0 ); + dev_data->sc_pool =mempool_create_kmalloc_pool (64 ,SA_CTX_MAX_SZ );if (!dev_data->sc_pool) {dev_err (dev,"Failed to create dma pool" );return -ENOMEM ;
@@ -2348,7 +2362,7 @@ static int sa_ul_probe(struct platform_device *pdev)dma_release_channel (dev_data->dma_tx); destroy_dma_pool: -dma_pool_destroy (dev_data->sc_pool); +mempool_destroy (dev_data->sc_pool); disable_pm:pm_runtime_put_sync (dev);
@@ -2369,7 +2383,7 @@ static void sa_ul_remove(struct platform_device *pdev)dma_release_channel (dev_data->dma_rx1);dma_release_channel (dev_data->dma_tx); -dma_pool_destroy (dev_data->sc_pool); +mempool_destroy (dev_data->sc_pool);platform_set_drvdata (pdev,NULL );
drivers/crypto/sa2ul.h +1 -1
@@ -163,7 +163,7 @@ struct sa_match_data;struct sa_crypto_data {const struct sa_match_data *match_data;struct platform_device *pdev; -struct dma_pool *sc_pool; +mempool_t *sc_pool;struct device *dev;spinlock_t scid_lock;/* lock for SC-ID allocation */ /* Security context data */
Signature · 1 line
2.43.0
PATCH v2 27/30 crypto: sa2ul - use synchronous fallbacks and fixed request sizes
In reply to Manorit ChawdhryTo move away from crypto_*_set_reqsize(), use synchronous skcipher fallbacks with stack requests to avoid reserving fallback storage in each hardware request.
On arm64, native HMAC authenc requests need 448 bytes for SHA1 and 488 bytes for SHA256, exceeding crypto_alloc_sync_aead()'s 384-byte limit. Since the sa2ul driver registers these authenc requests, keep ordinary AEAD fallbacks with dynamically sized request storage.
For the other requests, start using cra_reqsize, keeping its embedded fallback request for incremental hashing.
Also, while at it, switch hash transform callbacks to init_tfm and exit_tfm.
Signed-off-by Manorit Chawdhry <[email protected]>
--- drivers/crypto/sa2ul.c | 131 ++++++++++++++++++++++++------------------------- drivers/crypto/sa2ul.h | 2 +- 2 files changed, 65 insertions(+), 68 deletions(-)
drivers/crypto/sa2ul.c +64 -67
@@ -760,7 +760,7 @@ static int sa_init_ctx_info(struct sa_ctx_info *ctx,return err; } -static void sa_cipher_cra_exit (struct crypto_skcipher *tfm) +static void sa_cipher_exit_tfm (struct crypto_skcipher *tfm) {struct sa_tfm_ctx *ctx =crypto_skcipher_ctx (tfm);struct sa_crypto_data *data =dev_get_drvdata (sa_k3_dev);
@@ -772,15 +772,15 @@ static void sa_cipher_cra_exit(struct crypto_skcipher *tfm)sa_free_ctx_info (&ctx->enc, data);sa_free_ctx_info (&ctx->dec, data); -crypto_free_skcipher (ctx->fallback.skcipher); +crypto_free_sync_skcipher (ctx->fallback.skcipher); } -static int sa_cipher_cra_init (struct crypto_skcipher *tfm) +static int sa_cipher_init_tfm (struct crypto_skcipher *tfm) {struct sa_tfm_ctx *ctx =crypto_skcipher_ctx (tfm);struct sa_crypto_data *data =dev_get_drvdata (sa_k3_dev);const char *name =crypto_tfm_alg_name (&tfm->base); -struct crypto_skcipher *child; +struct crypto_sync_skcipher *child;int ret;memzero_explicit (ctx,sizeof (*ctx));
@@ -793,7 +793,7 @@ static int sa_cipher_cra_init(struct crypto_skcipher *tfm)if (ret) goto ctx_dec_err; - child =crypto_alloc_skcipher (name,0 ,CRYPTO_ALG_NEED_FALLBACK ); + child =crypto_alloc_sync_skcipher (name,0 ,CRYPTO_ALG_NEED_FALLBACK );if (IS_ERR (child)) {dev_err (sa_k3_dev,"Error allocating fallback algo %s\n" , name); ret =PTR_ERR (child);
@@ -801,8 +801,6 @@ static int sa_cipher_cra_init(struct crypto_skcipher *tfm) } ctx->fallback.skcipher = child; -crypto_skcipher_set_reqsize (tfm,crypto_skcipher_reqsize (child) + -sizeof (struct skcipher_request )); dev_dbg(sa_k3_dev, "%s(0x%p) sc-ids(0x%x(0x%pad), 0x%x(0x%pad))\n", __func__, tfm, ctx->enc.sc_id, &ctx->enc.sc_phys,
@@ -819,7 +817,7 @@ static int sa_cipher_setkey(struct crypto_skcipher *tfm, const u8 *key, unsigned int keylen, struct algo_data *ad) { struct sa_tfm_ctx *ctx = crypto_skcipher_ctx(tfm); -struct crypto_skcipher *child = ctx->fallback.skcipher; +struct crypto_sync_skcipher *child = ctx->fallback.skcipher;int cmdl_len;struct sa_cmdl_cfg cfg;int ret;
@@ -835,10 +833,10 @@ static int sa_cipher_setkey(struct crypto_skcipher *tfm, const u8 *key, cfg.enc_eng_id = ad->enc_eng.eng_id; cfg.iv_size =crypto_skcipher_ivsize (tfm); -crypto_skcipher_clear_flags (child,CRYPTO_TFM_REQ_MASK ); -crypto_skcipher_set_flags (child, tfm->base.crt_flags & +crypto_sync_skcipher_clear_flags (child,CRYPTO_TFM_REQ_MASK ); +crypto_sync_skcipher_set_flags (child, tfm->base.crt_flags &CRYPTO_TFM_REQ_MASK ); - ret =crypto_skcipher_setkey (child, key, keylen); + ret =crypto_sync_skcipher_setkey (child, key, keylen);if (ret)return ret;
@@ -1248,18 +1246,16 @@ static int sa_cipher_run(struct skcipher_request *req, u8 *iv, int enc)if (req->cryptlen >SA_MAX_DATA_SZ || (req->cryptlen >=SA_UNSAFE_DATA_SZ_MIN && req->cryptlen <=SA_UNSAFE_DATA_SZ_MAX )) { -struct skcipher_request *subreq =skcipher_request_ctx (req); +SYNC_SKCIPHER_REQUEST_ON_STACK (subreq, ctx->fallback.skcipher); +int ret; -skcipher_request_set_tfm (subreq, ctx->fallback.skcipher); -skcipher_request_set_callback (subreq, req->base.flags, - req->base.complete, - req->base.data); +skcipher_request_set_callback (subreq, req->base.flags,NULL ,NULL );skcipher_request_set_crypt (subreq, req->src, req->dst, req->cryptlen, req->iv); -if (enc) -return crypto_skcipher_encrypt (subreq); -else -return crypto_skcipher_decrypt (subreq); + ret = enc ?crypto_skcipher_encrypt (subreq) : +crypto_skcipher_decrypt (subreq); +skcipher_request_zero (subreq); +return ret; } sa_req.size = req->cryptlen;
@@ -1415,9 +1411,9 @@ static int sa_sha_setup(struct sa_tfm_ctx *ctx, struct algo_data *ad)return -EINVAL ; } -static int sa_sha_cra_init_alg (struct crypto_tfm *tfm,const char *alg_base) +static int sa_sha_init_tfm (struct crypto_ahash *tfm,const char *alg_base) { -struct sa_tfm_ctx *ctx =crypto_tfm_ctx (tfm); +struct sa_tfm_ctx *ctx =crypto_ahash_ctx (tfm);struct sa_crypto_data *data =dev_get_drvdata (sa_k3_dev);int ret;
@@ -1444,10 +1440,6 @@ static int sa_sha_cra_init_alg(struct crypto_tfm *tfm, const char *alg_base) __func__, tfm, ctx->enc.sc_id, &ctx->enc.sc_phys, ctx->dec.sc_id, &ctx->dec.sc_phys); -crypto_ahash_set_reqsize (__crypto_ahash_cast (tfm), -sizeof (struct sa_sha_req_ctx ) + -crypto_ahash_reqsize (ctx->fallback.ahash)); -return 0 ; err_free_ctx:
@@ -1455,7 +1447,7 @@ static int sa_sha_cra_init_alg(struct crypto_tfm *tfm, const char *alg_base)return ret; } -static void sa_sha_cra_exit (struct crypto_tfm *tfm); +static void sa_sha_exit_tfm (struct crypto_ahash *tfm);static int sa_sha_init (struct ahash_request *req) {
@@ -1525,13 +1517,13 @@ static int sa_sha_export(struct ahash_request *req, void *out)return crypto_ahash_export (subreq, out); } -static int sa_sha1_cra_init (struct crypto_tfm *tfm) +static int sa_sha1_init_tfm (struct crypto_ahash *tfm) {struct algo_data ad = {0 }; -struct sa_tfm_ctx *ctx =crypto_tfm_ctx (tfm); +struct sa_tfm_ctx *ctx =crypto_ahash_ctx (tfm);int ret =0 ; - ret =sa_sha_cra_init_alg (tfm,"sha1" ); + ret =sa_sha_init_tfm (tfm,"sha1" );if (ret)return ret;
@@ -1540,18 +1532,18 @@ static int sa_sha1_cra_init(struct crypto_tfm *tfm) ret =sa_sha_setup (ctx, &ad);if (ret) -sa_sha_cra_exit (tfm); +sa_sha_exit_tfm (tfm);return ret; } -static int sa_sha256_cra_init (struct crypto_tfm *tfm) +static int sa_sha256_init_tfm (struct crypto_ahash *tfm) {struct algo_data ad = {0 }; -struct sa_tfm_ctx *ctx =crypto_tfm_ctx (tfm); +struct sa_tfm_ctx *ctx =crypto_ahash_ctx (tfm);int ret =0 ; - ret =sa_sha_cra_init_alg (tfm,"sha256" ); + ret =sa_sha_init_tfm (tfm,"sha256" );if (ret)return ret;
@@ -1560,18 +1552,18 @@ static int sa_sha256_cra_init(struct crypto_tfm *tfm) ret =sa_sha_setup (ctx, &ad);if (ret) -sa_sha_cra_exit (tfm); +sa_sha_exit_tfm (tfm);return ret; } -static int sa_sha512_cra_init (struct crypto_tfm *tfm) +static int sa_sha512_init_tfm (struct crypto_ahash *tfm) {struct algo_data ad = {0 }; -struct sa_tfm_ctx *ctx =crypto_tfm_ctx (tfm); +struct sa_tfm_ctx *ctx =crypto_ahash_ctx (tfm);int ret =0 ; - ret =sa_sha_cra_init_alg (tfm,"sha512" ); + ret =sa_sha_init_tfm (tfm,"sha512" );if (ret)return ret;
@@ -1580,14 +1572,14 @@ static int sa_sha512_cra_init(struct crypto_tfm *tfm) ret =sa_sha_setup (ctx, &ad);if (ret) -sa_sha_cra_exit (tfm); +sa_sha_exit_tfm (tfm);return ret; } -static void sa_sha_cra_exit (struct crypto_tfm *tfm) +static void sa_sha_exit_tfm (struct crypto_ahash *tfm) { -struct sa_tfm_ctx *ctx =crypto_tfm_ctx (tfm); +struct sa_tfm_ctx *ctx =crypto_ahash_ctx (tfm);struct sa_crypto_data *data =dev_get_drvdata (sa_k3_dev); dev_dbg(sa_k3_dev, "%s(0x%p) sc-ids(0x%x(0x%pad), 0x%x(0x%pad))\n",
@@ -1596,8 +1588,7 @@ static void sa_sha_cra_exit(struct crypto_tfm *tfm)crypto_free_ahash (ctx->fallback.ahash); -if (crypto_tfm_alg_type (tfm) ==CRYPTO_ALG_TYPE_AHASH ) -sa_free_ctx_info (&ctx->enc, data); +sa_free_ctx_info (&ctx->enc, data); } static void sa_aead_dma_in_callback(void *data,
@@ -1643,7 +1634,7 @@ static void sa_aead_dma_in_callback(void *data,aead_request_complete (req, err); } -static int sa_cra_init_aead (struct crypto_aead *tfm,const char *fallback) +static int sa_aead_init_tfm (struct crypto_aead *tfm,const char *fallback) {struct sa_tfm_ctx *ctx =crypto_aead_ctx (tfm);struct sa_crypto_data *data =dev_get_drvdata (sa_k3_dev);
@@ -1685,17 +1676,17 @@ static int sa_cra_init_aead(struct crypto_aead *tfm, const char *fallback)return ret; } -static int sa_cra_init_aead_sha1 (struct crypto_aead *tfm) +static int sa_aead_sha1_init_tfm (struct crypto_aead *tfm) { -return sa_cra_init_aead (tfm,"authenc(hmac(sha1),cbc(aes))" ); +return sa_aead_init_tfm (tfm,"authenc(hmac(sha1),cbc(aes))" ); } -static int sa_cra_init_aead_sha256 (struct crypto_aead *tfm) +static int sa_aead_sha256_init_tfm (struct crypto_aead *tfm) { -return sa_cra_init_aead (tfm,"authenc(hmac(sha256),cbc(aes))" ); +return sa_aead_init_tfm (tfm,"authenc(hmac(sha256),cbc(aes))" ); } -static void sa_exit_tfm_aead (struct crypto_aead *tfm) +static void sa_aead_exit_tfm (struct crypto_aead *tfm) {struct sa_tfm_ctx *ctx =crypto_aead_ctx (tfm);struct sa_crypto_data *data =dev_get_drvdata (sa_k3_dev);
@@ -1884,8 +1875,8 @@ static struct sa_alg_tmpl sa_algs[] = { .base.cra_blocksize = AES_BLOCK_SIZE, .base.cra_ctxsize = sizeof(struct sa_tfm_ctx), .base.cra_module = THIS_MODULE, - .init = sa_cipher_cra_init, - .exit = sa_cipher_cra_exit, + .init = sa_cipher_init_tfm, + .exit = sa_cipher_exit_tfm, .min_keysize = AES_MIN_KEY_SIZE, .max_keysize = AES_MAX_KEY_SIZE, .ivsize = AES_BLOCK_SIZE,
@@ -1907,8 +1898,8 @@ static struct sa_alg_tmpl sa_algs[] = { .base.cra_blocksize = AES_BLOCK_SIZE, .base.cra_ctxsize = sizeof(struct sa_tfm_ctx), .base.cra_module = THIS_MODULE, - .init = sa_cipher_cra_init, - .exit = sa_cipher_cra_exit, + .init = sa_cipher_init_tfm, + .exit = sa_cipher_exit_tfm, .min_keysize = AES_MIN_KEY_SIZE, .max_keysize = AES_MAX_KEY_SIZE, .setkey = sa_aes_ecb_setkey,
@@ -1929,8 +1920,8 @@ static struct sa_alg_tmpl sa_algs[] = { .base.cra_blocksize = DES_BLOCK_SIZE, .base.cra_ctxsize = sizeof(struct sa_tfm_ctx), .base.cra_module = THIS_MODULE, - .init = sa_cipher_cra_init, - .exit = sa_cipher_cra_exit, + .init = sa_cipher_init_tfm, + .exit = sa_cipher_exit_tfm, .min_keysize = 3 * DES_KEY_SIZE, .max_keysize = 3 * DES_KEY_SIZE, .ivsize = DES_BLOCK_SIZE,
@@ -1952,8 +1943,8 @@ static struct sa_alg_tmpl sa_algs[] = { .base.cra_blocksize = DES_BLOCK_SIZE, .base.cra_ctxsize = sizeof(struct sa_tfm_ctx), .base.cra_module = THIS_MODULE, - .init = sa_cipher_cra_init, - .exit = sa_cipher_cra_exit, + .init = sa_cipher_init_tfm, + .exit = sa_cipher_exit_tfm, .min_keysize = 3 * DES_KEY_SIZE, .max_keysize = 3 * DES_KEY_SIZE, .setkey = sa_3des_ecb_setkey,
@@ -1974,13 +1965,15 @@ static struct sa_alg_tmpl sa_algs[] = { CRYPTO_ALG_NEED_FALLBACK, .cra_blocksize = SHA1_BLOCK_SIZE, .cra_ctxsize = sizeof(struct sa_tfm_ctx), + .cra_reqsize =sizeof (struct sa_sha_req_ctx ) + + MAX_SYNC_HASH_REQSIZE, .cra_module = THIS_MODULE, - .cra_init = sa_sha1_cra_init, - .cra_exit = sa_sha_cra_exit, }, .halg.digestsize = SHA1_DIGEST_SIZE, .halg.statesize =sizeof (struct sa_sha_req_ctx ) + sizeof(struct sha1_state), + .init_tfm = sa_sha1_init_tfm, + .exit_tfm = sa_sha_exit_tfm, .init = sa_sha_init, .update = sa_sha_update, .final = sa_sha_final,
@@ -2003,13 +1996,15 @@ static struct sa_alg_tmpl sa_algs[] = { CRYPTO_ALG_NEED_FALLBACK, .cra_blocksize = SHA256_BLOCK_SIZE, .cra_ctxsize = sizeof(struct sa_tfm_ctx), + .cra_reqsize =sizeof (struct sa_sha_req_ctx ) + + MAX_SYNC_HASH_REQSIZE, .cra_module = THIS_MODULE, - .cra_init = sa_sha256_cra_init, - .cra_exit = sa_sha_cra_exit, }, .halg.digestsize = SHA256_DIGEST_SIZE, .halg.statesize =sizeof (struct sa_sha_req_ctx ) + sizeof(struct sha256_state), + .init_tfm = sa_sha256_init_tfm, + .exit_tfm = sa_sha_exit_tfm, .init = sa_sha_init, .update = sa_sha_update, .final = sa_sha_final,
@@ -2032,13 +2027,15 @@ static struct sa_alg_tmpl sa_algs[] = { CRYPTO_ALG_NEED_FALLBACK, .cra_blocksize = SHA512_BLOCK_SIZE, .cra_ctxsize = sizeof(struct sa_tfm_ctx), + .cra_reqsize =sizeof (struct sa_sha_req_ctx ) + + MAX_SYNC_HASH_REQSIZE, .cra_module = THIS_MODULE, - .cra_init = sa_sha512_cra_init, - .cra_exit = sa_sha_cra_exit, }, .halg.digestsize = SHA512_DIGEST_SIZE, .halg.statesize =sizeof (struct sa_sha_req_ctx ) + sizeof(struct sha512_state), + .init_tfm = sa_sha512_init_tfm, + .exit_tfm = sa_sha_exit_tfm, .init = sa_sha_init, .update = sa_sha_update, .final = sa_sha_final,
@@ -2067,8 +2064,8 @@ static struct sa_alg_tmpl sa_algs[] = { .ivsize = AES_BLOCK_SIZE, .maxauthsize = SHA1_DIGEST_SIZE, - .init = sa_cra_init_aead_sha1, - .exit = sa_exit_tfm_aead, + .init = sa_aead_sha1_init_tfm, + .exit = sa_aead_exit_tfm, .setkey = sa_aead_cbc_sha1_setkey, .setauthsize = sa_aead_setauthsize, .encrypt = sa_aead_encrypt,
@@ -2095,8 +2092,8 @@ static struct sa_alg_tmpl sa_algs[] = { .ivsize = AES_BLOCK_SIZE, .maxauthsize = SHA256_DIGEST_SIZE, - .init = sa_cra_init_aead_sha256, - .exit = sa_exit_tfm_aead, + .init = sa_aead_sha256_init_tfm, + .exit = sa_aead_exit_tfm, .setkey = sa_aead_cbc_sha256_setkey, .setauthsize = sa_aead_setauthsize, .encrypt = sa_aead_encrypt,
drivers/crypto/sa2ul.h +1 -1
@@ -242,7 +242,7 @@ struct sa_tfm_ctx {int iv_idx;/* for fallback */ union { -struct crypto_skcipher *skcipher; +struct crypto_sync_skcipher *skcipher;struct crypto_ahash *ahash;struct crypto_aead *aead; } fallback;
Signature · 1 line
2.43.0
PATCH v2 28/30 crypto: sa2ul - route requests through crypto_engine
In reply to Manorit ChawdhryUse crypto_engine to queue skcipher, ahash and AEAD requests and process them in a sleepable context. Keep the cipher and AEAD operation direction in each request context until submission.
Allocate and start the engine during probe, and release it on teardown.
Signed-off-by Manorit Chawdhry <[email protected]>
--- drivers/crypto/Kconfig | 1 + drivers/crypto/sa2ul.c | 646 ++++++++++++++++++++++++++++++++----------------- drivers/crypto/sa2ul.h | 28 +++ 3 files changed, 454 insertions(+), 221 deletions(-)
drivers/crypto/Kconfig +1 -0
@@ -816,6 +816,7 @@ config CRYPTO_DEV_SA2UL select CRYPTO_ALGAPI select CRYPTO_AUTHENC select CRYPTO_DES + select CRYPTO_ENGINE select CRYPTO_SHA1 select CRYPTO_SHA256 select CRYPTO_SHA512
drivers/crypto/sa2ul.c +425 -221
923 lines not on this page · raw message
drivers/crypto/sa2ul.h +28 -0
@@ -15,6 +15,7 @@#include <crypto/aes.h> #include <crypto/sha1.h> #include <crypto/sha2.h> +#include <linux/mutex.h> #define SA_ENGINE_STATUS 0x0008#define SA_ENGINE_ENABLE_CONTROL 0x1000
@@ -146,6 +147,7 @@ struct sa_tfm_ctx;#define SA_UNSAFE_DATA_SZ_MAX 255struct sa_match_data ; +struct sa_rx_data ;/** * struct sa_crypto_data - Crypto driver instance data
@@ -159,6 +161,11 @@ struct sa_match_data;* @dma_rx1: Pointer to DMA rx channel for sizes < 256 Bytes * @dma_rx2: Pointer to DMA rx channel for sizes > 256 Bytes * @dma_tx: Pointer to DMA TX channel +* @engine: crypto_engine instance for this device +* @rxd: DMA state for the active engine request +* @request_lock: protects request admission against teardown +* @dma_lock: serializes DMA setup against cancellation +* @stopping: reject hardware requests during teardown */ struct sa_crypto_data {const struct sa_match_data *match_data;
@@ -173,6 +180,11 @@ struct sa_crypto_data {struct dma_chan *dma_rx1;struct dma_chan *dma_rx2;struct dma_chan *dma_tx; +struct crypto_engine *engine; +struct sa_rx_data *rxd; +spinlock_t request_lock;/* request admission */ +struct mutex dma_lock;/* DMA setup and cancellation */ +bool stopping; };/**
@@ -256,6 +268,22 @@ struct sa_sha_req_ctx {struct ahash_request fallback_req; }; +/** +* struct sa_cipher_req_ctx: Structure used for skcipher request +* @enc: True if this request is an encrypt operation +*/ +struct sa_cipher_req_ctx { +bool enc; +}; + +/** +* struct sa_aead_req_ctx: Structure used for aead request +* @enc: True if this request is an encrypt operation +*/ +struct sa_aead_req_ctx { +bool enc; +}; +/** * struct sa_eng_info: Security accelerator engine info * @eng_id: Engine ID
Signature · 1 line
2.43.0
[email protected]
AI reviewRe: [PATCH v2 28/30] crypto: sa2ul - route requests through crypto_engine
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] crypto: sa2ul: unused-but-set variable enc_size in sa_aead_run() --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] crypto: sa2ul: unused-but-set variable enc_size in sa_aead_run()
--
commit f24f2777678674102188bb8e0c6e2374169ce53e
Author: Manorit Chawdhry <[email protected]>
crypto: sa2ul - route requests through crypto_engine
This commit routes skcipher, ahash, and AEAD requests in the SA2UL driver through crypto_engine so that request setup and submission run in a sleepable context.
Quoted 4 lines
drivers/crypto/sa2ul.c +0
-0
[ ... ]
Quoted 39 lines
drivers/crypto/sa2ul.c +22
-2@@ -1800,12 +1925,14 @@ static int sa_aead_cbc_sha256_setkey(struct crypto_aead *authenc,return sa_aead_setkey (authenc, key, keylen, &ad); } -static int sa_aead_run (struct aead_request *req,u8 *iv,int enc) +static int sa_aead_run (struct aead_request *req,int enc) {struct crypto_aead *tfm =crypto_aead_reqtfm (req);struct sa_tfm_ctx *ctx =crypto_aead_ctx (tfm); -struct sa_req sa_req = {0 }; +struct sa_aead_req_ctx *rctx =aead_request_ctx (req);size_t auth_size, enc_size; +unsigned long flags; +int ret; enc_size = req->cryptlen; auth_size = req->assoclen + req->cryptlen;@@ -1833,271 +1960,330 @@ static int sa_aead_run(struct aead_request *req, u8 *iv, int enc)return ret; } + rctx->enc = enc; + +spin_lock_irqsave (&ctx->dev_data->request_lock, flags); + ret = ctx->dev_data->stopping ? -ESHUTDOWN : +crypto_transfer_aead_request_to_engine (ctx->dev_data->engine, req); +spin_unlock_irqrestore (&ctx->dev_data->request_lock, flags); +return ret; +} + +static int sa_aead_do_one_req (struct crypto_engine *engine,void *areq) +{ + struct aead_request *req = container_of(areq, struct aead_request, base); +struct crypto_aead *tfm =crypto_aead_reqtfm (req); +struct sa_tfm_ctx *ctx =crypto_aead_ctx (tfm); +struct sa_aead_req_ctx *rctx =aead_request_ctx (req); +struct sa_req sa_req = {0 }; +size_t auth_size, enc_size; +
drivers/crypto/sa2ul.c static int sa_aead_run(struct aead_request *req, u8 *iv, int enc) · line 1990
Quoted 10 lines
drivers/crypto/sa2ul.c +8
-0+ enc_size = req->cryptlen; + auth_size = req->assoclen + req->cryptlen; + +if (!rctx->enc) { + enc_size -=crypto_aead_authsize (tfm); + auth_size -=crypto_aead_authsize (tfm); + } + sa_req.enc_offset = req->assoclen; sa_req.enc_size = enc_size;
[Severity: Low]
Now that the struct sa_req setup has been moved to sa_aead_do_one_req(), enc_size in sa_aead_run() is still assigned and decremented when !enc:
enc_size = req->cryptlen;
auth_size = req->assoclen + req->cryptlen;
if (!enc) {
enc_size -= crypto_aead_authsize(tfm);
auth_size -= crypto_aead_authsize(tfm);
}but it is never read anywhere in sa_aead_run(). Does this trigger an -Wunused-but-set-variable warning when building drivers/crypto/sa2ul.o with W=1? Should enc_size be removed from sa_aead_run()?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=28
PATCH v2 29/30 crypto: sa2ul - report SA engine hardware revision
In reply to Manorit ChawdhryThe SA2UL/SA3UL crypto engine exposes a REVISION register at offset 0x0 that encodes the major/minor hardware version (major == 2 for SA2_UL, major == 3 and minor == 0 for SA3_UL, major == 3 and minor == 1 for SA3_UL_CM). This version information is needed to detect the AES-CM (SA3UL_CM) variant so the driver can enable the extra TRNG-priming and AES key-expansion workarounds it requires.
Add the SA_REVISION register offset and its major/minor field masks, read the register during probe, and log the decoded version via dev_info() for diagnostics.
Signed-off-by Manorit Chawdhry <[email protected]>
--- drivers/crypto/sa2ul.c | 8 +++++++- drivers/crypto/sa2ul.h | 9 +++++++++ 2 files changed, 16 insertions(+), 1 deletion(-)
drivers/crypto/sa2ul.c +7 -1
@@ -2480,7 +2480,8 @@ static int sa_ul_probe(struct platform_device *pdev)struct device_node *node = dev->of_node;void __iomem *saul_base;struct sa_crypto_data *dev_data; -u32 status, val; +u32 status, val, rev; +unsigned int major, minor;int ret; dev_data =devm_kzalloc (dev,sizeof (*dev_data),GFP_KERNEL );
@@ -2509,6 +2510,11 @@ static int sa_ul_probe(struct platform_device *pdev)return ret; } + rev =readl_relaxed (saul_base +SA_REVISION ); + major =FIELD_GET (SA_REVISION_MAJOR_MASK , rev); + minor =FIELD_GET (SA_REVISION_MINOR_MASK , rev); +dev_info (dev,"SAxUL_VERSION: %u.%u\n" , major, minor); + ret =sa_init_mem (dev_data);if (ret) goto disable_pm;
drivers/crypto/sa2ul.h +9 -0
@@ -17,9 +17,18 @@#include <crypto/sha2.h> #include <linux/mutex.h> +#define SA_REVISION 0x0000#define SA_ENGINE_STATUS 0x0008#define SA_ENGINE_ENABLE_CONTROL 0x1000 +/* +* SA_REVISION register fields (see REVISION Register, offset 0x0) +* major == 2: SA2_UL, major == 3: SA3_UL (minor 0 == SA3_UL, minor 1 == +* SA3_UL_CM aka SA3_UL v3.1) +*/ +#define SA_REVISION_MAJOR_MASK GENMASK(10, 8) +#define SA_REVISION_MINOR_MASK GENMASK(5, 0) +struct sa_tfm_ctx ;/* * SA_ENGINE_ENABLE_CONTROL register bits
Signature · 1 line
2.43.0
PATCH v2 30/30 crypto: sa2ul - add SA3UL_CM AES support
In reply to Manorit ChawdhrySA3UL_CM revision 3.1 needs an extra key-schedule round for AES-256 decryption and a random AES-ECB operation after power-up to activate its side-channel countermeasures.
Derive round 15 in a separate temporary and wipe it after copying, rather than overflowing the expanded-key array.
Prime the hardware through sa_run() before registering algorithms, so self-tests cannot precede the random operation. Use a temporary context and a callback that releases DMA state and wakes the waiter. Use a DMA result callback so an aborted or short transfer fails probe instead of being treated as successful priming. Bound the wait to ten seconds and terminate DMA synchronously on timeout before releasing the temporary context, buffers and stack-backed completion state.
Signed-off-by Manorit Chawdhry <[email protected]>
--- drivers/crypto/sa2ul.c | 156 ++++++++++++++++++++++++++++++++++++++++++++++++- drivers/crypto/sa2ul.h | 2 + 2 files changed, 156 insertions(+), 2 deletions(-)
drivers/crypto/sa2ul.c +154 -2
@@ -13,6 +13,7 @@#include <linux/dmaengine.h> #include <linux/dmapool.h> #include <linux/interrupt.h> +#include <linux/jiffies.h> #include <linux/kernel.h> #include <linux/mempool.h> #include <linux/module.h>
@@ -21,6 +22,8 @@#include <linux/of_platform.h> #include <linux/platform_device.h> #include <linux/pm_runtime.h> +#include <linux/random.h> +#include <linux/slab.h> #include <crypto/aes.h> #include <crypto/algapi.h>
@@ -401,11 +404,22 @@ static void sa_prepare_iopads(struct algo_data *data, const u8 *key,memzero_explicit (&k,sizeof (k)); } +/* SubWord using kernel AES S-box */ +static inline u32 sa_aes_subword (u32 w) +{ +return (u32 )crypto_aes_sbox[w &0xff ] | + ((u32 )crypto_aes_sbox[(w >>8 ) &0xff ] <<8 ) | + ((u32 )crypto_aes_sbox[(w >>16 ) &0xff ] <<16 ) | + ((u32 )crypto_aes_sbox[(w >>24 ) &0xff ] <<24 ); +} +/* Derive the inverse key used in AES-CBC decryption operation */ -static inline int sa_aes_inv_key (u8 *inv_key,const u8 *key,u16 key_sz) +static inline int sa_aes_inv_key (u8 *inv_key,const u8 *key,u16 key_sz, +bool aes_cm) { struct crypto_aes_ctx ctx __cleanup(aes_zeroize_ctx);int key_pos; +u32 round15[4 ];if (aes_expandkey (&ctx, key, key_sz)) {dev_err (sa_k3_dev,"%s: bad key len(%d)\n" , __func__, key_sz);
@@ -418,6 +432,24 @@ static inline int sa_aes_inv_key(u8 *inv_key, const u8 *key, u16 key_sz) ctx.key_enc[53 ] = ctx.key_enc[52 ] ^ ctx.key_enc[47 ]; } +/* +* SA3UL CM hardware variant needs an extra round 15 for AES-256. +* ctx.key_enc[] only holds 60 words (rounds 0-14), so round 15 is +* computed into a local array. +*/ +if (key_sz ==AES_KEYSIZE_256 && aes_cm) { + round15[0 ] =sa_aes_subword (ctx.key_enc[59 ]) ^ ctx.key_enc[52 ]; + round15[1 ] = round15[0 ] ^ ctx.key_enc[53 ]; + round15[2 ] = round15[1 ] ^ ctx.key_enc[54 ]; + round15[3 ] = round15[2 ] ^ ctx.key_enc[55 ]; + +/* Decrypt key = round14 || round15 */ +memcpy (inv_key, &ctx.key_enc[56 ],16 ); +memcpy (inv_key +16 , round15,16 ); +memzero_explicit (round15,sizeof (round15)); +return 0 ; + } +/* Based crypto_aes_expand_key logic */ switch (key_sz) {case AES_KEYSIZE_128 :
@@ -457,7 +489,10 @@ static int sa_set_sc_enc(struct algo_data *ad, const u8 *key, u16 key_sz,/* For AES-CBC decryption get the inverse key */ if (ad->inv_key && !enc) { -if (sa_aes_inv_key (&sc_buf[SC_ENC_KEY_OFFSET ], key, key_sz)) +struct sa_crypto_data *dev_data =dev_get_drvdata (sa_k3_dev); +bool aes_cm = dev_data ? dev_data->aes_cm : false; + +if (sa_aes_inv_key (&sc_buf[SC_ENC_KEY_OFFSET ], key, key_sz, aes_cm))return -EINVAL ;/* For all other cases: key is used */ }else {
@@ -2474,6 +2509,114 @@ static const struct of_device_id of_match[] = { };MODULE_DEVICE_TABLE (of, of_match); +/* +* SA3UL_CM (aes_cm) requires a one-time TRNG-seeded AES-ECB priming +* operation after every power-up/reset before its side-channel +* countermeasures are considered active. +*/ +static void sa_aes_cm_trng_prime_callback (void *data, +const struct dmaengine_result *result) +{ +struct sa_rx_data *rxd = data; +struct crypto_async_request *base = rxd->req; +int err = result->result ==DMA_TRANS_NOERROR && !result->residue ?0 : -EIO ; + +/* sa_run() bypasses crypto_engine, so complete the wait directly. */ +sa_sync_from_device (rxd); +sa_free_sa_rx_data (rxd); +crypto_req_done (base->data, err); +} + +static int sa_aes_cm_trng_prime (struct sa_crypto_data *dev_data) +{ +struct sa_tfm_ctx *ctx; +struct algo_data ad = {0 }; +struct sa_cmdl_cfg cfg = {0 }; +struct crypto_async_request base = {0 }; +struct crypto_wait wait; +struct sa_req req = {0 }; +struct scatterlist sg; +u8 key[AES_KEYSIZE_128 ] = {0 }; +u8 *pt =NULL ; +bool ctx_ready = false; +int cmdl_len; +int ret; + + ctx =kzalloc_obj (*ctx,GFP_KERNEL ); +if (!ctx) +return -ENOMEM ; + + ctx->dev_data = dev_data; + ret =sa_init_ctx_info (&ctx->enc, dev_data); +if (ret) + goto out; + ctx_ready = true; + + ad.enc_eng.eng_id =SA_ENG_ID_EM1 ; + ad.enc_eng.sc_size =SA_CTX_ENC_TYPE1_SZ ; + ad.mci_enc = mci_ecb_enc_array[0 ]; + +get_random_bytes (key,sizeof (key)); + pt =kmalloc (AES_BLOCK_SIZE ,GFP_KERNEL ); +if (!pt) { + ret = -ENOMEM ; + goto out; + } +get_random_bytes (pt,AES_BLOCK_SIZE ); + + ret =sa_init_sc (&ctx->enc, dev_data->match_data, key,sizeof (key), +NULL ,0 , &ad,1 , &ctx->enc.epib[1 ]); +if (ret) + goto out; + + cfg.enc = true; + cfg.enc_eng_id = ad.enc_eng.eng_id; + cfg.iv_size =0 ; + cmdl_len =sa_format_cmdl_gen (&cfg, (u8 *)ctx->enc.cmdl, + &ctx->enc.cmdl_upd_info); +if (cmdl_len <=0 || cmdl_len >SA_MAX_CMDL_WORDS *sizeof (u32)) { + ret = -EINVAL ; + goto out; + } + ctx->enc.cmdl_size = cmdl_len; + +sg_init_one (&sg, pt,AES_BLOCK_SIZE ); +crypto_init_wait (&wait); + base.flags =CRYPTO_TFM_REQ_MAY_SLEEP ; + base.complete = crypto_req_done; + base.data = &wait; + + req.size =AES_BLOCK_SIZE ; + req.enc_size =AES_BLOCK_SIZE ; + req.type =CRYPTO_ALG_TYPE_SKCIPHER ; + req.base = &base; + req.ctx = ctx; + req.enc = true; + req.src = &sg; + req.dst = &sg; + req.callback = sa_aes_cm_trng_prime_callback; + + ret =sa_run (&req); +if (ret == -EINPROGRESS ) { +if (!wait_for_completion_timeout (&wait.completion, +msecs_to_jiffies (10000 ))) { +/* Stop DMA and late callbacks before releasing the stack wait. */ +sa_cancel_dma (dev_data); + ret = -ETIMEDOUT ; + }else { + ret = wait.err; + } + } + +out: +if (ctx_ready) +sa_free_ctx_info (&ctx->enc, dev_data); +memzero_explicit (key,sizeof (key)); +kfree_sensitive (pt); +kfree_sensitive (ctx); +return ret; +} +static int sa_ul_probe (struct platform_device *pdev) {struct device *dev = &pdev->dev;
@@ -2514,6 +2657,7 @@ static int sa_ul_probe(struct platform_device *pdev) major =FIELD_GET (SA_REVISION_MAJOR_MASK , rev); minor =FIELD_GET (SA_REVISION_MINOR_MASK , rev);dev_info (dev,"SAxUL_VERSION: %u.%u\n" , major, minor); + dev_data->aes_cm = (major ==3 && minor ==1 ); ret =sa_init_mem (dev_data);if (ret)
@@ -2545,6 +2689,14 @@ static int sa_ul_probe(struct platform_device *pdev)if (val & ~status)writel_relaxed (val, saul_base +SA_ENGINE_ENABLE_CONTROL ); +if (dev_data->aes_cm) { + ret =sa_aes_cm_trng_prime (dev_data); +if (ret) { +dev_err_probe (dev, ret,"SA3UL CM TRNG priming failed\n" ); + goto free_engine; + } + } +sa_register_algos (dev_data); ret =of_platform_populate (node,NULL ,NULL , dev);
drivers/crypto/sa2ul.h +2 -0
@@ -194,6 +194,8 @@ struct sa_crypto_data {spinlock_t request_lock;/* request admission */ struct mutex dma_lock;/* DMA setup and cancellation */ bool stopping; +/* true if HW is SA3UL_CM (SA_REVISION major=3 minor=1) */ +bool aes_cm; };/**
Signature · 1 line
2.43.0
[email protected]
AI reviewRe: [PATCH v2 30/30] crypto: sa2ul - add SA3UL_CM AES support
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] crypto: sa2ul: missing kernel-doc entry for aes_cm in struct sa_crypto_data --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] crypto: sa2ul: missing kernel-doc entry for aes_cm in struct sa_crypto_data --
commit bbd9935e2ba3dc8e6f81dc7c3285e99df2e66513
Author: Manorit Chawdhry <[email protected]>
crypto: sa2ul - add SA3UL_CM AES support
This commit adds support for SA3UL_CM revision 3.1 by deriving an extra key-schedule round 15 for AES-256 decryption and running a TRNG-seeded AES-ECB priming operation during probe before registering algorithms.
drivers/crypto/sa2ul.h struct sa_crypto_data { · line 199
Quoted 11 lines
drivers/crypto/sa2ul.h +2
-0@@ -194,6 +194,8 @@ struct sa_crypto_data {spinlock_t request_lock;/* request admission */ struct mutex dma_lock;/* DMA setup and cancellation */ bool stopping; +/* true if HW is SA3UL_CM (SA_REVISION major=3 minor=1) */ +bool aes_cm; };
[Severity: Low]
This isn't a bug, but struct sa_crypto_data in drivers/crypto/sa2ul.h has a kernel-doc comment block above it documenting its members. Should @aes_cm be added to that kernel-doc comment so that running scripts/kernel-doc or building with W=1 does not emit a warning?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=30