[PATCH v1] examples/fips_validation: fix memory leak in sha test

[PATCH v1] examples/fips_validation: fix memory leak in sha test

[Gowrishankar Muthukrishnan]

There is wrong size used for allocation of digest buffer which in
some cases cause memory corruption. Also, fixed places where memory
leak is observed.

Fixes: 93d797d94f1 ("examples/fips_validation: add parsing for sha")

Signed-off-by: Gowrishankar Muthukrishnan <gmuthukrishn@marvell.com>
---
 examples/fips_validation/fips_validation_sha.c | 10 ++++++++--
 examples/fips_validation/main.c                |  1 +
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/examples/fips_validation/fips_validation_sha.c b/examples/fips_validation/fips_validation_sha.c
index a2928618d7..538cb6647a 100644
--- a/examples/fips_validation/fips_validation_sha.c
+++ b/examples/fips_validation/fips_validation_sha.c
@@ -229,13 +229,19 @@ parse_test_sha_json_algorithm(void)
 	for (i = 0; i < RTE_DIM(phsc); i++) {
 		if (info.interim_info.sha_data.algo == phsc[i].algo) {
 			vec.cipher_auth.digest.len = atoi(phsc[i].str);
-			vec.cipher_auth.digest.val = calloc(0, vec.cipher_auth.digest.len * 8);
+			if (vec.cipher_auth.digest.val)
+				free(vec.cipher_auth.digest.val);
+
+			vec.cipher_auth.digest.val = calloc(1, vec.cipher_auth.digest.len);
 			break;
 		}
 	}
 
-	if (i == RTE_DIM(phsc))
+	if (i == RTE_DIM(phsc)) {
+		free(vec.cipher_auth.digest.val);
+		vec.cipher_auth.digest.val = NULL;
 		return -1;
+	}
 
 	return 0;
 }
diff --git a/examples/fips_validation/main.c b/examples/fips_validation/main.c
index 6d52048b5c..8bd5a66889 100644
--- a/examples/fips_validation/main.c
+++ b/examples/fips_validation/main.c
@@ -2099,6 +2099,7 @@ fips_test_one_json_file(void)
 		json_info.json_vector_set = json_array_get(json_info.json_root, vector_set_idx);
 		fips_test_one_vector_set();
 		json_array_append_new(json_info.json_write_root, json_info.json_write_set);
+		json_incref(json_info.json_write_set);
 	}
 
 	json_dumpf(json_info.json_write_root, info.fp_wr, JSON_INDENT(4));
-- 
2.25.1

[PATCH v2] examples/fips_validation: fix memory leak in sha test

[Gowrishankar Muthukrishnan]

There is wrong size used for allocation of digest buffer which in
some cases cause memory corruption. Also, fixed places where memory
leak is observed. This fix would enable sha 384 and 512 NIST vectors
be supported fully.

Fixes: 93d797d94f1 ("examples/fips_validation: add parsing for sha")

Signed-off-by: Gowrishankar Muthukrishnan <gmuthukrishn@marvell.com>
---
v2:
 - doc update to support 384 and 512 sha.
---
 doc/guides/sample_app_ug/fips_validation.rst   |  2 +-
 examples/fips_validation/fips_validation_sha.c | 10 ++++++++--
 examples/fips_validation/main.c                |  1 +
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/doc/guides/sample_app_ug/fips_validation.rst b/doc/guides/sample_app_ug/fips_validation.rst
index 4b68226665..6f4bd34726 100644
--- a/doc/guides/sample_app_ug/fips_validation.rst
+++ b/doc/guides/sample_app_ug/fips_validation.rst
@@ -63,7 +63,7 @@ ACVP
     * AES-CMAC (128,192,256) - AFT
     * AES-XTS (128,256) - AFT
     * HMAC (SHA1, SHA224, SHA256, SHA384, SHA512)
-    * SHA (1,256) - AFT, MCT
+    * SHA (1, 256, 384, 512) - AFT, MCT
 
 
 Application Information
