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

Reply via email to