cryptodev: update symmetric session structure

This patch updates the rte_cryptodev_sym_session structure for
cryptodev library. The updates include a changed session private
data array and an added nb_drivers field. They are used to
calculate the correct session header size and ensure safe access
of the session private data.

Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
Acked-by: Fiona Trahe <fiona.trahe@intel.com>
Acked-by: Akhil Goyal <akhil.goyal@nxp.com>

3 files changed, 87 insertions, 34 deletions

diff --git a/lib/librte_cryptodev/rte_cryptodev.c b/lib/librte_cryptodev/rte_cryptodev.c
index 4a105b08dc..2a8e07d273 100644
--- a/lib/librte_cryptodev/rte_cryptodev.c
+++ b/lib/librte_cryptodev/rte_cryptodev.c
@@ -978,6 +978,30 @@ rte_cryptodev_queue_pair_setup(uint8_t dev_id, uint16_t queue_pair_id,
 		return -EINVAL;
 	}
 
+	if (qp_conf->mp_session) {
+		struct rte_cryptodev_sym_session_pool_private_data *pool_priv;
+		uint32_t obj_size = qp_conf->mp_session->elt_size;
+		uint32_t obj_priv_size = qp_conf->mp_session_private->elt_size;
+		struct rte_cryptodev_sym_session s = {0};
+
+		pool_priv = rte_mempool_get_priv(qp_conf->mp_session);
+		if (!pool_priv || qp_conf->mp_session->private_data_size <
+				sizeof(*pool_priv)) {
+			CDEV_LOG_ERR("Invalid mempool\n");
+			return -EINVAL;
+		}
+
+		s.nb_drivers = pool_priv->nb_drivers;
+
+		if ((rte_cryptodev_sym_get_existing_header_session_size(&s) >
+			obj_size) || (s.nb_drivers <= dev->driver_id) ||
+			rte_cryptodev_sym_get_private_session_size(dev_id) >
+				obj_priv_size) {
+			CDEV_LOG_ERR("Invalid mempool\n");
+			return -EINVAL;
+		}
+	}
+
 	if (dev->data->dev_started) {
 		CDEV_LOG_ERR(
 		    "device %d must be stopped to allow configuration", dev_id);
@@ -1172,6 +1196,8 @@ rte_cryptodev_sym_session_init(uint8_t dev_id,
 		struct rte_mempool *mp)
 {
 	struct rte_cryptodev *dev;
+	uint32_t sess_priv_sz = rte_cryptodev_sym_get_private_session_size(
+			dev_id);
 	uint8_t index;
 	int ret;
 
@@ -1180,11 +1206,16 @@ rte_cryptodev_sym_session_init(uint8_t dev_id,
 	if (sess == NULL || xforms == NULL || dev == NULL)
 		return -EINVAL;
 
+	if (mp->elt_size < sess_priv_sz)
+		return -EINVAL;
+
 	index = dev->driver_id;
+	if (index >= sess->nb_drivers)
+		return -EINVAL;
 
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->sym_session_configure, -ENOTSUP);
 
-	if (sess->sess_private_data[index] == NULL) {
+	if (sess->sess_data[index].data == NULL) {
 		ret = dev->dev_ops->sym_session_configure(dev, xforms,
 							sess, mp);
 		if (ret < 0) {
@@ -1273,10 +1304,29 @@ rte_cryptodev_sym_session_pool_create(const char *name, uint32_t nb_elts,
 	return mp;
 }
 
+static unsigned int
+rte_cryptodev_sym_session_data_size(struct rte_cryptodev_sym_session *sess)
+{
+	return (sizeof(sess->sess_data[0]) * sess->nb_drivers);
+}
+
 struct rte_cryptodev_sym_session *
 rte_cryptodev_sym_session_create(struct rte_mempool *mp)
 {
 	struct rte_cryptodev_sym_session *sess;
+	struct rte_cryptodev_sym_session_pool_private_data *pool_priv;
+
+	if (!mp) {
+		CDEV_LOG_ERR("Invalid mempool\n");
+		return NULL;
+	}
+
+	pool_priv = rte_mempool_get_priv(mp);
+
+	if (!pool_priv || mp->private_data_size < sizeof(*pool_priv)) {
+		CDEV_LOG_ERR("Invalid mempool\n");
+		return NULL;
+	}
 
 	/* Allocate a session structure from the session pool */
 	if (rte_mempool_get(mp, (void **)&sess)) {
@@ -1284,10 +1334,14 @@ rte_cryptodev_sym_session_create(struct rte_mempool *mp)
 		return NULL;
 	}
 
+	sess->nb_drivers = pool_priv->nb_drivers;
+
+
 	/* Clear device session pointer.
 	 * Include the flag indicating presence of user data
 	 */
-	memset(sess, 0, (sizeof(void *) * nb_drivers) + sizeof(uint8_t));
+	memset(sess->sess_data, 0,
+			rte_cryptodev_sym_session_data_size(sess));
 
 	return sess;
 }
@@ -1395,16 +1449,20 @@ rte_cryptodev_asym_session_free(struct rte_cryptodev_asym_session *sess)
 	return 0;
 }
 
-
 unsigned int
 rte_cryptodev_sym_get_header_session_size(void)
 {
 	/*
-	 * Header contains pointers to the private data
-	 * of all registered drivers, and a flag which
-	 * indicates presence of user data
+	 * Header contains pointers to the private data of all registered
+	 * drivers and all necessary information to ensure safely clear
+	 * or free al session.
 	 */
-	return ((sizeof(void *) * nb_drivers) + sizeof(uint8_t));
+	struct rte_cryptodev_sym_session s = {0};
+
+	s.nb_drivers = nb_drivers;
+
+	return (unsigned int)(sizeof(s) +
+			rte_cryptodev_sym_session_data_size(&s));
 }
 
 unsigned int __rte_experimental
@@ -1414,7 +1472,8 @@ rte_cryptodev_sym_get_existing_header_session_size(
 	if (!sess)
 		return 0;
 	else
-		return rte_cryptodev_sym_get_header_session_size();
+		return (unsigned int)(sizeof(*sess) +
+				rte_cryptodev_sym_session_data_size(sess));
 }
 
 unsigned int __rte_experimental
@@ -1432,7 +1491,6 @@ unsigned int
 rte_cryptodev_sym_get_private_session_size(uint8_t dev_id)
 {
 	struct rte_cryptodev *dev;
-	unsigned int header_size = sizeof(void *) * nb_drivers;
 	unsigned int priv_sess_size;
 
 	if (!rte_cryptodev_pmd_is_valid_dev(dev_id))
@@ -1445,16 +1503,7 @@ rte_cryptodev_sym_get_private_session_size(uint8_t dev_id)
 
 	priv_sess_size = (*dev->dev_ops->sym_session_get_size)(dev);
 
-	/*
-	 * If size is less than session header size,
-	 * return the latter, as this guarantees that
-	 * sessionless operations will work
-	 */
-	if (priv_sess_size < header_size)
-		return header_size;
-
 	return priv_sess_size;
-
 }
 
 unsigned int __rte_experimental
@@ -1486,15 +1535,10 @@ rte_cryptodev_sym_session_set_user_data(
 					void *data,
 					uint16_t size)
 {
-	uint16_t off_set = sizeof(void *) * nb_drivers;
-	uint8_t *user_data_present = (uint8_t *)sess + off_set;
-
 	if (sess == NULL)
 		return -EINVAL;
 
-	*user_data_present = 1;
-	off_set += sizeof(uint8_t);
-	rte_memcpy((uint8_t *)sess + off_set, data, size);
+	rte_memcpy(sess->sess_data + sess->nb_drivers, data, size);
 	return 0;
 }
 
@@ -1502,14 +1546,10 @@ void * __rte_experimental
 rte_cryptodev_sym_session_get_user_data(
 					struct rte_cryptodev_sym_session *sess)
 {
-	uint16_t off_set = sizeof(void *) * nb_drivers;
-	uint8_t *user_data_present = (uint8_t *)sess + off_set;
-
-	if (sess == NULL || !*user_data_present)
+	if (sess == NULL)
 		return NULL;
 
-	off_set += sizeof(uint8_t);
-	return (uint8_t *)sess + off_set;
+	return (void *)(sess->sess_data + sess->nb_drivers);
 }
 
 /** Initialise rte_crypto_op mempool element */
diff --git a/lib/librte_cryptodev/rte_cryptodev.h b/lib/librte_cryptodev/rte_cryptodev.h
index d0eaf00260..1bc4a375b4 100644
--- a/lib/librte_cryptodev/rte_cryptodev.h
+++ b/lib/librte_cryptodev/rte_cryptodev.h
@@ -953,8 +953,12 @@ rte_cryptodev_enqueue_burst(uint8_t dev_id, uint16_t qp_id,
  * has a fixed algo, key, op-type, digest_len etc.
  */
 struct rte_cryptodev_sym_session {
-	__extension__ void *sess_private_data[0];
-	/**< Private symmetric session material */
+	uint16_t nb_drivers;
+	/**< number of elements in sess_data array */
+	__extension__ struct {
+		void *data;
+	} sess_data[0];
+	/**< Driver specific session material, variable size */
 };
 
 /** Cryptodev asymmetric crypto session */
diff --git a/lib/librte_cryptodev/rte_cryptodev_pmd.h b/lib/librte_cryptodev/rte_cryptodev_pmd.h
index f15c9af306..defe05ea05 100644
--- a/lib/librte_cryptodev/rte_cryptodev_pmd.h
+++ b/lib/librte_cryptodev/rte_cryptodev_pmd.h
@@ -475,14 +475,23 @@ RTE_INIT(init_ ##driver_id)\
 static inline void *
 get_sym_session_private_data(const struct rte_cryptodev_sym_session *sess,
 		uint8_t driver_id) {
-	return sess->sess_private_data[driver_id];
+	if (unlikely(sess->nb_drivers <= driver_id))
+		return NULL;
+
+	return sess->sess_data[driver_id].data;
 }
 
 static inline void
 set_sym_session_private_data(struct rte_cryptodev_sym_session *sess,
 		uint8_t driver_id, void *private_data)
 {
-	sess->sess_private_data[driver_id] = private_data;
+	if (unlikely(sess->nb_drivers <= driver_id)) {
+		CDEV_LOG_ERR("Set private data for driver %u not allowed\n",
+				driver_id);
+		return;
+	}
+
+	sess->sess_data[driver_id].data = private_data;
 }
 
 static inline void *