diff mbox series

[v4,11/12] cryptodev: add reference count to session private data

Message ID 20190109225609.20590-12-roy.fan.zhang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: akhil goyal
Headers
Series cryptodev: change qp conf and sym session |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Fan Zhang Jan. 9, 2019, 10:56 p.m. UTC 
  This patch adds a refcnt field to every session private data in the
cryptodev symmetric session. The counter is used to prevent freeing
symmetric session blindly before it is not cleared by every type of
crypto device in use.

Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
Acked-by: Fiona Trahe <fiona.trahe@intel.com>
---
 doc/guides/prog_guide/img/cryptodev_sym_sess.svg |  7 +++++++
 doc/guides/rel_notes/release_19_02.rst           |  6 ++++++
 lib/librte_cryptodev/rte_cryptodev.c             | 14 +++++++++-----
 lib/librte_cryptodev/rte_cryptodev.h             |  1 +
 4 files changed, 23 insertions(+), 5 deletions(-)

Comments

De Lara Guarch, Pablo Jan. 10, 2019, 12:35 p.m. UTC | #1 
> -----Original Message-----
> From: Zhang, Roy Fan
> Sent: Wednesday, January 9, 2019 10:56 PM
> To: dev@dpdk.org
> Cc: akhil.goyal@nxp.com; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; Trahe, Fiona <fiona.trahe@intel.com>
> Subject: [PATCH v4 11/12] cryptodev: add reference count to session private
> data
> 
> This patch adds a refcnt field to every session private data in the cryptodev
> symmetric session. The counter is used to prevent freeing symmetric session
> blindly before it is not cleared by every type of crypto device in use.
> 
> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> Acked-by: Fiona Trahe <fiona.trahe@intel.com>
> ---
>  doc/guides/prog_guide/img/cryptodev_sym_sess.svg |  7 +++++++
>  doc/guides/rel_notes/release_19_02.rst           |  6 ++++++
>  lib/librte_cryptodev/rte_cryptodev.c             | 14 +++++++++-----
>  lib/librte_cryptodev/rte_cryptodev.h             |  1 +
>  4 files changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/doc/guides/prog_guide/img/cryptodev_sym_sess.svg
> b/doc/guides/prog_guide/img/cryptodev_sym_sess.svg
> index 20059cc0f..7d7052c38 100644
> --- a/doc/guides/prog_guide/img/cryptodev_sym_sess.svg

I think it is worth mentioning in the programmer's guide that it is expected for all devices to call sym_session_init, even if they are the same device type (for safety reasons).
diff mbox series

Patch

diff --git a/doc/guides/prog_guide/img/cryptodev_sym_sess.svg b/doc/guides/prog_guide/img/cryptodev_sym_sess.svg
index 20059cc0f..7d7052c38 100644
--- a/doc/guides/prog_guide/img/cryptodev_sym_sess.svg
+++ b/doc/guides/prog_guide/img/cryptodev_sym_sess.svg
@@ -308,6 +308,13 @@ 
        class="st2"
        y="189.4823"
        x="-185.78569">user_data</text>
+<text
+       transform="scale(0.71276665,1.4029837)"
+       style="font-size:14.02988338px;font-family:Calibri;overflow:visible;color-interpolation-filters:sRGB;fill:#386288;fill-rule:evenodd;stroke-width:1.40298378;stroke-linecap:square;stroke-miterlimit:3"
+       id="text24-5-5-1-4"
+       class="st2"
+       y="129.23468"
+       x="-204.95244">uint16_t refcnt;</text>
 </g><g
      transform="matrix(1.022976,0,0,0.71529071,199.82034,-39.936699)"
      id="shape19-6-5"><title
diff --git a/doc/guides/rel_notes/release_19_02.rst b/doc/guides/rel_notes/release_19_02.rst
index 740b24dd7..e64d52866 100644
--- a/doc/guides/rel_notes/release_19_02.rst
+++ b/doc/guides/rel_notes/release_19_02.rst
@@ -110,6 +110,12 @@  New Features
   Added a new performance test tool to test the compressdev PMD. The tool tests
   compression ratio and compression throughput.
 
+* **Added security checks to cryptodev symmetric session operations.**
+
+  Added a set of security checks to the access cryptodev symmetric session.
+  The checks include the session's user data read/write check and the
+  session private data referencing status check while freeing a session.
+
 
 Removed Items
 -------------
diff --git a/lib/librte_cryptodev/rte_cryptodev.c b/lib/librte_cryptodev/rte_cryptodev.c
index 82ebf58f0..3835272fe 100644
--- a/lib/librte_cryptodev/rte_cryptodev.c
+++ b/lib/librte_cryptodev/rte_cryptodev.c
@@ -1216,7 +1216,7 @@  rte_cryptodev_sym_session_init(uint8_t dev_id,
 
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->sym_session_configure, -ENOTSUP);
 
-	if (sess->sess_data[index].data == NULL) {
+	if (sess->sess_data[index].refcnt == 0) {
 		ret = dev->dev_ops->sym_session_configure(dev, xforms,
 							sess, mp);
 		if (ret < 0) {
@@ -1227,6 +1227,7 @@  rte_cryptodev_sym_session_init(uint8_t dev_id,
 		}
 	}
 
+	sess->sess_data[index].refcnt++;
 	return 0;
 }
 
@@ -1372,12 +1373,17 @@  rte_cryptodev_sym_session_clear(uint8_t dev_id,
 		struct rte_cryptodev_sym_session *sess)
 {
 	struct rte_cryptodev *dev;
+	uint8_t driver_id;
 
 	dev = rte_cryptodev_pmd_get_dev(dev_id);
 
 	if (dev == NULL || sess == NULL)
 		return -EINVAL;
 
+	driver_id = dev->driver_id;
+	if (--sess->sess_data[driver_id].refcnt != 0)
+		return -EBUSY;
+
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->sym_session_clear, -ENOTSUP);
 
 	dev->dev_ops->sym_session_clear(dev, sess);
@@ -1407,16 +1413,14 @@  int
 rte_cryptodev_sym_session_free(struct rte_cryptodev_sym_session *sess)
 {
 	uint8_t i;
-	void *sess_priv;
 	struct rte_mempool *sess_mp;
 
 	if (sess == NULL)
 		return -EINVAL;
 
 	/* Check that all device private data has been freed */
-	for (i = 0; i < nb_drivers; i++) {
-		sess_priv = get_sym_session_private_data(sess, i);
-		if (sess_priv != NULL)
+	for (i = 0; i < sess->nb_drivers; i++) {
+		if (sess->sess_data[i].refcnt != 0)
 			return -EBUSY;
 	}
 
diff --git a/lib/librte_cryptodev/rte_cryptodev.h b/lib/librte_cryptodev/rte_cryptodev.h
index 4ca7425c6..ec0d1c567 100644
--- a/lib/librte_cryptodev/rte_cryptodev.h
+++ b/lib/librte_cryptodev/rte_cryptodev.h
@@ -963,6 +963,7 @@  struct rte_cryptodev_sym_session {
 	/**< session user data will be placed after sess_data */
 	__extension__ struct {
 		void *data;
+		uint16_t refcnt;
 	} sess_data[0];
 	/**< Driver specific session material, variable size */
 };