* [PATCH] crypto/qat: fix to avoid buffer overwrite in OOP case
@ 2016-11-24 11:17 Fiona Trahe
2016-12-09 12:05 ` De Lara Guarch, Pablo
2016-12-09 15:39 ` [PATCH v2] " Fiona Trahe
0 siblings, 2 replies; 5+ messages in thread
From: Fiona Trahe @ 2016-11-24 11:17 UTC (permalink / raw)
To: dev; +Cc: pablo.de.lara.guarch, fiona.trahe, john.griffin, arkadiuszx.kusztal
In out-of-place operation, data is DMAed from source mbuf
to destination mbuf. To avoid header data in dest mbuf being
overwritten, the minimal data-set should be DMAed.
Fixes: 39e0bee48e81 ("crypto/qat: rework request builder for performance")
Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
---
This patch depends on following patch :
crypto: remove unused digest-appended feature
http://dpdk.org/dev/patchwork/patch/17079/
drivers/crypto/qat/qat_crypto.c | 66 ++++++++++++++++++++---------------------
drivers/crypto/qat/qat_crypto.h | 1 +
2 files changed, 34 insertions(+), 33 deletions(-)
diff --git a/drivers/crypto/qat/qat_crypto.c b/drivers/crypto/qat/qat_crypto.c
index 6a6bd2e..afce4ac 100644
--- a/drivers/crypto/qat/qat_crypto.c
+++ b/drivers/crypto/qat/qat_crypto.c
@@ -955,7 +955,7 @@ qat_write_hw_desc_entry(struct rte_crypto_op *op, uint8_t *out_msg)
uint32_t cipher_len = 0, cipher_ofs = 0;
uint32_t auth_len = 0, auth_ofs = 0;
uint32_t min_ofs = 0;
- uint64_t buf_start = 0;
+ uint64_t src_buf_start = 0, dst_buf_start = 0;
#ifdef RTE_LIBRTE_PMD_QAT_DEBUG_TX
@@ -1077,27 +1077,40 @@ qat_write_hw_desc_entry(struct rte_crypto_op *op, uint8_t *out_msg)
if (do_cipher && do_auth)
min_ofs = cipher_ofs < auth_ofs ? cipher_ofs : auth_ofs;
-
- /* Start DMA at nearest aligned address below min_ofs */
- #define QAT_64_BTYE_ALIGN_MASK (~0x3f)
- buf_start = rte_pktmbuf_mtophys_offset(op->sym->m_src, min_ofs) &
- QAT_64_BTYE_ALIGN_MASK;
-
- if (unlikely((rte_pktmbuf_mtophys(op->sym->m_src)
- - rte_pktmbuf_headroom(op->sym->m_src)) > buf_start)) {
- /* alignment has pushed addr ahead of start of mbuf
- * so revert and take the performance hit
+ if (unlikely(op->sym->m_dst != NULL)) {
+ /* Out-of-place operation (OOP)
+ * Don't align DMA start. DMA the minimum data-set
+ * so as not to overwrite data in dest buffer
*/
- buf_start = rte_pktmbuf_mtophys(op->sym->m_src);
+ src_buf_start =
+ rte_pktmbuf_mtophys_offset(op->sym->m_src, min_ofs);
+ dst_buf_start =
+ rte_pktmbuf_mtophys_offset(op->sym->m_dst, min_ofs);
+ } else {
+ /* In-place operation
+ * Start DMA at nearest aligned address below min_ofs
+ */
+ src_buf_start =
+ rte_pktmbuf_mtophys_offset(op->sym->m_src, min_ofs)
+ & QAT_64_BTYE_ALIGN_MASK;
+
+ if (unlikely((rte_pktmbuf_mtophys(op->sym->m_src) -
+ rte_pktmbuf_headroom(op->sym->m_src))
+ > src_buf_start)) {
+ /* alignment has pushed addr ahead of start of mbuf
+ * so revert and take the performance hit
+ */
+ src_buf_start =
+ rte_pktmbuf_mtophys_offset(op->sym->m_src,
+ min_ofs);
+ }
+ dst_buf_start = src_buf_start;
}
- qat_req->comn_mid.dest_data_addr =
- qat_req->comn_mid.src_data_addr = buf_start;
-
if (do_cipher) {
cipher_param->cipher_offset =
- (uint32_t)rte_pktmbuf_mtophys_offset(
- op->sym->m_src, cipher_ofs) - buf_start;
+ (uint32_t)rte_pktmbuf_mtophys_offset(
+ op->sym->m_src, cipher_ofs) - src_buf_start;
cipher_param->cipher_length = cipher_len;
} else {
cipher_param->cipher_offset = 0;
@@ -1105,7 +1118,7 @@ qat_write_hw_desc_entry(struct rte_crypto_op *op, uint8_t *out_msg)
}
if (do_auth) {
auth_param->auth_off = (uint32_t)rte_pktmbuf_mtophys_offset(
- op->sym->m_src, auth_ofs) - buf_start;
+ op->sym->m_src, auth_ofs) - src_buf_start;
auth_param->auth_len = auth_len;
} else {
auth_param->auth_off = 0;
@@ -1118,21 +1131,8 @@ qat_write_hw_desc_entry(struct rte_crypto_op *op, uint8_t *out_msg)
(cipher_param->cipher_offset + cipher_param->cipher_length)
: (auth_param->auth_off + auth_param->auth_len);
-
- /* out-of-place operation (OOP) */
- if (unlikely(op->sym->m_dst != NULL)) {
-
- if (do_auth)
- qat_req->comn_mid.dest_data_addr =
- rte_pktmbuf_mtophys_offset(op->sym->m_dst,
- auth_ofs)
- - auth_param->auth_off;
- else
- qat_req->comn_mid.dest_data_addr =
- rte_pktmbuf_mtophys_offset(op->sym->m_dst,
- cipher_ofs)
- - cipher_param->cipher_offset;
- }
+ qat_req->comn_mid.src_data_addr = src_buf_start;
+ qat_req->comn_mid.dest_data_addr = dst_buf_start;
if (ctx->qat_hash_alg == ICP_QAT_HW_AUTH_ALGO_GALOIS_128 ||
ctx->qat_hash_alg == ICP_QAT_HW_AUTH_ALGO_GALOIS_64) {
diff --git a/drivers/crypto/qat/qat_crypto.h b/drivers/crypto/qat/qat_crypto.h
index 0afe74e..6b84488 100644
--- a/drivers/crypto/qat/qat_crypto.h
+++ b/drivers/crypto/qat/qat_crypto.h
@@ -43,6 +43,7 @@
*/
#define ALIGN_POW2_ROUNDUP(num, align) \
(((num) + (align) - 1) & ~((align) - 1))
+#define QAT_64_BTYE_ALIGN_MASK (~0x3f)
/**
* Structure associated with each queue.
--
2.5.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] crypto/qat: fix to avoid buffer overwrite in OOP case
2016-11-24 11:17 [PATCH] crypto/qat: fix to avoid buffer overwrite in OOP case Fiona Trahe
@ 2016-12-09 12:05 ` De Lara Guarch, Pablo
2016-12-09 15:39 ` [PATCH v2] " Fiona Trahe
1 sibling, 0 replies; 5+ messages in thread
From: De Lara Guarch, Pablo @ 2016-12-09 12:05 UTC (permalink / raw)
To: Trahe, Fiona, dev; +Cc: Griffin, John, Kusztal, ArkadiuszX
Hi Fiona,
> -----Original Message-----
> From: Trahe, Fiona
> Sent: Thursday, November 24, 2016 11:18 AM
> To: dev@dpdk.org
> Cc: De Lara Guarch, Pablo; Trahe, Fiona; Griffin, John; Kusztal, ArkadiuszX
> Subject: [PATCH] crypto/qat: fix to avoid buffer overwrite in OOP case
>
> In out-of-place operation, data is DMAed from source mbuf
> to destination mbuf. To avoid header data in dest mbuf being
> overwritten, the minimal data-set should be DMAed.
>
> Fixes: 39e0bee48e81 ("crypto/qat: rework request builder for
> performance")
>
> Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
Should this be in the stable tree? If so, could you add: "CC: stable@dpdk.org" in the commit message?
Thanks,
Pablo
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] crypto/qat: fix to avoid buffer overwrite in OOP case
2016-11-24 11:17 [PATCH] crypto/qat: fix to avoid buffer overwrite in OOP case Fiona Trahe
2016-12-09 12:05 ` De Lara Guarch, Pablo
@ 2016-12-09 15:39 ` Fiona Trahe
2016-12-16 11:20 ` Griffin, John
1 sibling, 1 reply; 5+ messages in thread
From: Fiona Trahe @ 2016-12-09 15:39 UTC (permalink / raw)
To: dev; +Cc: pablo.de.lara.guarch, fiona.trahe, stable
In out-of-place operation, data is DMAed from source mbuf
to destination mbuf. To avoid header data in dest mbuf being
overwritten, the minimal data-set should be DMAed.
Fixes: 39e0bee48e81 ("crypto/qat: rework request builder for performance")
Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
CC: stable@dpdk.org
---
This patch depends on following patch :
crypto: remove unused digest-appended feature
http://dpdk.org/dev/patchwork/patch/17079/
v2:
- included ref for stable
drivers/crypto/qat/qat_crypto.c | 66 ++++++++++++++++++++---------------------
drivers/crypto/qat/qat_crypto.h | 1 +
2 files changed, 34 insertions(+), 33 deletions(-)
diff --git a/drivers/crypto/qat/qat_crypto.c b/drivers/crypto/qat/qat_crypto.c
index 6a6bd2e..afce4ac 100644
--- a/drivers/crypto/qat/qat_crypto.c
+++ b/drivers/crypto/qat/qat_crypto.c
@@ -955,7 +955,7 @@ qat_write_hw_desc_entry(struct rte_crypto_op *op, uint8_t *out_msg)
uint32_t cipher_len = 0, cipher_ofs = 0;
uint32_t auth_len = 0, auth_ofs = 0;
uint32_t min_ofs = 0;
- uint64_t buf_start = 0;
+ uint64_t src_buf_start = 0, dst_buf_start = 0;
#ifdef RTE_LIBRTE_PMD_QAT_DEBUG_TX
@@ -1077,27 +1077,40 @@ qat_write_hw_desc_entry(struct rte_crypto_op *op, uint8_t *out_msg)
if (do_cipher && do_auth)
min_ofs = cipher_ofs < auth_ofs ? cipher_ofs : auth_ofs;
-
- /* Start DMA at nearest aligned address below min_ofs */
- #define QAT_64_BTYE_ALIGN_MASK (~0x3f)
- buf_start = rte_pktmbuf_mtophys_offset(op->sym->m_src, min_ofs) &
- QAT_64_BTYE_ALIGN_MASK;
-
- if (unlikely((rte_pktmbuf_mtophys(op->sym->m_src)
- - rte_pktmbuf_headroom(op->sym->m_src)) > buf_start)) {
- /* alignment has pushed addr ahead of start of mbuf
- * so revert and take the performance hit
+ if (unlikely(op->sym->m_dst != NULL)) {
+ /* Out-of-place operation (OOP)
+ * Don't align DMA start. DMA the minimum data-set
+ * so as not to overwrite data in dest buffer
*/
- buf_start = rte_pktmbuf_mtophys(op->sym->m_src);
+ src_buf_start =
+ rte_pktmbuf_mtophys_offset(op->sym->m_src, min_ofs);
+ dst_buf_start =
+ rte_pktmbuf_mtophys_offset(op->sym->m_dst, min_ofs);
+ } else {
+ /* In-place operation
+ * Start DMA at nearest aligned address below min_ofs
+ */
+ src_buf_start =
+ rte_pktmbuf_mtophys_offset(op->sym->m_src, min_ofs)
+ & QAT_64_BTYE_ALIGN_MASK;
+
+ if (unlikely((rte_pktmbuf_mtophys(op->sym->m_src) -
+ rte_pktmbuf_headroom(op->sym->m_src))
+ > src_buf_start)) {
+ /* alignment has pushed addr ahead of start of mbuf
+ * so revert and take the performance hit
+ */
+ src_buf_start =
+ rte_pktmbuf_mtophys_offset(op->sym->m_src,
+ min_ofs);
+ }
+ dst_buf_start = src_buf_start;
}
- qat_req->comn_mid.dest_data_addr =
- qat_req->comn_mid.src_data_addr = buf_start;
-
if (do_cipher) {
cipher_param->cipher_offset =
- (uint32_t)rte_pktmbuf_mtophys_offset(
- op->sym->m_src, cipher_ofs) - buf_start;
+ (uint32_t)rte_pktmbuf_mtophys_offset(
+ op->sym->m_src, cipher_ofs) - src_buf_start;
cipher_param->cipher_length = cipher_len;
} else {
cipher_param->cipher_offset = 0;
@@ -1105,7 +1118,7 @@ qat_write_hw_desc_entry(struct rte_crypto_op *op, uint8_t *out_msg)
}
if (do_auth) {
auth_param->auth_off = (uint32_t)rte_pktmbuf_mtophys_offset(
- op->sym->m_src, auth_ofs) - buf_start;
+ op->sym->m_src, auth_ofs) - src_buf_start;
auth_param->auth_len = auth_len;
} else {
auth_param->auth_off = 0;
@@ -1118,21 +1131,8 @@ qat_write_hw_desc_entry(struct rte_crypto_op *op, uint8_t *out_msg)
(cipher_param->cipher_offset + cipher_param->cipher_length)
: (auth_param->auth_off + auth_param->auth_len);
-
- /* out-of-place operation (OOP) */
- if (unlikely(op->sym->m_dst != NULL)) {
-
- if (do_auth)
- qat_req->comn_mid.dest_data_addr =
- rte_pktmbuf_mtophys_offset(op->sym->m_dst,
- auth_ofs)
- - auth_param->auth_off;
- else
- qat_req->comn_mid.dest_data_addr =
- rte_pktmbuf_mtophys_offset(op->sym->m_dst,
- cipher_ofs)
- - cipher_param->cipher_offset;
- }
+ qat_req->comn_mid.src_data_addr = src_buf_start;
+ qat_req->comn_mid.dest_data_addr = dst_buf_start;
if (ctx->qat_hash_alg == ICP_QAT_HW_AUTH_ALGO_GALOIS_128 ||
ctx->qat_hash_alg == ICP_QAT_HW_AUTH_ALGO_GALOIS_64) {
diff --git a/drivers/crypto/qat/qat_crypto.h b/drivers/crypto/qat/qat_crypto.h
index 0afe74e..6b84488 100644
--- a/drivers/crypto/qat/qat_crypto.h
+++ b/drivers/crypto/qat/qat_crypto.h
@@ -43,6 +43,7 @@
*/
#define ALIGN_POW2_ROUNDUP(num, align) \
(((num) + (align) - 1) & ~((align) - 1))
+#define QAT_64_BTYE_ALIGN_MASK (~0x3f)
/**
* Structure associated with each queue.
--
2.5.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] crypto/qat: fix to avoid buffer overwrite in OOP case
2016-12-09 15:39 ` [PATCH v2] " Fiona Trahe
@ 2016-12-16 11:20 ` Griffin, John
2016-12-16 15:11 ` De Lara Guarch, Pablo
0 siblings, 1 reply; 5+ messages in thread
From: Griffin, John @ 2016-12-16 11:20 UTC (permalink / raw)
To: Trahe, Fiona, dev; +Cc: De Lara Guarch, Pablo, Trahe, Fiona, stable
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Fiona Trahe
> Sent: Friday, December 9, 2016 3:39 PM
> To: dev@dpdk.org
> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Trahe, Fiona
> <fiona.trahe@intel.com>; stable@dpdk.org
> Subject: [dpdk-dev] [PATCH v2] crypto/qat: fix to avoid buffer overwrite in
> OOP case
>
> In out-of-place operation, data is DMAed from source mbuf to destination
> mbuf. To avoid header data in dest mbuf being overwritten, the minimal data-
> set should be DMAed.
>
> Fixes: 39e0bee48e81 ("crypto/qat: rework request builder for
> performance")
>
> Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
Acked-by: John Griffin <john.griffin@intel.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] crypto/qat: fix to avoid buffer overwrite in OOP case
2016-12-16 11:20 ` Griffin, John
@ 2016-12-16 15:11 ` De Lara Guarch, Pablo
0 siblings, 0 replies; 5+ messages in thread
From: De Lara Guarch, Pablo @ 2016-12-16 15:11 UTC (permalink / raw)
To: Griffin, John, Trahe, Fiona, dev; +Cc: Trahe, Fiona, stable
> -----Original Message-----
> From: Griffin, John
> Sent: Friday, December 16, 2016 11:21 AM
> To: Trahe, Fiona; dev@dpdk.org
> Cc: De Lara Guarch, Pablo; Trahe, Fiona; stable@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2] crypto/qat: fix to avoid buffer overwrite
> in OOP case
>
>
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Fiona Trahe
> > Sent: Friday, December 9, 2016 3:39 PM
> > To: dev@dpdk.org
> > Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Trahe, Fiona
> > <fiona.trahe@intel.com>; stable@dpdk.org
> > Subject: [dpdk-dev] [PATCH v2] crypto/qat: fix to avoid buffer overwrite in
> > OOP case
> >
> > In out-of-place operation, data is DMAed from source mbuf to destination
> > mbuf. To avoid header data in dest mbuf being overwritten, the minimal
> data-
> > set should be DMAed.
> >
> > Fixes: 39e0bee48e81 ("crypto/qat: rework request builder for
> > performance")
> >
> > Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
> Acked-by: John Griffin <john.griffin@intel.com>
Applied to dpdk-next-crypto.
Thanks,
Pablo
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-12-16 15:11 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-24 11:17 [PATCH] crypto/qat: fix to avoid buffer overwrite in OOP case Fiona Trahe
2016-12-09 12:05 ` De Lara Guarch, Pablo
2016-12-09 15:39 ` [PATCH v2] " Fiona Trahe
2016-12-16 11:20 ` Griffin, John
2016-12-16 15:11 ` De Lara Guarch, Pablo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.