Hi Akhil,
> -----Original Message-----
> From: Akhil Goyal <gak...@marvell.com>
> Sent: Wednesday, July 7, 2021 7:38 PM
> To: Gujjar, Abhinandan S <abhinandan.guj...@intel.com>; dev@dpdk.org;
> Jerin Jacob Kollanukkaran <jer...@marvell.com>
> Cc: Power, Ciara <ciara.po...@intel.com>
> Subject: RE: [EXT] [PATCH] test: fix crypto_op length for sessionless case
>
> Hi Abhinandan,
>
> > Currently, private_data_offset for the sessionless is computed wrongly
> > which includes extra bytes added because of using sizeof(struct
> > rte_crypto_sym_xform) * 2) instead of (sizeof(union
> > rte_event_crypto_metadata)). Due to this buffer overflow, the
> > corruption was leading to test application crash while freeing the ops
> > mempool.
> >
> > Fixes: 3c2c535ecfc0 ("test: add event crypto adapter auto-test")
> > Reported-by: ciara.po...@intel.com
> >
> > Signed-off-by: Abhinandan Gujjar <abhinandan.guj...@intel.com>
> > ---
> > app/test/test_event_crypto_adapter.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/app/test/test_event_crypto_adapter.c
> > b/app/test/test_event_crypto_adapter.c
> > index f689bc1f2..688ac0b2f 100644
> > --- a/app/test/test_event_crypto_adapter.c
> > +++ b/app/test/test_event_crypto_adapter.c
> > @@ -229,7 +229,7 @@ test_op_forward_mode(uint8_t session_less)
> > first_xform = &cipher_xform;
> > sym_op->xform = first_xform;
> > uint32_t len = IV_OFFSET + MAXIMUM_IV_LENGTH +
> > - (sizeof(struct rte_crypto_sym_xform) * 2);
> > + (sizeof(union rte_event_crypto_metadata));
> > op->private_data_offset = len;
> I do not understand the need for this patch.
This is patch provide fix for segfault at the end of
event_crypto_adapter_autotest()
RTE>>event_crypto_adapter_autotest
+ ------------------------------------------------------- +
+ Test Suite : Event crypto adapter test suite
CRYPTODEV: Creating cryptodev crypto_nullCRYPTODEV: Initialisation parameters -
name: crypto_null,socket id: 0, max queue pairs: 8
CRYPTODEV: elt_size 0 is expanded to 336 +
------------------------------------------------------- +
+ TestCase [ 0] : test_crypto_adapter_create succeeded
+ TestCase [ 1] : test_crypto_adapter_qp_add_del succeeded
+------------------------------------------------------+
+ Crypto adapter stats for instance 0:
+ Event port poll count 0
+ Event dequeue count 0
+ Cryptodev enqueue count 0
+ Cryptodev enqueue failed count 0
+ Cryptodev dequeue count 0
+ Event enqueue count 0
+ Event enqueue retry count 0
+ Event enqueue fail count 0
+------------------------------------------------------+
+ TestCase [ 2] : test_crypto_adapter_stats succeeded
Segmentation fault (core dumped)
> Event metadata is copied after private data offset, and this patch is changing
> the offset value.
>
> You changed the value of len = iv_off + max_iv_len + metadata_size, but
> metadata is copied after this 'len'. See this rte_memcpy((uint8_t *)op + len,
> &m_data, sizeof(m_data));
Op_mpool is created with element of priv_size = DEFAULT_NUM_XFORMS *
sizeof(struct rte_crypto_sym_xform) + MAXIMUM_IV_LENGTH.
Whereas for the "sessionless" length is set to " uint32_t len = IV_OFFSET +
MAXIMUM_IV_LENGTH + (sizeof(struct rte_crypto_sym_xform) * 2)"
Whereas, IV_OFFSET = (sizeof(struct rte_crypto_op) + sizeof(struct
rte_crypto_sym_op) + DEFAULT_NUM_XFORMS * sizeof(struct rte_crypto_sym_xform)).
So substituting IV_OFFSET, len = (sizeof(struct rte_crypto_op) + sizeof(struct
rte_crypto_sym_op) + DEFAULT_NUM_XFORMS * sizeof(struct rte_crypto_sym_xform))
+ MAXIMUM_IV_LENGTH + (sizeof(struct rte_crypto_sym_xform) * 2).
Which is a way ahead of the boundary which causes buffer overflow.
When memcpy is executed -> rte_memcpy((uint8_t *)op + len, &m_data,
sizeof(m_data));
The m_data will overwrite the beyond the boundary. Hope this clarifies the need
for fix.
>
> I do not agree with this patch, am I missing something?
>
> > /* Fill in private data information */
> > rte_memcpy(&m_data.response_info, &response_info, @@ -
> 424,7 +424,7
> > @@ test_op_new_mode(uint8_t session_less)
> > first_xform = &cipher_xform;
> > sym_op->xform = first_xform;
> > uint32_t len = IV_OFFSET + MAXIMUM_IV_LENGTH +
> > - (sizeof(struct rte_crypto_sym_xform) * 2);
> > + (sizeof(union rte_event_crypto_metadata));
> > op->private_data_offset = len;
> > /* Fill in private data information */
> > rte_memcpy(&m_data.response_info, &response_info,
> > --
> > 2.25.1