diff --git a/examples/fips_validation/fips_validation_sha.c b/examples/fips_validation/fips_validation_sha.c
index a2928618d7..538cb6647a 100644
--- a/examples/fips_validation/fips_validation_sha.c
+++ b/examples/fips_validation/fips_validation_sha.c
@@ -229,13 +229,19 @@ parse_test_sha_json_algorithm(void)
 	for (i = 0; i < RTE_DIM(phsc); i++) {
 		if (info.interim_info.sha_data.algo == phsc[i].algo) {
 			vec.cipher_auth.digest.len = atoi(phsc[i].str);
-			vec.cipher_auth.digest.val = calloc(0, vec.cipher_auth.digest.len * 8);
+			if (vec.cipher_auth.digest.val)
+				free(vec.cipher_auth.digest.val);
+
+			vec.cipher_auth.digest.val = calloc(1, vec.cipher_auth.digest.len);
 			break;
 		}
 	}
 
-	if (i == RTE_DIM(phsc))
+	if (i == RTE_DIM(phsc)) {
+		free(vec.cipher_auth.digest.val);
+		vec.cipher_auth.digest.val = NULL;
 		return -1;
+	}
 
 	return 0;
 }
diff --git a/examples/fips_validation/main.c b/examples/fips_validation/main.c
index 6d52048b5c..8bd5a66889 100644
--- a/examples/fips_validation/main.c
+++ b/examples/fips_validation/main.c
@@ -2099,6 +2099,7 @@ fips_test_one_json_file(void)
 		json_info.json_vector_set = json_array_get(json_info.json_root, vector_set_idx);
 		fips_test_one_vector_set();
 		json_array_append_new(json_info.json_write_root, json_info.json_write_set);
+		json_incref(json_info.json_write_set);
 	}
 
 	json_dumpf(json_info.json_write_root, info.fp_wr, JSON_INDENT(4));
-- 
2.25.1

Re: [PATCH v2] examples/fips_validation: fix memory leak in sha test

[David Marchand]

On Sat, Jul 2, 2022 at 4:59 PM Gowrishankar Muthukrishnan
<gmuthukrishn@marvell.com> wrote:
>
> There is wrong size used for allocation of digest buffer which in
> some cases cause memory corruption. Also, fixed places where memory
> leak is observed. This fix would enable sha 384 and 512 NIST vectors
> be supported fully.
>
> Fixes: 93d797d94f1 ("examples/fips_validation: add parsing for sha")

This sha1 is not valid anymore, or maybe Akhil will squash the fix in
the next-crypto tree before reaching main.

>
> Signed-off-by: Gowrishankar Muthukrishnan <gmuthukrishn@marvell.com>
> ---
> v2:
>  - doc update to support 384 and 512 sha.
> ---
>  doc/guides/sample_app_ug/fips_validation.rst   |  2 +-
>  examples/fips_validation/fips_validation_sha.c | 10 ++++++++--
>  examples/fips_validation/main.c                |  1 +
>  3 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/doc/guides/sample_app_ug/fips_validation.rst b/doc/guides/sample_app_ug/fips_validation.rst
> index 4b68226665..6f4bd34726 100644
> --- a/doc/guides/sample_app_ug/fips_validation.rst
> +++ b/doc/guides/sample_app_ug/fips_validation.rst
> @@ -63,7 +63,7 @@ ACVP
>      * AES-CMAC (128,192,256) - AFT
>      * AES-XTS (128,256) - AFT
>      * HMAC (SHA1, SHA224, SHA256, SHA384, SHA512)
> -    * SHA (1,256) - AFT, MCT
> +    * SHA (1, 256, 384, 512) - AFT, MCT
>
>
>  Application Information
> diff --git a/examples/fips_validation/fips_validation_sha.c b/examples/fips_validation/fips_validation_sha.c
> index a2928618d7..538cb6647a 100644
> --- a/examples/fips_validation/fips_validation_sha.c
> +++ b/examples/fips_validation/fips_validation_sha.c
> @@ -229,13 +229,19 @@ parse_test_sha_json_algorithm(void)
>         for (i = 0; i < RTE_DIM(phsc); i++) {
>                 if (info.interim_info.sha_data.algo == phsc[i].algo) {
>                         vec.cipher_auth.digest.len = atoi(phsc[i].str);
> -                       vec.cipher_auth.digest.val = calloc(0, vec.cipher_auth.digest.len * 8);
> +                       if (vec.cipher_auth.digest.val)
> +                               free(vec.cipher_auth.digest.val);

Unneeded if().

We just did a tree-wide cleanup to avoid these.
Please don't reintroduce some.


> +
> +                       vec.cipher_auth.digest.val = calloc(1, vec.cipher_auth.digest.len);

Don't we need to check for allocation success?


>                         break;
>                 }
>         }
>
> -       if (i == RTE_DIM(phsc))
> +       if (i == RTE_DIM(phsc)) {
> +               free(vec.cipher_auth.digest.val);
> +               vec.cipher_auth.digest.val = NULL;
>                 return -1;
> +       }
>
>         return 0;
>  }


