From be2421606e3dcaece4ccacbcf738c590e6928a06 Mon Sep 17 00:00:00 2001 From: Yang Shen Date: Fri, 16 Dec 2022 11:32:02 +0800 Subject: [PATCH] libwd: fix a bug for ecc To get more information, please visit the pr: https://github.com/Linaro/uadk/pull/532 Signed-off-by: Yang Shen (cherry picked from commit 1d663ab6195da739d79fb73ee6b137a71a83a717) --- ...ix-buffer-overwritten-of-sm2-decrypt.patch | 97 +++++++++++ 0002-uadk-ecc-cleanup-sm2-dec-out-code.patch | 71 ++++++++ ...cc-bugfix-incorrect-parameter-length.patch | 153 ++++++++++++++++++ ...cc-bugfix-incorrect-parameter-length.patch | 74 +++++++++ warpdrive.spec | 9 +- 5 files changed, 403 insertions(+), 1 deletion(-) create mode 100644 0001-uadk-v1-ecc-bugfix-buffer-overwritten-of-sm2-decrypt.patch create mode 100644 0002-uadk-ecc-cleanup-sm2-dec-out-code.patch create mode 100644 0003-uadk-v1-ecc-bugfix-incorrect-parameter-length.patch create mode 100644 0004-uadk-ecc-bugfix-incorrect-parameter-length.patch diff --git a/0001-uadk-v1-ecc-bugfix-buffer-overwritten-of-sm2-decrypt.patch b/0001-uadk-v1-ecc-bugfix-buffer-overwritten-of-sm2-decrypt.patch new file mode 100644 index 0000000..b33a8c7 --- /dev/null +++ b/0001-uadk-v1-ecc-bugfix-buffer-overwritten-of-sm2-decrypt.patch @@ -0,0 +1,97 @@ +From 01ca714340a06ee7f559ba4086f3173351692413 Mon Sep 17 00:00:00 2001 +From: Zhiqi Song +Date: Thu, 15 Dec 2022 16:08:31 +0800 +Subject: [PATCH 1/4] uadk/v1/ecc: bugfix buffer overwritten of sm2 decryption + +Fix the problem that the input data field 'c1.x' is overwitten +by the output data calculated by the hardware. + +Signed-off-by: Zhiqi Song +--- + v1/drv/hisi_hpre_udrv.c | 24 ++++++++++++------------ + 1 file changed, 12 insertions(+), 12 deletions(-) + +diff --git a/v1/drv/hisi_hpre_udrv.c b/v1/drv/hisi_hpre_udrv.c +index 4a97917..191beab 100644 +--- a/v1/drv/hisi_hpre_udrv.c ++++ b/v1/drv/hisi_hpre_udrv.c +@@ -1969,7 +1969,6 @@ static int fill_sm2_dec_sqe(void *message, struct qm_queue_info *info, __u16 i) + + /* compute d * c1 */ + memcpy(dst, req_src, sizeof(*dst)); +- + dst->op_type = HPRE_SM2_DEC; + *(struct wcrypto_ecc_msg **)(dst + 1) = req_src; + dst->in = (void *)&din->c1; +@@ -2047,8 +2046,8 @@ static int qm_parse_ecc_sqe_general(void *msg, const struct qm_queue_info *info, + + dma_out = DMA_ADDR(hw_msg->hi_out, hw_msg->low_out); + dma_key = DMA_ADDR(hw_msg->hi_key, hw_msg->low_key); +- dma_in = DMA_ADDR(hw_msg->hi_in, hw_msg->hi_in); +- drv_iova_unmap(q, NULL, (void *)(uintptr_t)dma_in, olen); ++ dma_in = DMA_ADDR(hw_msg->hi_in, hw_msg->low_in); ++ drv_iova_unmap(q, NULL, (void *)(uintptr_t)dma_in, ilen); + drv_iova_unmap(q, NULL, (void *)(uintptr_t)dma_out, olen); + drv_iova_unmap(q, NULL, (void *)(uintptr_t)dma_key, kbytes); + +@@ -2265,8 +2264,8 @@ static int is_equal(struct wd_dtb *src, struct wd_dtb *dst) + static int sm2_hash(struct wd_dtb *out, struct wcrypto_ecc_point *x2y2, + struct wd_dtb *msg, struct q_info *q_info) + { +- struct wcrypto_hash_mt *hash = &q_info->hash; + __u64 lens = (__u64)msg->dsize + 2 * (__u64)x2y2->x.dsize; ++ struct wcrypto_hash_mt *hash = &q_info->hash; + char hash_out[MAX_HASH_LENS] = {0}; + __u64 in_len = 0; + __u32 h_bytes; +@@ -2353,8 +2352,10 @@ static int sm2_convert_dec_out(struct wcrypto_ecc_msg *src, + struct wcrypto_sm2_dec_out *dout = &out->param.dout; + struct wcrypto_ecc_in *in = (void *)src->in; + struct wcrypto_sm2_dec_in *din = &in->param.din; ++ char buff[MAX_HASH_LENS] = {0}; + struct wcrypto_ecc_point x2y2; + __u32 ksz = dst->key_bytes; ++ struct wd_dtb u = {0}; + int ret; + + /* dec origin out data fmt: +@@ -2362,8 +2363,6 @@ static int sm2_convert_dec_out(struct wcrypto_ecc_msg *src, + * final out data fmt: + * | plaintext | + */ +- +- /* x2y2 :copy x2y2 into din->c1 */ + x2y2.x.data = (void *)dst->out; + x2y2.y.data = (void *)(dst->out + ksz); + x2y2.x.dsize = ksz; +@@ -2376,20 +2375,21 @@ static int sm2_convert_dec_out(struct wcrypto_ecc_msg *src, + return ret; + } + +- /* M' = C2 XOR t */ ++ /* M' = c2 XOR t */ + sm2_xor(&dout->plaintext, &din->c2); + +- /* u = hash(x2 || M' || y2), save u to din->c2 */ +- ret = sm2_hash(&din->c1.x, &x2y2, &dout->plaintext, q_info); ++ /* u = hash(x2 || M' || y2) */ ++ u.data = buff; ++ ret = sm2_hash(&u, &x2y2, &dout->plaintext, q_info); + if (unlikely(ret)) { + WD_ERR("failed to compute c3, ret = %d!\n", ret); + return ret; + } + +- /* u == c3 */ +- ret = is_equal(&din->c1.x, &din->c3); ++ /* Judge whether u == c3 */ ++ ret = is_equal(&u, &din->c3); + if (ret) +- WD_ERR("failed to dec sm2, u != C3!\n"); ++ WD_ERR("failed to dec sm2, u != c3!\n"); + + return ret; + } +-- +2.25.1 + diff --git a/0002-uadk-ecc-cleanup-sm2-dec-out-code.patch b/0002-uadk-ecc-cleanup-sm2-dec-out-code.patch new file mode 100644 index 0000000..5d7f1b2 --- /dev/null +++ b/0002-uadk-ecc-cleanup-sm2-dec-out-code.patch @@ -0,0 +1,71 @@ +From 3cd236bcafd4b6ff20363892a70dc5769af7e51a Mon Sep 17 00:00:00 2001 +From: Zhiqi Song +Date: Thu, 15 Dec 2022 16:07:57 +0800 +Subject: [PATCH 2/4] uadk/ecc: cleanup sm2 dec out code + +Delete incorrect comments and use variable name +that meet the algorithm criteria. + +Signed-off-by: Zhiqi Song +--- + drv/hisi_hpre.c | 19 +++++++++---------- + 1 file changed, 9 insertions(+), 10 deletions(-) + +diff --git a/drv/hisi_hpre.c b/drv/hisi_hpre.c +index 552a565..a172df5 100644 +--- a/drv/hisi_hpre.c ++++ b/drv/hisi_hpre.c +@@ -2191,11 +2191,11 @@ static int sm2_convert_dec_out(struct wd_ecc_msg *src, + struct wd_sm2_dec_out *dout = &out->param.dout; + struct wd_ecc_in *in = (void *)src->req.src; + struct wd_sm2_dec_in *din = &in->param.din; ++ char buff[MAX_HASH_LENS] = {0}; + struct wd_ecc_dh_out *dh_out; + __u32 ksz = dst->key_bytes; + struct wd_ecc_point x2y2; +- struct wd_dtb tmp = {0}; +- char buff[64] = {0}; ++ struct wd_dtb u = {0}; + int ret; + + /* +@@ -2211,8 +2211,6 @@ static int sm2_convert_dec_out(struct wd_ecc_msg *src, + x2y2.x.dsize = ksz; + x2y2.y.dsize = ksz; + +- tmp.data = buff; +- + /* t = KDF(x2 || y2, klen) */ + ret = sm2_kdf(&dout->plaintext, &x2y2, din->c2.dsize, &src->hash); + if (unlikely(ret)) { +@@ -2220,20 +2218,21 @@ static int sm2_convert_dec_out(struct wd_ecc_msg *src, + return ret; + } + +- /* M' = C2 XOR t */ ++ /* M' = c2 XOR t */ + sm2_xor(&dout->plaintext, &din->c2); + +- /* u = hash(x2 || M' || y2), save u to din->c2 */ +- ret = sm2_hash(&tmp, &x2y2, &dout->plaintext, &src->hash); ++ /* u = hash(x2 || M' || y2) */ ++ u.data = buff; ++ ret = sm2_hash(&u, &x2y2, &dout->plaintext, &src->hash); + if (unlikely(ret)) { + WD_ERR("failed to compute c3, ret = %d!\n", ret); + return ret; + } + +- /* u == c3 */ +- ret = is_equal(&tmp, &din->c3); ++ /* Judge whether u == c3 */ ++ ret = is_equal(&u, &din->c3); + if (ret) +- WD_ERR("failed to decode sm2, u != C3!\n"); ++ WD_ERR("failed to decode sm2, u != c3!\n"); + + return ret; + } +-- +2.25.1 + diff --git a/0003-uadk-v1-ecc-bugfix-incorrect-parameter-length.patch b/0003-uadk-v1-ecc-bugfix-incorrect-parameter-length.patch new file mode 100644 index 0000000..440a0b9 --- /dev/null +++ b/0003-uadk-v1-ecc-bugfix-incorrect-parameter-length.patch @@ -0,0 +1,153 @@ +From 770967b993db3bb6dcdb9e23d4d9f6c14869cd4d Mon Sep 17 00:00:00 2001 +From: Zhiqi Song +Date: Thu, 15 Dec 2022 16:50:20 +0800 +Subject: [PATCH 3/4] uadk/v1/ecc: bugfix incorrect parameter length + +In 'struct wd_dtb', the member 'bsize' indicates the whole +buffer size of the data, the member 'dsize' indicates the +valid size of the data. The 'bsize' is generally greater +than or equal to 'dsize'. In some cases, the data will be +filled with zero, the value of 'dsize' will not be updated, +so directly using 'dsize' may truncate the data and cause +erroneous judgement. + +The solution is when check whether a 'struct wd_dtb' type +variable is all zero, if 'bsize' is larger than 'dsize', +the length of the data should be the value of 'bsize'. +And the key size value is redundant. + +Signed-off-by: Zhiqi Song +--- + v1/drv/hisi_hpre_udrv.c | 22 +++++++++++++--------- + v1/wd_ecc.c | 14 +++++++------- + 2 files changed, 20 insertions(+), 16 deletions(-) + +diff --git a/v1/drv/hisi_hpre_udrv.c b/v1/drv/hisi_hpre_udrv.c +index 191beab..9e3bd3b 100644 +--- a/v1/drv/hisi_hpre_udrv.c ++++ b/v1/drv/hisi_hpre_udrv.c +@@ -1111,17 +1111,21 @@ static void correct_random(struct wd_dtb *k) + k->data[lens] = 0; + } + +-static bool is_all_zero(struct wd_dtb *e, struct wcrypto_ecc_msg *msg, +- const char *p_name) ++static bool is_all_zero(struct wd_dtb *e, const char *p_name) + { + int i; + +- for (i = 0; i < e->dsize && i < msg->key_bytes; i++) { ++ if (!e || !e->data) { ++ WD_ERR("invalid: %s is NULL!\n", p_name); ++ return true; ++ } ++ ++ for (i = 0; i < e->bsize; i++) { + if (e->data[i]) + return false; + } + +- WD_ERR("error: %s all zero!\n", p_name); ++ WD_ERR("invalid: %s all zero!\n", p_name); + + return true; + } +@@ -1144,15 +1148,15 @@ static int ecc_prepare_sign_in(struct wcrypto_ecc_msg *msg, + e = &in->dgst; + if (!in->k_set) { + if (msg->op_type != WCRYPTO_SM2_SIGN) { +- WD_ERR("random k not set!\n"); ++ WD_ERR("invalid: random k not set!\n"); + return -WD_EINVAL; + } + hw_msg->sm2_ksel = 1; +- } else if (is_all_zero(k, msg, "ecc sgn k")) { ++ } else if (is_all_zero(k, "ecc sgn k")) { + return -WD_EINVAL; + } + +- if (is_all_zero(e, msg, "ecc sgn e")) ++ if (is_all_zero(e, "ecc sgn e")) + return -WD_EINVAL; + + ret = qm_crypto_bin_to_hpre_bin(e->data, (const char *)e->data, +@@ -1192,7 +1196,7 @@ static int ecc_prepare_verf_in(struct wcrypto_ecc_msg *msg, void **data) + s = &vin->s; + r = &vin->r; + +- if (is_all_zero(e, msg, "ecc vrf e")) ++ if (is_all_zero(e, "ecc vrf e")) + return -WD_EINVAL; + + ret = qm_crypto_bin_to_hpre_bin(e->data, (const char *)e->data, +@@ -1274,7 +1278,7 @@ static int ecc_prepare_sm2_enc_in(struct wcrypto_ecc_msg *msg, + int ret; + + if (ein->k_set) { +- if (is_all_zero(k, msg, "sm2 enc k")) ++ if (is_all_zero(k, "sm2 enc k")) + return -WD_EINVAL; + + ret = qm_crypto_bin_to_hpre_bin(k->data, (const char *)k->data, +diff --git a/v1/wd_ecc.c b/v1/wd_ecc.c +index e108051..e6b771a 100644 +--- a/v1/wd_ecc.c ++++ b/v1/wd_ecc.c +@@ -544,6 +544,7 @@ static struct wcrypto_ecc_in *create_ecc_sign_in(struct wcrypto_ecc_ctx *ctx, + { + if (is_dgst) + return create_ecc_in(ctx, ECC_SIGN_IN_PARAM_NUM); ++ + return create_sm2_sign_in(ctx, m_len); + } + +@@ -1489,7 +1490,6 @@ static int ecc_request_init(struct wcrypto_ecc_msg *req, + if (req->op_type == WCRYPTO_ECXDH_GEN_KEY || + req->op_type == WCRYPTO_SM2_KG) { + struct wcrypto_ecc_point *g = NULL; +- + wcrypto_get_ecc_prikey_params((void *)key, NULL, NULL, + NULL, NULL, &g, NULL); + req->in = (void *)g; +@@ -1715,11 +1715,11 @@ static bool less_than_latter(struct wd_dtb *d, struct wd_dtb *n) + return ret < 0; + } + +-static bool is_all_zero(struct wd_dtb *p, struct wcrypto_ecc_ctx *ctx) ++static bool is_all_zero(struct wd_dtb *p) + { + int i; + +- for (i = 0; i < p->dsize && i < ctx->key_size; i++) { ++ for (i = 0; i < p->bsize; i++) { + if (p->data[i]) + return false; + } +@@ -1733,7 +1733,7 @@ static bool check_k_param(struct wd_dtb *k, struct wcrypto_ecc_ctx *ctx) + int ret; + + if (unlikely(!k->data)) { +- WD_ERR("error: k->data NULL!\n"); ++ WD_ERR("invalid: k->data NULL!\n"); + return false; + } + +@@ -1744,12 +1744,12 @@ static bool check_k_param(struct wd_dtb *k, struct wcrypto_ecc_ctx *ctx) + } + + if (unlikely(!less_than_latter(k, &cv->n))) { +- WD_ERR("error: k >= n\n"); ++ WD_ERR("invalid: k >= n!\n"); + return false; + } + +- if (unlikely(is_all_zero(k, ctx))) { +- WD_ERR("error: k all zero\n"); ++ if (unlikely(is_all_zero(k))) { ++ WD_ERR("invalid: k all zero!\n"); + return false; + } + +-- +2.25.1 + diff --git a/0004-uadk-ecc-bugfix-incorrect-parameter-length.patch b/0004-uadk-ecc-bugfix-incorrect-parameter-length.patch new file mode 100644 index 0000000..8c03355 --- /dev/null +++ b/0004-uadk-ecc-bugfix-incorrect-parameter-length.patch @@ -0,0 +1,74 @@ +From 835ae83290197bd32b5c0e146221fc86f13d57da Mon Sep 17 00:00:00 2001 +From: Zhiqi Song +Date: Thu, 15 Dec 2022 16:04:33 +0800 +Subject: [PATCH 4/4] uadk/ecc: bugfix incorrect parameter length + +When check whether a 'struct wd_dtb' type variable is all +zero, the length of the data should be the value of 'bsize'. +Directly using 'dsize' may truncate the data in some cases +and cause erroneous judgement. + +Signed-off-by: Zhiqi Song +--- + drv/hisi_hpre.c | 17 +++++++++++------ + 1 file changed, 11 insertions(+), 6 deletions(-) + +diff --git a/drv/hisi_hpre.c b/drv/hisi_hpre.c +index a172df5..d8b212d 100644 +--- a/drv/hisi_hpre.c ++++ b/drv/hisi_hpre.c +@@ -1054,11 +1054,16 @@ static void ecc_get_io_len(__u32 atype, __u32 hsz, size_t *ilen, + } + } + +-static bool is_all_zero(struct wd_dtb *e, struct wd_ecc_msg *msg) ++static bool is_all_zero(struct wd_dtb *e) + { + int i; + +- for (i = 0; i < e->dsize && i < msg->key_bytes; i++) { ++ if (!e || !e->data) { ++ WD_ERR("invalid: e or e->data is NULL\n"); ++ return true; ++ } ++ ++ for (i = 0; i < e->bsize; i++) { + if (e->data[i]) + return false; + } +@@ -1094,12 +1099,12 @@ static int ecc_prepare_sign_in(struct wd_ecc_msg *msg, + return -WD_EINVAL; + } + hw_msg->sm2_ksel = 1; +- } else if (is_all_zero(k, msg)) { ++ } else if (is_all_zero(k)) { + WD_ERR("invalid: ecc sign in k all zero!\n"); + return -WD_EINVAL; + } + +- if (is_all_zero(e, msg)) { ++ if (is_all_zero(e)) { + WD_ERR("invalid: ecc sign in e all zero!\n"); + return -WD_EINVAL; + } +@@ -1141,7 +1146,7 @@ static int ecc_prepare_verf_in(struct wd_ecc_msg *msg, + s = &vin->s; + r = &vin->r; + +- if (is_all_zero(e, msg)) { ++ if (is_all_zero(e)) { + WD_ERR("invalid: ecc verf in e all zero!\n"); + return -WD_EINVAL; + } +@@ -1282,7 +1287,7 @@ static int u_is_in_p(struct wd_ecc_msg *msg) + return -WD_EINVAL; + } + +- if (is_all_zero(&pbk->x, msg)) { ++ if (is_all_zero(&pbk->x)) { + WD_ERR("invalid: ux is zero!\n"); + return -WD_EINVAL; + } +-- +2.25.1 + diff --git a/warpdrive.spec b/warpdrive.spec index a071e65..7e187ec 100644 --- a/warpdrive.spec +++ b/warpdrive.spec @@ -1,7 +1,7 @@ Name: libwd Summary: User Space Accelerator Development Kit Version: 2.3.37 -Release: 1 +Release: 2 License: Apache-2.0 Source: %{name}-%{version}.tar.gz Vendor: Huawei Corporation @@ -14,6 +14,10 @@ BuildRequires: numactl-devel, openssl-devel BuildRequires: automake, autoconf, libtool BuildRequires: gcc, make ExclusiveArch: aarch64 +Patch0001: 0001-uadk-v1-ecc-bugfix-buffer-overwritten-of-sm2-decrypt.patch +Patch0002: 0002-uadk-ecc-cleanup-sm2-dec-out-code.patch +Patch0003: 0003-uadk-v1-ecc-bugfix-incorrect-parameter-length.patch +Patch0004: 0004-uadk-ecc-bugfix-incorrect-parameter-length.patch %description This package contains the User Space Accelerator Library @@ -171,6 +175,9 @@ fi /sbin/ldconfig %changelog +* Tue Dec 15 2022 Yang Shen 2.3.37-2 +- libwd: fix a bug for ecc + * Mon Oct 10 2022 Yang Shen 2.3.37-1 - libwd: update the source code to 2.3.37