session_set_ec() checked qat_session->xform.ec.pkey.length for the NULL-allocation guards, but that field is zero (from the preceding memset) at that point, so the guards never fired. When test_ecpm() created a session the xform had no pkey or public-key fields set (ECPM only needs curve_id), so the unchecked memcpy calls received a NULL destination and a garbage-length source, causing a segfault.
Fix session_set_ec() to check xform->ec.*.length, the caller- supplied value, before allocating and copying each optional field (pkey, q.x, q.y), skipping the allocation entirely when the length is zero. Fix test_ecpm() to zero-initialise the xform with memset so that uninitialised pkey/q fields are never passed to the driver. Also add a QAT_LOG(ERR) in pick_curve() default case so that unsupported curve IDs are surfaced immediately rather than producing the misleading 'Unsupported xform type' message from qat_asym_session_configure(). Fixes: 064ef1b098d1 (\"test/crypto: remove PMD-specific asym test suites\") Cc: [email protected] Signed-off-by: Kai Ji <[email protected]> --- app/test/test_cryptodev_asym.c | 2 +- drivers/crypto/qat/qat_asym.c | 58 ++++++++++++++++++---------------- drivers/crypto/qat/qat_ec.h | 1 + 3 files changed, 32 insertions(+), 29 deletions(-) diff --git a/app/test/test_cryptodev_asym.c b/app/test/test_cryptodev_asym.c index 07e5eb5842..bf1a1fc417 100644 --- a/app/test/test_cryptodev_asym.c +++ b/app/test/test_cryptodev_asym.c @@ -1816,7 +1816,7 @@ test_ecpm(enum curve curve_id) asym_op = op->asym; /* Setup asym xform */ - xform.next = NULL; + memset(&xform, 0, sizeof(xform)); xform.xform_type = RTE_CRYPTO_ASYM_XFORM_ECPM; xform.ec.curve_id = input_params.curve; diff --git a/drivers/crypto/qat/qat_asym.c b/drivers/crypto/qat/qat_asym.c index 7a296cad6c..9276b86c0e 100644 --- a/drivers/crypto/qat/qat_asym.c +++ b/drivers/crypto/qat/qat_asym.c @@ -1492,36 +1492,38 @@ session_set_ec(struct qat_asym_session *qat_session, uint8_t *q_x = xform->ec.q.x.data; uint8_t *q_y = xform->ec.q.y.data; - qat_session->xform.ec.pkey.data = - rte_malloc(NULL, xform->ec.pkey.length, 0); - if (qat_session->xform.ec.pkey.length && - qat_session->xform.ec.pkey.data == NULL) - return -ENOMEM; - qat_session->xform.ec.q.x.data = rte_malloc(NULL, - xform->ec.q.x.length, 0); - if (qat_session->xform.ec.q.x.length && - qat_session->xform.ec.q.x.data == NULL) { - rte_free(qat_session->xform.ec.pkey.data); - return -ENOMEM; + if (xform->ec.pkey.length) { + qat_session->xform.ec.pkey.data = + rte_malloc(NULL, xform->ec.pkey.length, 0); + if (qat_session->xform.ec.pkey.data == NULL) + return -ENOMEM; + memcpy(qat_session->xform.ec.pkey.data, pkey, + xform->ec.pkey.length); + qat_session->xform.ec.pkey.length = xform->ec.pkey.length; } - qat_session->xform.ec.q.y.data = rte_malloc(NULL, - xform->ec.q.y.length, 0); - if (qat_session->xform.ec.q.y.length && - qat_session->xform.ec.q.y.data == NULL) { - rte_free(qat_session->xform.ec.pkey.data); - rte_free(qat_session->xform.ec.q.x.data); - return -ENOMEM; + if (xform->ec.q.x.length) { + qat_session->xform.ec.q.x.data = rte_malloc(NULL, + xform->ec.q.x.length, 0); + if (qat_session->xform.ec.q.x.data == NULL) { + rte_free(qat_session->xform.ec.pkey.data); + return -ENOMEM; + } + memcpy(qat_session->xform.ec.q.x.data, q_x, + xform->ec.q.x.length); + qat_session->xform.ec.q.x.length = xform->ec.q.x.length; + } + if (xform->ec.q.y.length) { + qat_session->xform.ec.q.y.data = rte_malloc(NULL, + xform->ec.q.y.length, 0); + if (qat_session->xform.ec.q.y.data == NULL) { + rte_free(qat_session->xform.ec.pkey.data); + rte_free(qat_session->xform.ec.q.x.data); + return -ENOMEM; + } + memcpy(qat_session->xform.ec.q.y.data, q_y, + xform->ec.q.y.length); + qat_session->xform.ec.q.y.length = xform->ec.q.y.length; } - - memcpy(qat_session->xform.ec.pkey.data, pkey, - xform->ec.pkey.length); - qat_session->xform.ec.pkey.length = xform->ec.pkey.length; - memcpy(qat_session->xform.ec.q.x.data, q_x, - xform->ec.q.x.length); - qat_session->xform.ec.q.x.length = xform->ec.q.x.length; - memcpy(qat_session->xform.ec.q.y.data, q_y, - xform->ec.q.y.length); - qat_session->xform.ec.q.y.length = xform->ec.q.y.length; qat_session->xform.ec.curve_id = xform->ec.curve_id; return 0; diff --git a/drivers/crypto/qat/qat_ec.h b/drivers/crypto/qat/qat_ec.h index 0e02722c18..653854abc4 100644 --- a/drivers/crypto/qat/qat_ec.h +++ b/drivers/crypto/qat/qat_ec.h @@ -274,6 +274,7 @@ pick_curve(const struct rte_crypto_asym_xform *xform) case RTE_CRYPTO_EC_GROUP_SECP521R1: return SECP521R1; default: + QAT_LOG(ERR, "Unsupported curve id %d", xform->ec.curve_id); return -1; } } -- 2.43.0