-- 
David Marchand

RE: [EXT] Re: [PATCH v2] examples/fips_validation: fix memory leak in sha test

[Akhil Goyal]

> On Sat, Jul 2, 2022 at 4:59 PM Gowrishankar Muthukrishnan
> <gmuthukrishn@marvell.com> wrote:
> >
> > There is wrong size used for allocation of digest buffer which in
> > some cases cause memory corruption. Also, fixed places where memory
> > leak is observed. This fix would enable sha 384 and 512 NIST vectors
> > be supported fully.
> >
> > Fixes: 93d797d94f1 ("examples/fips_validation: add parsing for sha")
> 
> This sha1 is not valid anymore, or maybe Akhil will squash the fix in
> the next-crypto tree before reaching main.
Fixed the SHA and applied on top of next-crypto as I had already asked Thomas to pull the tree.

Re: [EXT] Re: [PATCH v2] examples/fips_validation: fix memory leak in sha test

[Thomas Monjalon]

04/07/2022 09:50, Akhil Goyal:
> > On Sat, Jul 2, 2022 at 4:59 PM Gowrishankar Muthukrishnan
> > <gmuthukrishn@marvell.com> wrote:
> > >
> > > There is wrong size used for allocation of digest buffer which in
> > > some cases cause memory corruption. Also, fixed places where memory
> > > leak is observed. This fix would enable sha 384 and 512 NIST vectors
> > > be supported fully.
> > >
> > > Fixes: 93d797d94f1 ("examples/fips_validation: add parsing for sha")
> > 
> > This sha1 is not valid anymore, or maybe Akhil will squash the fix in
> > the next-crypto tree before reaching main.
> Fixed the SHA and applied on top of next-crypto as I had already asked Thomas to pull the tree.

David did some good comments, I will wait for a better version of the patch.

RE: [EXT] Re: [PATCH v2] examples/fips_validation: fix memory leak in sha test

[Gowrishankar Muthukrishnan]

> vec.cipher_auth.digest.len * 8);
> > +                       if (vec.cipher_auth.digest.val)
> > +                               free(vec.cipher_auth.digest.val);
> 
> Unneeded if().
> 
Ack.

> We just did a tree-wide cleanup to avoid these.
> Please don't reintroduce some.
> 
> 
> > +
> > +                       vec.cipher_auth.digest.val = calloc(1,
> > + vec.cipher_auth.digest.len);
> 
> Don't we need to check for allocation success?
> 
Sure. I ll post fix for the  above and thanks for the review.

Regards,
Gowrishankar
> 
> >                         break;
> >                 }
> >         }
> >
> > -       if (i == RTE_DIM(phsc))
> > +       if (i == RTE_DIM(phsc)) {
> > +               free(vec.cipher_auth.digest.val);
> > +               vec.cipher_auth.digest.val = NULL;
> >                 return -1;
> > +       }
> >
> >         return 0;
> >  }
> 
> 
> --
> David Marchand

