[v2] crypto/ipsec_mb: do not dequeue ops after flush
Checks
Commit Message
Previously it was possible to increment `processed_jobs` to a value
greater than requested `nb_ops`, because after flushing at most
`nb_ops` jobs the while loop continued, so `processed_jobs` could
still be incremented and it was possible for this variable to be
greater than `nb_ops`. If `ops` provided to the function were
only `nb_ops` long, then the `aesni_mb_dequeue_burst()` would
write to the memory outside of `ops` array.
Fixes: b50b8b5b38f8 ("crypto/ipsec_mb: use burst API in AESNI")
Cc: stable@dpdk.org
Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
---
v2:
* Shortened title to fit in the limit.
* Changed nb_ops decrementing from 'nb_jobs' to 'i'.
drivers/crypto/ipsec_mb/pmd_aesni_mb.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
Comments
> -----Original Message-----
> From: Karas, Krzysztof <krzysztof.karas@intel.com>
> Sent: Monday, November 13, 2023 8:02 AM
> To: Ji, Kai <kai.ji@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; Cornu, Marcel D
> <marcel.d.cornu@intel.com>; Power, Ciara <ciara.power@intel.com>
> Cc: dev@dpdk.org; Karas, Krzysztof <krzysztof.karas@intel.com>;
> stable@dpdk.org
> Subject: [PATCH v2] crypto/ipsec_mb: do not dequeue ops after flush
>
> Previously it was possible to increment `processed_jobs` to a value greater
> than requested `nb_ops`, because after flushing at most `nb_ops` jobs the
> while loop continued, so `processed_jobs` could still be incremented and it
> was possible for this variable to be greater than `nb_ops`. If `ops` provided to
> the function were only `nb_ops` long, then the `aesni_mb_dequeue_burst()`
> would write to the memory outside of `ops` array.
>
> Fixes: b50b8b5b38f8 ("crypto/ipsec_mb: use burst API in AESNI")
> Cc: stable@dpdk.org
>
> Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
Acked-by: Ciara Power <ciara.power@intel.com>
> -----Original Message-----
> From: Karas, Krzysztof <krzysztof.karas@intel.com>
> Sent: Monday, November 13, 2023 8:02 AM
> To: Ji, Kai <kai.ji@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; Cornu, Marcel D
> <marcel.d.cornu@intel.com>; Power, Ciara <ciara.power@intel.com>
> Cc: dev@dpdk.org; Karas, Krzysztof <krzysztof.karas@intel.com>;
> stable@dpdk.org
> Subject: [PATCH v2] crypto/ipsec_mb: do not dequeue ops after flush
>
> Previously it was possible to increment `processed_jobs` to a value greater than
> requested `nb_ops`, because after flushing at most `nb_ops` jobs the while loop
> continued, so `processed_jobs` could still be incremented and it was possible for
> this variable to be greater than `nb_ops`. If `ops` provided to the function were
> only `nb_ops` long, then the `aesni_mb_dequeue_burst()` would write to the
> memory outside of `ops` array.
>
> Fixes: b50b8b5b38f8 ("crypto/ipsec_mb: use burst API in AESNI")
> Cc: stable@dpdk.org
>
> Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
Acked-by: Marcel Cornu <marcel.d.cornu@intel.com>
> -----Original Message-----
> From: Cornu, Marcel D <marcel.d.cornu@intel.com>
> Sent: Monday, November 13, 2023 11:13 AM
> To: Karas, Krzysztof <krzysztof.karas@intel.com>; Ji, Kai <kai.ji@intel.com>; De
> Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Power, Ciara
> <ciara.power@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: RE: [PATCH v2] crypto/ipsec_mb: do not dequeue ops after flush
>
>
> > -----Original Message-----
> > From: Karas, Krzysztof <krzysztof.karas@intel.com>
> > Sent: Monday, November 13, 2023 8:02 AM
> > To: Ji, Kai <kai.ji@intel.com>; De Lara Guarch, Pablo
> > <pablo.de.lara.guarch@intel.com>; Cornu, Marcel D
> > <marcel.d.cornu@intel.com>; Power, Ciara <ciara.power@intel.com>
> > Cc: dev@dpdk.org; Karas, Krzysztof <krzysztof.karas@intel.com>;
> > stable@dpdk.org
> > Subject: [PATCH v2] crypto/ipsec_mb: do not dequeue ops after flush
> >
> > Previously it was possible to increment `processed_jobs` to a value
> > greater than requested `nb_ops`, because after flushing at most
> > `nb_ops` jobs the while loop continued, so `processed_jobs` could
> > still be incremented and it was possible for this variable to be
> > greater than `nb_ops`. If `ops` provided to the function were only
> > `nb_ops` long, then the `aesni_mb_dequeue_burst()` would write to the
> memory outside of `ops` array.
> >
> > Fixes: b50b8b5b38f8 ("crypto/ipsec_mb: use burst API in AESNI")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
>
> Acked-by: Marcel Cornu <marcel.d.cornu@intel.com>
Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> > > Subject: [PATCH v2] crypto/ipsec_mb: do not dequeue ops after flush
> > >
> > > Previously it was possible to increment `processed_jobs` to a value
> > > greater than requested `nb_ops`, because after flushing at most
> > > `nb_ops` jobs the while loop continued, so `processed_jobs` could
> > > still be incremented and it was possible for this variable to be
> > > greater than `nb_ops`. If `ops` provided to the function were only
> > > `nb_ops` long, then the `aesni_mb_dequeue_burst()` would write to the
> > memory outside of `ops` array.
> > >
> > > Fixes: b50b8b5b38f8 ("crypto/ipsec_mb: use burst API in AESNI")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
> >
> > Acked-by: Marcel Cornu <marcel.d.cornu@intel.com>
>
> Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
Applied to dpdk-next-crypto
Thanks.
@@ -2056,7 +2056,7 @@ aesni_mb_dequeue_burst(void *queue_pair, struct rte_crypto_op **ops,
uint16_t n = (nb_ops / burst_sz) ?
burst_sz : nb_ops;
- while (unlikely((IMB_GET_NEXT_BURST(mb_mgr, n, jobs)) < n)) {
+ if (unlikely((IMB_GET_NEXT_BURST(mb_mgr, n, jobs)) < n)) {
/*
* Not enough free jobs in the queue
* Flush n jobs until enough jobs available
@@ -2074,6 +2074,8 @@ aesni_mb_dequeue_burst(void *queue_pair, struct rte_crypto_op **ops,
break;
}
}
+ nb_ops -= i;
+ continue;
}
/*