[dpdk-dev] [PATCH] app: fix buffer overrun

[dpdk-dev] [PATCH] app: fix buffer overrun

[Przemyslaw Zegan]

This patch fixes a possible buffer overrun problem in crypto perf test.
Previously when user configured aad size is over 12 bytes the copy of template aad will cause a buffer overrun.
The problem is fixed by only copy up to 12 bytes of aad template.

Fixes: 761a321acf91 ("event/cnxk: support vectorized Tx event fast path" )
Cc: pbhagavatula@marvell.com

Signed-off-by: Przemyslaw Zegan <przemyslawx.zegan@intel.com>
---
 app/test-crypto-perf/cperf_test_vectors.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/app/test-crypto-perf/cperf_test_vectors.c b/app/test-crypto-perf/cperf_test_vectors.c
index 0af01ff911..2c7e314ec8 100644
--- a/app/test-crypto-perf/cperf_test_vectors.c
+++ b/app/test-crypto-perf/cperf_test_vectors.c
@@ -548,12 +548,16 @@ cperf_test_vector_get_dummy(struct cperf_options *options)
 		t_vec->aead_key.data = aead_key;
 
 		if (options->aead_aad_sz) {
-			t_vec->aad.data = rte_malloc(NULL,
+			t_vec->aad.data = rte_zmalloc(NULL,
 					options->aead_aad_sz, 16);
 			if (t_vec->aad.data == NULL) {
 				rte_free(t_vec);
 				return NULL;
 			}
+
+			if(options->aead_aad_sz > 12)
+				options->aead_aad_sz = 12;
+
 			memcpy(t_vec->aad.data, aad, options->aead_aad_sz);
 			t_vec->aad.phys_addr = rte_malloc_virt2iova(t_vec->aad.data);
 			t_vec->aad.length = options->aead_aad_sz;
-- 
2.17.1

--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.

Re: [dpdk-dev] [EXT] [PATCH] app: fix buffer overrun

[Pavan Nikhilesh Bhagavatula]

>This patch fixes a possible buffer overrun problem in crypto perf test.
>Previously when user configured aad size is over 12 bytes the copy of
>template aad will cause a buffer overrun.
>The problem is fixed by only copy up to 12 bytes of aad template.
>
>Fixes: 761a321acf91 ("event/cnxk: support vectorized Tx event fast
>path" )
>Cc: pbhagavatula@marvell.com
>

I don't see how the above mentioned patch is related to test-crypto-perf.
Could you please recheck?

>Signed-off-by: Przemyslaw Zegan <przemyslawx.zegan@intel.com>
>---
> app/test-crypto-perf/cperf_test_vectors.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
>diff --git a/app/test-crypto-perf/cperf_test_vectors.c b/app/test-
>crypto-perf/cperf_test_vectors.c
>index 0af01ff911..2c7e314ec8 100644
>--- a/app/test-crypto-perf/cperf_test_vectors.c
>+++ b/app/test-crypto-perf/cperf_test_vectors.c
>@@ -548,12 +548,16 @@ cperf_test_vector_get_dummy(struct
>cperf_options *options)
> 		t_vec->aead_key.data = aead_key;
>
> 		if (options->aead_aad_sz) {
>-			t_vec->aad.data = rte_malloc(NULL,
>+			t_vec->aad.data = rte_zmalloc(NULL,
> 					options->aead_aad_sz, 16);
> 			if (t_vec->aad.data == NULL) {
> 				rte_free(t_vec);
> 				return NULL;
> 			}
>+
>+			if(options->aead_aad_sz > 12)
>+				options->aead_aad_sz = 12;
>+
> 			memcpy(t_vec->aad.data, aad, options-
>>aead_aad_sz);
> 			t_vec->aad.phys_addr =
>rte_malloc_virt2iova(t_vec->aad.data);
> 			t_vec->aad.length = options->aead_aad_sz;
>--
>2.17.1
>
>--------------------------------------------------------------
>Intel Research and Development Ireland Limited
>Registered in Ireland
>Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
>Registered Number: 308263
>
>
>This e-mail and any attachments may contain confidential material for
>the sole
>use of the intended recipient(s). Any review or distribution by others is
>strictly prohibited. If you are not the intended recipient, please contact
>the
>sender and delete all copies.

[dpdk-dev] [dpdk-dev v2] app: fix buffer overrun

[Przemyslaw Zegan]

This patch fixes a possible buffer overrun problem in crypto perf test.
Previously when user configured aad size is over 12 bytes the copy of template aad will cause a buffer overrun.
The problem is fixed by only copy up to 12 bytes of aad template.

Fixes: 8a5b494a7f99 ("app/test-crypto-perf: add AEAD parameters")
Cc: pablo.de.lara.guarch@intel.com

Signed-off-by: Przemyslaw Zegan <przemyslawx.zegan@intel.com>
---
v2:
- changed to correct fixed line.

 app/test-crypto-perf/cperf_test_vectors.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/app/test-crypto-perf/cperf_test_vectors.c b/app/test-crypto-perf/cperf_test_vectors.c
index 0af01ff911..2c7e314ec8 100644
--- a/app/test-crypto-perf/cperf_test_vectors.c
+++ b/app/test-crypto-perf/cperf_test_vectors.c
@@ -548,12 +548,16 @@ cperf_test_vector_get_dummy(struct cperf_options *options)
 		t_vec->aead_key.data = aead_key;
 
 		if (options->aead_aad_sz) {
-			t_vec->aad.data = rte_malloc(NULL,
+			t_vec->aad.data = rte_zmalloc(NULL,
 					options->aead_aad_sz, 16);
 			if (t_vec->aad.data == NULL) {
 				rte_free(t_vec);
 				return NULL;
 			}
+
+			if(options->aead_aad_sz > 12)
+				options->aead_aad_sz = 12;
+
 			memcpy(t_vec->aad.data, aad, options->aead_aad_sz);
 			t_vec->aad.phys_addr = rte_malloc_virt2iova(t_vec->aad.data);
 			t_vec->aad.length = options->aead_aad_sz;
-- 
2.17.1

--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.

Re: [dpdk-dev] [EXT] [dpdk-dev v2] app: fix buffer overrun

[Akhil Goyal]

> This patch fixes a possible buffer overrun problem in crypto perf test.
> Previously when user configured aad size is over 12 bytes the copy of
> template aad will cause a buffer overrun.
> The problem is fixed by only copy up to 12 bytes of aad template.
> 
> Fixes: 8a5b494a7f99 ("app/test-crypto-perf: add AEAD parameters")
> Cc: pablo.de.lara.guarch@intel.com
> 
> Signed-off-by: Przemyslaw Zegan <przemyslawx.zegan@intel.com>
> ---
> v2:
> - changed to correct fixed line.
> 
>  app/test-crypto-perf/cperf_test_vectors.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test-crypto-perf/cperf_test_vectors.c b/app/test-crypto-
> perf/cperf_test_vectors.c
> index 0af01ff911..2c7e314ec8 100644
> --- a/app/test-crypto-perf/cperf_test_vectors.c
> +++ b/app/test-crypto-perf/cperf_test_vectors.c
> @@ -548,12 +548,16 @@ cperf_test_vector_get_dummy(struct
> cperf_options *options)
>  		t_vec->aead_key.data = aead_key;
> 
>  		if (options->aead_aad_sz) {
> -			t_vec->aad.data = rte_malloc(NULL,
> +			t_vec->aad.data = rte_zmalloc(NULL,
>  					options->aead_aad_sz, 16);
>  			if (t_vec->aad.data == NULL) {
>  				rte_free(t_vec);
>  				return NULL;
>  			}
> +
> +			if(options->aead_aad_sz > 12)
> +				options->aead_aad_sz = 12;

Instead of hardcoding, shouldn't this be sizeof(aad)

> +
>  			memcpy(t_vec->aad.data, aad, options-
> >aead_aad_sz);
>  			t_vec->aad.phys_addr = rte_malloc_virt2iova(t_vec-
> >aad.data);
>  			t_vec->aad.length = options->aead_aad_sz;
> --
> 2.17.1
> 
> --------------------------------------------------------------
> Intel Research and Development Ireland Limited
> Registered in Ireland
> Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
> Registered Number: 308263
> 
> 
> This e-mail and any attachments may contain confidential material for the
> sole
> use of the intended recipient(s). Any review or distribution by others is
> strictly prohibited. If you are not the intended recipient, please contact the
> sender and delete all copies.

[dpdk-dev] [dpdk-dev v3] app: fix buffer overrun

[Przemyslaw Zegan]

This patch fixes a possible buffer overrun problem in crypto perf test.
Previously when user configured aad size is over 12 bytes the copy of template aad will cause a buffer overrun.
The problem is fixed by only copy up to 12 bytes of aad template.

Fixes: 8a5b494a7f99 ("app/test-crypto-perf: add AEAD parameters")
Cc: pablo.de.lara.guarch@intel.com

Signed-off-by: Przemyslaw Zegan <przemyslawx.zegan@intel.com>
---
v3:
- replaced hardcoded values by sizeof(aad)

 app/test-crypto-perf/cperf_test_vectors.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/test-crypto-perf/cperf_test_vectors.c b/app/test-crypto-perf/cperf_test_vectors.c
index 2c7e314ec8..271e91c13c 100644
--- a/app/test-crypto-perf/cperf_test_vectors.c
+++ b/app/test-crypto-perf/cperf_test_vectors.c
@@ -555,8 +555,8 @@ cperf_test_vector_get_dummy(struct cperf_options *options)
 				return NULL;
 			}
 
-			if(options->aead_aad_sz > 12)
-				options->aead_aad_sz = 12;
+			if(options->aead_aad_sz > sizeof(aad))
+				options->aead_aad_sz = sizeof(aad);
 
 			memcpy(t_vec->aad.data, aad, options->aead_aad_sz);
 			t_vec->aad.phys_addr = rte_malloc_virt2iova(t_vec->aad.data);
-- 
2.30.2

Re: [dpdk-dev] [dpdk-dev v3] app: fix buffer overrun

[Zhang, Roy Fan]

> -----Original Message-----
> From: Zegan, PrzemyslawX <przemyslawx.zegan@intel.com>
> Sent: Monday, October 11, 2021 9:05 AM
> To: dev@dpdk.org
> Cc: gakhil@marvell.com; Zhang, Roy Fan <roy.fan.zhang@intel.com>; Zegan,
> PrzemyslawX <przemyslawx.zegan@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>
> Subject: [dpdk-dev v3] app: fix buffer overrun
> 
> This patch fixes a possible buffer overrun problem in crypto perf test.
> Previously when user configured aad size is over 12 bytes the copy of
> template aad will cause a buffer overrun.
> The problem is fixed by only copy up to 12 bytes of aad template.
> 
> Fixes: 8a5b494a7f99 ("app/test-crypto-perf: add AEAD parameters")
> Cc: pablo.de.lara.guarch@intel.com
> 
> Signed-off-by: Przemyslaw Zegan <przemyslawx.zegan@intel.com>
> ---
Acked-by: Fan Zhang <roy.fan.zhang@intel.com>

[dpdk-dev] [dpdk-dev v4] app: fix buffer overrun

[Przemyslaw Zegan]

This patch fixes a possible buffer overrun problem in crypto perf test.
Previously when user configured aad size is over 12 bytes the copy of template aad will cause a buffer overrun.
The problem is fixed by only copy up to 12 bytes of aad template.

Fixes: 8a5b494a7f99 ("app/test-crypto-perf: add AEAD parameters")
Cc: pablo.de.lara.guarch@intel.com

Signed-off-by: Przemyslaw Zegan <przemyslawx.zegan@intel.com>
Acked-by: Fan Zhang <roy.fan.zhang@intel.com>
---
v4:
- rebased on top of latest master
v3:
- replaced hardcoded values by sizeof(aad)
v2:
- changed to correct fixed line.

 app/test-crypto-perf/cperf_test_vectors.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/app/test-crypto-perf/cperf_test_vectors.c b/app/test-crypto-perf/cperf_test_vectors.c
index 4bba405961..314e2b7710 100644
--- a/app/test-crypto-perf/cperf_test_vectors.c
+++ b/app/test-crypto-perf/cperf_test_vectors.c
@@ -590,6 +590,10 @@ cperf_test_vector_get_dummy(struct cperf_options *options)
 				rte_free(t_vec);
 				return NULL;
 			}
+
+			if(options->aead_aad_sz > sizeof(aad))
+				options->aead_aad_sz = sizeof(aad);
+
 			memcpy(t_vec->aad.data, aad, options->aead_aad_sz);
 			t_vec->aad.phys_addr = rte_malloc_virt2iova(t_vec->aad.data);
 			t_vec->aad.length = options->aead_aad_sz;
-- 
2.30.2

Re: [dpdk-dev] [EXT] [dpdk-dev v4] app: fix buffer overrun

[Akhil Goyal]

> This patch fixes a possible buffer overrun problem in crypto perf test.
> Previously when user configured aad size is over 12 bytes the copy of
> template aad will cause a buffer overrun.
> The problem is fixed by only copy up to 12 bytes of aad template.
> 
> Fixes: 8a5b494a7f99 ("app/test-crypto-perf: add AEAD parameters")
> Cc: pablo.de.lara.guarch@intel.com
> 
> Signed-off-by: Przemyslaw Zegan <przemyslawx.zegan@intel.com>
> Acked-by: Fan Zhang <roy.fan.zhang@intel.com>
Applied to dpdk-next-crypto

Re: [dpdk-dev] [EXT] [dpdk-dev v4] app: fix buffer overrun

[Akhil Goyal]

> > This patch fixes a possible buffer overrun problem in crypto perf test.
> > Previously when user configured aad size is over 12 bytes the copy of
> > template aad will cause a buffer overrun.
> > The problem is fixed by only copy up to 12 bytes of aad template.
> >
> > Fixes: 8a5b494a7f99 ("app/test-crypto-perf: add AEAD parameters")
> > Cc: pablo.de.lara.guarch@intel.com
> >
> > Signed-off-by: Przemyslaw Zegan <przemyslawx.zegan@intel.com>
> > Acked-by: Fan Zhang <roy.fan.zhang@intel.com>
> Applied to dpdk-next-crypto
Cc: stable@dpdk.org