[PATCH v2] examples/fips_validation: fix memory leak in sha test

[Gowrishankar Muthukrishnan]

There is wrong size used for allocation of digest buffer which in
some cases cause memory corruption. Also, fixed places where memory
leak is observed. This fix would enable sha 384 and 512 NIST vectors
be supported fully.

Fixes: 93d797d94f1 ("examples/fips_validation: add parsing for sha")

Signed-off-by: Gowrishankar Muthukrishnan <gmuthukrishn@marvell.com>
---
Depends-on: series-23828 ("example/fips_validation: add xts and sha json parsing")
---
 doc/guides/sample_app_ug/fips_validation.rst   |  2 +-
 examples/fips_validation/fips_validation_sha.c | 10 ++++++++--
 examples/fips_validation/main.c                |  1 +
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/doc/guides/sample_app_ug/fips_validation.rst b/doc/guides/sample_app_ug/fips_validation.rst
index 4b68226665..6f4bd34726 100644
--- a/doc/guides/sample_app_ug/fips_validation.rst
+++ b/doc/guides/sample_app_ug/fips_validation.rst
@@ -63,7 +63,7 @@ ACVP
     * AES-CMAC (128,192,256) - AFT
     * AES-XTS (128,256) - AFT
     * HMAC (SHA1, SHA224, SHA256, SHA384, SHA512)
-    * SHA (1,256) - AFT, MCT
+    * SHA (1, 256, 384, 512) - AFT, MCT
 
 
 Application Information
diff --git a/examples/fips_validation/fips_validation_sha.c b/examples/fips_validation/fips_validation_sha.c
index a2928618d7..538cb6647a 100644
--- a/examples/fips_validation/fips_validation_sha.c
+++ b/examples/fips_validation/fips_validation_sha.c
@@ -229,13 +229,19 @@ parse_test_sha_json_algorithm(void)
 	for (i = 0; i < RTE_DIM(phsc); i++) {
 		if (info.interim_info.sha_data.algo == phsc[i].algo) {
 			vec.cipher_auth.digest.len = atoi(phsc[i].str);
-			vec.cipher_auth.digest.val = calloc(0, vec.cipher_auth.digest.len * 8);
+			if (vec.cipher_auth.digest.val)
+				free(vec.cipher_auth.digest.val);
+
+			vec.cipher_auth.digest.val = calloc(1, vec.cipher_auth.digest.len);
 			break;
 		}
 	}
 
-	if (i == RTE_DIM(phsc))
+	if (i == RTE_DIM(phsc)) {
+		free(vec.cipher_auth.digest.val);
+		vec.cipher_auth.digest.val = NULL;
 		return -1;
+	}
 
 	return 0;
 }
diff --git a/examples/fips_validation/main.c b/examples/fips_validation/main.c
index 6d52048b5c..8bd5a66889 100644
--- a/examples/fips_validation/main.c
+++ b/examples/fips_validation/main.c
@@ -2099,6 +2099,7 @@ fips_test_one_json_file(void)
 		json_info.json_vector_set = json_array_get(json_info.json_root, vector_set_idx);
 		fips_test_one_vector_set();
 		json_array_append_new(json_info.json_write_root, json_info.json_write_set);
+		json_incref(json_info.json_write_set);
 	}
 
 	json_dumpf(json_info.json_write_root, info.fp_wr, JSON_INDENT(4));
-- 
2.25.1

RE: [PATCH v2] examples/fips_validation: fix memory leak in sha test

[Akhil Goyal]

> Subject: [PATCH v2] examples/fips_validation: fix memory leak in sha test
> 
> There is wrong size used for allocation of digest buffer which in
> some cases cause memory corruption. Also, fixed places where memory
> leak is observed. This fix would enable sha 384 and 512 NIST vectors
> be supported fully.
> 
> Fixes: 93d797d94f1 ("examples/fips_validation: add parsing for sha")
> 
> Signed-off-by: Gowrishankar Muthukrishnan <gmuthukrishn@marvell.com>
> ---
Applied to dpdk-next-crypto

