crypto/qat: fix seg-fault

Message ID 1562752404-14017-1-git-send-email-fiona.trahe@intel.com (mailing list archive)
State Accepted, archived
Delegated to: akhil goyal
Headers
Series crypto/qat: fix seg-fault |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/intel-Performance-Testing success Performance Testing PASS
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/Intel-compilation success Compilation OK

Commit Message

Fiona Trahe July 10, 2019, 9:53 a.m. UTC
  Fix for seg-faults occurring:
1) in buffer re-alignment in-place sgl case
2) case where data end is exactly at end of an sgl segment.
Also renamed variable and increased comments for clearer code.

Fixes: 40002f6c2a24 ("crypto/qat: extend support for digest-encrypted auth-cipher")

Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
---
 drivers/crypto/qat/qat_sym.c |   35 ++++++++++++++++++++++-------------
 1 files changed, 22 insertions(+), 13 deletions(-)
  

Comments

Ananyev, Konstantin July 10, 2019, 10:10 a.m. UTC | #1
> 
> Fix for seg-faults occurring:
> 1) in buffer re-alignment in-place sgl case
> 2) case where data end is exactly at end of an sgl segment.
> Also renamed variable and increased comments for clearer code.
> 
> Fixes: 40002f6c2a24 ("crypto/qat: extend support for digest-encrypted auth-cipher")
> 
> Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
> ---

Confirm that the patch fixes crash of ipsec-secgw for multi-seg packets.
Tested-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

> --
> 1.7.0.7
  
Damian Nowak July 10, 2019, 3:03 p.m. UTC | #2
> -----Original Message-----
> From: Trahe, Fiona
> Sent: Wednesday, July 10, 2019 11:53
> To: dev@dpdk.org
> Cc: akhil.goyal@nxp.com; Trahe, Fiona <fiona.trahe@intel.com>; Nowak,
> DamianX <damianx.nowak@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>
> Subject: [PATCH] crypto/qat: fix seg-fault
> 
> Fix for seg-faults occurring:
> 1) in buffer re-alignment in-place sgl case
> 2) case where data end is exactly at end of an sgl segment.
> Also renamed variable and increased comments for clearer code.
> 
> Fixes: 40002f6c2a24 ("crypto/qat: extend support for digest-encrypted auth-
> cipher")
> 
> Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>

Acked-by: Damian Nowak <damianx.nowak@intel.com>
  
Akhil Goyal July 18, 2019, 3:23 p.m. UTC | #3
> 
> >
> > Fix for seg-faults occurring:
> > 1) in buffer re-alignment in-place sgl case
> > 2) case where data end is exactly at end of an sgl segment.
> > Also renamed variable and increased comments for clearer code.
> >
> > Fixes: 40002f6c2a24 ("crypto/qat: extend support for digest-encrypted auth-
> > cipher")
> >
> > Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
> 
> Acked-by: Damian Nowak <damianx.nowak@intel.com>

Applied to dpdk-next-crypto

Thanks
  

Patch

diff --git a/drivers/crypto/qat/qat_sym.c b/drivers/crypto/qat/qat_sym.c
index 2dc0614..46ef27a 100644
--- a/drivers/crypto/qat/qat_sym.c
+++ b/drivers/crypto/qat/qat_sym.c
@@ -156,9 +156,10 @@ 
 	uint32_t auth_len = 0, auth_ofs = 0;
 	uint32_t min_ofs = 0;
 	uint64_t src_buf_start = 0, dst_buf_start = 0;
-	uint64_t digest_start = 0;
+	uint64_t auth_data_end = 0;
 	uint8_t do_sgl = 0;
 	uint8_t in_place = 1;
+	int alignment_adjustment = 0;
 	struct rte_crypto_op *op = (struct rte_crypto_op *)in_op;
 	struct qat_sym_op_cookie *cookie =
 				(struct qat_sym_op_cookie *)op_cookie;
@@ -465,6 +466,10 @@ 
 								min_ofs);
 		}
 		dst_buf_start = src_buf_start;
+
+		/* remember any adjustment for later, note, can be +/- */
+		alignment_adjustment = src_buf_start -
+			rte_pktmbuf_iova_offset(op->sym->m_src, min_ofs);
 	}
 
 	if (do_cipher || do_aead) {
@@ -494,33 +499,37 @@ 
 		: (auth_param->auth_off + auth_param->auth_len);
 
 	if (do_auth && do_cipher) {
+		/* Handle digest-encrypted cases, i.e.
+		 * auth-gen-then-cipher-encrypt and
+		 * cipher-decrypt-then-auth-verify
+		 */
+		 /* First find the end of the data */
 		if (do_sgl) {
 			uint32_t remaining_off = auth_param->auth_off +
-				auth_param->auth_len;
+				auth_param->auth_len + alignment_adjustment;
 			struct rte_mbuf *sgl_buf =
 				(in_place ?
-				op->sym->m_src : op->sym->m_dst);
-			while (remaining_off >= rte_pktmbuf_data_len(
-					sgl_buf)) {
-				remaining_off -= rte_pktmbuf_data_len(
-						sgl_buf);
+					op->sym->m_src : op->sym->m_dst);
+
+			while (remaining_off >= rte_pktmbuf_data_len(sgl_buf)
+					&& sgl_buf->next != NULL) {
+				remaining_off -= rte_pktmbuf_data_len(sgl_buf);
 				sgl_buf = sgl_buf->next;
 			}
-			digest_start = (uint64_t)rte_pktmbuf_iova_offset(
+
+			auth_data_end = (uint64_t)rte_pktmbuf_iova_offset(
 				sgl_buf, remaining_off);
 		} else {
-			digest_start = (in_place ?
+			auth_data_end = (in_place ?
 				src_buf_start : dst_buf_start) +
 				auth_param->auth_off + auth_param->auth_len;
 		}
-		/* Handle cases of auth-gen-then-cipher and
-		 * cipher-decrypt-then-auth-verify with digest encrypted
-		 */
+		/* Then check if digest-encrypted conditions are met */
 		if ((auth_param->auth_off + auth_param->auth_len <
 					cipher_param->cipher_offset +
 					cipher_param->cipher_length) &&
 				(op->sym->auth.digest.phys_addr ==
-					digest_start)) {
+					auth_data_end)) {
 			/* Handle partial digest encryption */
 			if (cipher_param->cipher_offset +
 					cipher_param->cipher_length <