>
> In order to process crypto operations in the AESNI MB PMD,
> they need to be sent to the buffer manager of the Multi-buffer library,
> through the "job" structure.
>
> Currently, it is checked if there are outstanding operations to process
> in the ring, before getting a new job. However, if there are no available
> jobs in the manager, a flush operation needs to take place, freeing some of
> the jobs,
> so it can be used for the outstanding operation.
>
> In order to avoid leaving the dequeued operation without being processed,
> the maximum number of operations that can be flushed is the remaining
> operations
> to return, which is the maximum number of operations that can be return minus
> the number of operations ready to be returned (nb_ops - processed_jobs),
> minus 1 (for the new operation).
>
> The problem comes when (nb_ops - processed_jobs) is 1 (last operation to
> dequeue).
> In that case, flush_mb_mgr is called with maximum number of operations equal
> to 0,
> which is wrong, causing a potential overrun in the "ops" array.
> Besides, the operation dequeued from the ring will be leaked, as no more
> operations can
> be returned.
>
> The solution is to first check if there are jobs available in the manager.
> If there are not, flush operation gets called, and if enough operations are
> returned
> from the manager, then no more outstanding operations get dequeued from the
> ring,
> avoiding both the memory leak and the array overrun.
> If there are enough jobs, the PMD tries to dequeue an operation from the ring.
> If there are no operations in the ring, the new job pointer is not used,
> and it will be used in the next get_next_job call, so no memory leak happens.
>
> Fixes: 0f548b50a160 ("crypto/aesni_mb: process crypto op on dequeue")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Pablo de Lara <pablo.de.lara.gua...@intel.com>
> ---
> drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
> b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
> index 93dc7a443..e2dd834f0 100644
> --- a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
> +++ b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
> @@ -833,22 +833,30 @@ aesni_mb_pmd_dequeue_burst(void *queue_pair, struct
> rte_crypto_op **ops,
>
> uint8_t digest_idx = qp->digest_idx;
> do {
> - /* Get next operation to process from ingress queue */
> - retval = rte_ring_dequeue(qp->ingress_queue, (void **)&op);
> - if (retval < 0)
> - break;
> -
> /* Get next free mb job struct from mb manager */
> job = (*qp->op_fns->job.get_next)(qp->mb_mgr);
> if (unlikely(job == NULL)) {
> /* if no free mb job structs we need to flush mb_mgr */
> processed_jobs += flush_mb_mgr(qp,
> &ops[processed_jobs],
> - (nb_ops - processed_jobs) - 1);
> + nb_ops - processed_jobs);
> +
> + if (nb_ops == processed_jobs)
> + break;
>
> job = (*qp->op_fns->job.get_next)(qp->mb_mgr);
> }
>
> + /*
> + * Get next operation to process from ingress queue.
> + * There is no need to return the job to the MB_MGR
> + * if there are no more operations to process, since the MB_MGR
> + * can use that pointer again in next get_next calls.
> + */
> + retval = rte_ring_dequeue(qp->ingress_queue, (void **)&op);
> + if (retval < 0)
> + break;
> +
> retval = set_mb_job_params(job, qp, op, &digest_idx);
> if (unlikely(retval != 0)) {
> qp->stats.dequeue_err_count++;
> --
Acked-by: Konstantin Ananyev <konstantin.anan...@intel.com>
> 2.17.1