Thanks.

[PATCH v3] examples/fips_validation: fix memory leak in sha test

[Gowrishankar Muthukrishnan]

There is wrong size used for allocation of digest buffer which in
some cases cause memory corruption. Also, fixed places where memory
leak is observed. This fix would enable sha 384 and 512 NIST vectors
be supported fully.

Signed-off-by: Gowrishankar Muthukrishnan <gmuthukrishn@marvell.com>
---
v3:
 - addes fixes for suggestions in v2.
---
 doc/guides/sample_app_ug/fips_validation.rst   |  2 +-
 examples/fips_validation/fips_validation_sha.c | 11 +++++++++--
 examples/fips_validation/main.c                |  1 +
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/doc/guides/sample_app_ug/fips_validation.rst b/doc/guides/sample_app_ug/fips_validation.rst
index 4b68226665..6f4bd34726 100644
--- a/doc/guides/sample_app_ug/fips_validation.rst
+++ b/doc/guides/sample_app_ug/fips_validation.rst
@@ -63,7 +63,7 @@ ACVP
     * AES-CMAC (128,192,256) - AFT
     * AES-XTS (128,256) - AFT
     * HMAC (SHA1, SHA224, SHA256, SHA384, SHA512)
-    * SHA (1,256) - AFT, MCT
+    * SHA (1, 256, 384, 512) - AFT, MCT
 
 
 Application Information
diff --git a/examples/fips_validation/fips_validation_sha.c b/examples/fips_validation/fips_validation_sha.c
index a2928618d7..dff552586f 100644
--- a/examples/fips_validation/fips_validation_sha.c
+++ b/examples/fips_validation/fips_validation_sha.c
@@ -229,13 +229,20 @@ parse_test_sha_json_algorithm(void)
 	for (i = 0; i < RTE_DIM(phsc); i++) {
 		if (info.interim_info.sha_data.algo == phsc[i].algo) {
 			vec.cipher_auth.digest.len = atoi(phsc[i].str);
-			vec.cipher_auth.digest.val = calloc(0, vec.cipher_auth.digest.len * 8);
+			free(vec.cipher_auth.digest.val);
+			vec.cipher_auth.digest.val = calloc(1, vec.cipher_auth.digest.len);
+			if (vec.cipher_auth.digest.val == NULL)
+				return -1;
+
 			break;
 		}
 	}
 
-	if (i == RTE_DIM(phsc))
+	if (i == RTE_DIM(phsc)) {
+		free(vec.cipher_auth.digest.val);
+		vec.cipher_auth.digest.val = NULL;
 		return -1;
+	}
 
 	return 0;
 }
diff --git a/examples/fips_validation/main.c b/examples/fips_validation/main.c
index 6d52048b5c..8bd5a66889 100644
--- a/examples/fips_validation/main.c
+++ b/examples/fips_validation/main.c
@@ -2099,6 +2099,7 @@ fips_test_one_json_file(void)
 		json_info.json_vector_set = json_array_get(json_info.json_root, vector_set_idx);
 		fips_test_one_vector_set();
 		json_array_append_new(json_info.json_write_root, json_info.json_write_set);
+		json_incref(json_info.json_write_set);
 	}
 
 	json_dumpf(json_info.json_write_root, info.fp_wr, JSON_INDENT(4));
-- 
2.25.1

RE: [PATCH v3] examples/fips_validation: fix memory leak in sha test

[Akhil Goyal]

> Subject: [PATCH v3] examples/fips_validation: fix memory leak in sha test
> 
> There is wrong size used for allocation of digest buffer which in
> some cases cause memory corruption. Also, fixed places where memory
> leak is observed. This fix would enable sha 384 and 512 NIST vectors
> be supported fully.

