[v2] crypto/ipsec_mb: fix usage of untrusted value
Checks
Commit Message
This patch removes coverity defect CID 375828:
Untrusted value as argument (TAINTED_SCALAR)
Coverity issue: CID 375828
Fixes: 918fd2f1466b ("crypto/ipsec_mb: move aesni_mb PMD")
Signed-off-by: Piotr Bronowski <piotrx.bronowski@intel.com>
Cc: stable@dpdk.org
---
v2: use a different logic to check digest length
---
drivers/crypto/ipsec_mb/pmd_aesni_gcm.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
Comments
> -----Original Message-----
> From: Bronowski, PiotrX <piotrx.bronowski@intel.com>
> Sent: Monday, March 7, 2022 3:33 PM
> To: dev@dpdk.org
> Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>; thomas@monjalon.net;
> gakhil@marvell.com; Yigit, Ferruh <ferruh.yigit@intel.com>; Doherty, Declan
> <declan.doherty@intel.com>; Bronowski, PiotrX
> <piotrx.bronowski@intel.com>; stable@dpdk.org
> Subject: [PATCH v2] crypto/ipsec_mb: fix usage of untrusted value
>
> This patch removes coverity defect CID 375828:
> Untrusted value as argument (TAINTED_SCALAR)
>
> Coverity issue: CID 375828
> Fixes: 918fd2f1466b ("crypto/ipsec_mb: move aesni_mb PMD")
>
> Signed-off-by: Piotr Bronowski <piotrx.bronowski@intel.com>
>
> Cc: stable@dpdk.org
>
> ---
> v2: use a different logic to check digest length
> ---
Acked-by: Fan Zhang <roy.fan.zhang@intel.com>
> > -----Original Message-----
> > From: Bronowski, PiotrX <piotrx.bronowski@intel.com>
> > Sent: Monday, March 7, 2022 3:33 PM
> > To: dev@dpdk.org
> > Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>; thomas@monjalon.net;
> > gakhil@marvell.com; Yigit, Ferruh <ferruh.yigit@intel.com>; Doherty,
> > Declan <declan.doherty@intel.com>; Bronowski, PiotrX
> > <piotrx.bronowski@intel.com>; stable@dpdk.org
> > Subject: [PATCH v2] crypto/ipsec_mb: fix usage of untrusted value
> >
> > This patch removes coverity defect CID 375828:
> > Untrusted value as argument (TAINTED_SCALAR)
> >
> > Coverity issue: CID 375828
> > Fixes: 918fd2f1466b ("crypto/ipsec_mb: move aesni_mb PMD")
> >
> > Signed-off-by: Piotr Bronowski <piotrx.bronowski@intel.com>
> >
> > Cc: stable@dpdk.org
> >
> > ---
> > v2: use a different logic to check digest length
> > ---
> Acked-by: Fan Zhang <roy.fan.zhang@intel.com>
Acked-by: Kai Ji <kai.ji@intel.com>
Hi Piotr,
> -----Original Message-----
> From: Zhang, Roy Fan <roy.fan.zhang@intel.com>
> Sent: Monday, March 7, 2022 4:27 PM
> To: Bronowski, PiotrX <piotrx.bronowski@intel.com>; dev@dpdk.org
> Cc: thomas@monjalon.net; gakhil@marvell.com; Yigit, Ferruh
> <ferruh.yigit@intel.com>; Doherty, Declan <declan.doherty@intel.com>;
> stable@dpdk.org
> Subject: RE: [PATCH v2] crypto/ipsec_mb: fix usage of untrusted value
>
> > -----Original Message-----
> > From: Bronowski, PiotrX <piotrx.bronowski@intel.com>
> > Sent: Monday, March 7, 2022 3:33 PM
> > To: dev@dpdk.org
> > Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>; thomas@monjalon.net;
> > gakhil@marvell.com; Yigit, Ferruh <ferruh.yigit@intel.com>; Doherty,
> Declan
> > <declan.doherty@intel.com>; Bronowski, PiotrX
> > <piotrx.bronowski@intel.com>; stable@dpdk.org
> > Subject: [PATCH v2] crypto/ipsec_mb: fix usage of untrusted value
> >
> > This patch removes coverity defect CID 375828:
> > Untrusted value as argument (TAINTED_SCALAR)
> >
> > Coverity issue: CID 375828
> > Fixes: 918fd2f1466b ("crypto/ipsec_mb: move aesni_mb PMD")
> >
> > Signed-off-by: Piotr Bronowski <piotrx.bronowski@intel.com>
> >
> > Cc: stable@dpdk.org
> >
> > ---
> > v2: use a different logic to check digest length
> > ---
> Acked-by: Fan Zhang <roy.fan.zhang@intel.com>
Sorry I missed a point in your change and thanks for Ciara pointing this out.
You are changing the gen_digest_size to 64 which is wrong.
Please send v3.
Also instead of ack - Nack this patch.
Hi Piotr,
>-----Original Message-----
>From: Zhang, Roy Fan <roy.fan.zhang@intel.com>
>Sent: Wednesday 9 March 2022 14:35
>To: Zhang, Roy Fan <roy.fan.zhang@intel.com>; Bronowski, PiotrX
><piotrx.bronowski@intel.com>; dev@dpdk.org
>Cc: thomas@monjalon.net; gakhil@marvell.com; Yigit, Ferruh
><ferruh.yigit@intel.com>; Doherty, Declan <declan.doherty@intel.com>;
>stable@dpdk.org; Power, Ciara <ciara.power@intel.com>
>Subject: RE: [PATCH v2] crypto/ipsec_mb: fix usage of untrusted value
>
>Hi Piotr,
>
>> -----Original Message-----
>> From: Zhang, Roy Fan <roy.fan.zhang@intel.com>
>> Sent: Monday, March 7, 2022 4:27 PM
>> To: Bronowski, PiotrX <piotrx.bronowski@intel.com>; dev@dpdk.org
>> Cc: thomas@monjalon.net; gakhil@marvell.com; Yigit, Ferruh
>> <ferruh.yigit@intel.com>; Doherty, Declan <declan.doherty@intel.com>;
>> stable@dpdk.org
>> Subject: RE: [PATCH v2] crypto/ipsec_mb: fix usage of untrusted value
>>
>> > -----Original Message-----
>> > From: Bronowski, PiotrX <piotrx.bronowski@intel.com>
>> > Sent: Monday, March 7, 2022 3:33 PM
>> > To: dev@dpdk.org
>> > Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>; thomas@monjalon.net;
>> > gakhil@marvell.com; Yigit, Ferruh <ferruh.yigit@intel.com>; Doherty,
>> Declan
>> > <declan.doherty@intel.com>; Bronowski, PiotrX
>> > <piotrx.bronowski@intel.com>; stable@dpdk.org
>> > Subject: [PATCH v2] crypto/ipsec_mb: fix usage of untrusted value
>> >
>> > This patch removes coverity defect CID 375828:
>> > Untrusted value as argument (TAINTED_SCALAR)
>> >
>> > Coverity issue: CID 375828
>> > Fixes: 918fd2f1466b ("crypto/ipsec_mb: move aesni_mb PMD")
>> >
>> > Signed-off-by: Piotr Bronowski <piotrx.bronowski@intel.com>
>> >
>> > Cc: stable@dpdk.org
>> >
>> > ---
>> > v2: use a different logic to check digest length
>> > ---
>> Acked-by: Fan Zhang <roy.fan.zhang@intel.com>
>
>Sorry I missed a point in your change and thanks for Ciara pointing this out.
>You are changing the gen_digest_size to 64 which is wrong.
>Please send v3.
>Also instead of ack - Nack this patch.
[CP]
In the v3 I think Fixes line should also be updated to either:
Fixes: 746825e5c0ea ("crypto/ipsec_mb: move aesni_gcm PMD")
Or
Fixes: ceb863938708 ("crypto/aesni_gcm: support all truncated digest sizes")
Cc: pablo.de.lara.guarch@intel.com
(The second one seems to be where the code was introduced before being moved into the consolidated ipsec_mb PMD in 21.11)
Thanks,
Ciara
@@ -96,7 +96,9 @@ aesni_gcm_session_configure(IMB_MGR *mb_mgr, void *session,
sess->iv.length = auth_xform->auth.iv.length;
key_length = auth_xform->auth.key.length;
key = auth_xform->auth.key.data;
- sess->req_digest_length = auth_xform->auth.digest_length;
+ sess->req_digest_length =
+ RTE_MIN(auth_xform->auth.digest_length,
+ DIGEST_LENGTH_MAX);
break;
case IPSEC_MB_OP_AEAD_AUTHENTICATED_ENCRYPT:
case IPSEC_MB_OP_AEAD_AUTHENTICATED_DECRYPT:
@@ -116,7 +118,9 @@ aesni_gcm_session_configure(IMB_MGR *mb_mgr, void *session,
key_length = aead_xform->aead.key.length;
key = aead_xform->aead.key.data;
sess->aad_length = aead_xform->aead.aad_length;
- sess->req_digest_length = aead_xform->aead.digest_length;
+ sess->req_digest_length =
+ RTE_MIN(aead_xform->aead.digest_length,
+ DIGEST_LENGTH_MAX);
break;
default:
IPSEC_MB_LOG(
@@ -146,7 +150,7 @@ aesni_gcm_session_configure(IMB_MGR *mb_mgr, void *session,
}
/* Digest check */
- if (sess->req_digest_length > 16) {
+ if (sess->req_digest_length > DIGEST_LENGTH_MAX) {
IPSEC_MB_LOG(ERR, "Invalid digest length");
ret = -EINVAL;
goto error_exit;
@@ -157,7 +161,7 @@ aesni_gcm_session_configure(IMB_MGR *mb_mgr, void *session,
* the requested number of bytes.
*/
if (sess->req_digest_length < 4)
- sess->gen_digest_length = 16;
+ sess->gen_digest_length = DIGEST_LENGTH_MAX;
else
sess->gen_digest_length = sess->req_digest_length;