Fixes: c9819d389b4b ("examples/fips_validation: add parsing for SHA")

++David/Thomas
> 
> Signed-off-by: Gowrishankar Muthukrishnan <gmuthukrishn@marvell.com>
> ---
> v3:
>  - addes fixes for suggestions in v2.
> ---
>  doc/guides/sample_app_ug/fips_validation.rst   |  2 +-
>  examples/fips_validation/fips_validation_sha.c | 11 +++++++++--
>  examples/fips_validation/main.c                |  1 +
>  3 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/doc/guides/sample_app_ug/fips_validation.rst
> b/doc/guides/sample_app_ug/fips_validation.rst
> index 4b68226665..6f4bd34726 100644
> --- a/doc/guides/sample_app_ug/fips_validation.rst
> +++ b/doc/guides/sample_app_ug/fips_validation.rst
> @@ -63,7 +63,7 @@ ACVP
>      * AES-CMAC (128,192,256) - AFT
>      * AES-XTS (128,256) - AFT
>      * HMAC (SHA1, SHA224, SHA256, SHA384, SHA512)
> -    * SHA (1,256) - AFT, MCT
> +    * SHA (1, 256, 384, 512) - AFT, MCT
> 
> 
>  Application Information
> diff --git a/examples/fips_validation/fips_validation_sha.c
> b/examples/fips_validation/fips_validation_sha.c
> index a2928618d7..dff552586f 100644
> --- a/examples/fips_validation/fips_validation_sha.c
> +++ b/examples/fips_validation/fips_validation_sha.c
> @@ -229,13 +229,20 @@ parse_test_sha_json_algorithm(void)
>  	for (i = 0; i < RTE_DIM(phsc); i++) {
>  		if (info.interim_info.sha_data.algo == phsc[i].algo) {
>  			vec.cipher_auth.digest.len = atoi(phsc[i].str);
> -			vec.cipher_auth.digest.val = calloc(0,
> vec.cipher_auth.digest.len * 8);
> +			free(vec.cipher_auth.digest.val);
> +			vec.cipher_auth.digest.val = calloc(1,
> vec.cipher_auth.digest.len);
> +			if (vec.cipher_auth.digest.val == NULL)
> +				return -1;
> +
>  			break;
>  		}
>  	}
> 
> -	if (i == RTE_DIM(phsc))
> +	if (i == RTE_DIM(phsc)) {
> +		free(vec.cipher_auth.digest.val);
> +		vec.cipher_auth.digest.val = NULL;
>  		return -1;
> +	}
> 
>  	return 0;
>  }
> diff --git a/examples/fips_validation/main.c b/examples/fips_validation/main.c
> index 6d52048b5c..8bd5a66889 100644
> --- a/examples/fips_validation/main.c
> +++ b/examples/fips_validation/main.c
> @@ -2099,6 +2099,7 @@ fips_test_one_json_file(void)
>  		json_info.json_vector_set =
> json_array_get(json_info.json_root, vector_set_idx);
>  		fips_test_one_vector_set();
>  		json_array_append_new(json_info.json_write_root,
> json_info.json_write_set);
> +		json_incref(json_info.json_write_set);
>  	}
> 
>  	json_dumpf(json_info.json_write_root, info.fp_wr, JSON_INDENT(4));
> --
> 2.25.1

RE: [PATCH v3] examples/fips_validation: fix memory leak in sha test

[Akhil Goyal]

> Subject: RE: [PATCH v3] examples/fips_validation: fix memory leak in sha test
> 
> > Subject: [PATCH v3] examples/fips_validation: fix memory leak in sha test
> >
> > There is wrong size used for allocation of digest buffer which in
> > some cases cause memory corruption. Also, fixed places where memory
> > leak is observed. This fix would enable sha 384 and 512 NIST vectors
> > be supported fully.
> 
> Fixes: c9819d389b4b ("examples/fips_validation: add parsing for SHA")
> 
> ++David/Thomas
Squashed with original commit.

Thanks.