* [PATCH 22.11 21.11] test/crypto: fix vector global buffer overflow
@ 2024-05-07 13:40 Ciara Power
2024-05-07 14:25 ` Kevin Traynor
0 siblings, 1 reply; 2+ messages in thread
From: Ciara Power @ 2024-05-07 13:40 UTC (permalink / raw)
To: stable; +Cc: brian.dooley, arkadiuszx.kusztal, Ciara Power
When doing a memcpy of the test vector into a local union variable,
the size of the union was used. This meant extra bytes were being copied
from the test vector address in the case the vector was smaller in size
than the union. This caused a global buffer overflow error detected by
Address Sanitizer.
To fix this, the size of the test vector is also stored alongside the
address, so when copying takes place, the minimum of the union and test
vector can be used as the size reference.
Fixes: 488f5a23c219 ("test/crypto: check asymmetric crypto")
Signed-off-by: Ciara Power <ciara.power@intel.com>
---
This issue was fixed by a rework in 2023, so this fix is only applicable
to 21.11 and 22.11 LTS releases that are currently maintained.
It is not applicable to 23.11 LTS, or current upstream releases.
---
app/test/test_cryptodev_asym.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/app/test/test_cryptodev_asym.c b/app/test/test_cryptodev_asym.c
index 67659cd1a6..b3345c0a39 100644
--- a/app/test/test_cryptodev_asym.c
+++ b/app/test/test_cryptodev_asym.c
@@ -54,11 +54,15 @@ union test_case_structure {
struct rsa_test_data_2 rsa_data;
};
+struct vector_details {
+ uint32_t vector_size;
+ const void *address;
+};
struct test_cases_array {
uint32_t size;
- const void *address[TEST_VECTOR_SIZE];
+ struct vector_details details[TEST_VECTOR_SIZE];
};
-static struct test_cases_array test_vector = {0, { NULL } };
+static struct test_cases_array test_vector = {0, {} };
static uint32_t test_index;
@@ -513,14 +517,14 @@ test_cryptodev_asym_op(struct crypto_testsuite_params_asym *ts_params,
}
static int
-test_one_case(const void *test_case, int sessionless)
+test_one_case(struct vector_details test_case, int sessionless)
{
int status = TEST_SUCCESS, i = 0;
char test_msg[ASYM_TEST_MSG_LEN + 1];
/* Map the case to union */
union test_case_structure tc;
- memcpy(&tc, test_case, sizeof(tc));
+ rte_memcpy(&tc, test_case.address, RTE_MIN(sizeof(tc), test_case.vector_size));
if (tc.modex.xform_type == RTE_CRYPTO_ASYM_XFORM_MODEX
|| tc.modex.xform_type == RTE_CRYPTO_ASYM_XFORM_MODINV) {
@@ -572,7 +576,8 @@ load_test_vectors(void)
"TEST_VECTOR_SIZE too small\n");
return -1;
}
- test_vector.address[test_vector.size] = &modex_test_case[i];
+ test_vector.details[test_vector.size].address = &modex_test_case[i];
+ test_vector.details[test_vector.size].vector_size = sizeof(modex_test_case[i]);
test_vector.size++;
}
/* Load MODINV vector*/
@@ -583,7 +588,8 @@ load_test_vectors(void)
"TEST_VECTOR_SIZE too small\n");
return -1;
}
- test_vector.address[test_vector.size] = &modinv_test_case[i];
+ test_vector.details[test_vector.size].address = &modinv_test_case[i];
+ test_vector.details[test_vector.size].vector_size = sizeof(modinv_test_case[i]);
test_vector.size++;
}
/* Load RSA vector*/
@@ -594,7 +600,8 @@ load_test_vectors(void)
"TEST_VECTOR_SIZE too small\n");
return -1;
}
- test_vector.address[test_vector.size] = &rsa_test_case_list[i];
+ test_vector.details[test_vector.size].address = &rsa_test_case_list[i];
+ test_vector.details[test_vector.size].vector_size = sizeof(rsa_test_case_list[i]);
test_vector.size++;
}
return 0;
@@ -619,12 +626,12 @@ test_one_by_one(void)
/* Go through all test cases */
test_index = 0;
for (i = 0; i < test_vector.size; i++) {
- if (test_one_case(test_vector.address[i], 0) != TEST_SUCCESS)
+ if (test_one_case(test_vector.details[i], 0) != TEST_SUCCESS)
status = TEST_FAILED;
}
if (sessionless) {
for (i = 0; i < test_vector.size; i++) {
- if (test_one_case(test_vector.address[i], 1)
+ if (test_one_case(test_vector.details[i], 1)
!= TEST_SUCCESS)
status = TEST_FAILED;
}
--
2.25.1
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH 22.11 21.11] test/crypto: fix vector global buffer overflow
2024-05-07 13:40 [PATCH 22.11 21.11] test/crypto: fix vector global buffer overflow Ciara Power
@ 2024-05-07 14:25 ` Kevin Traynor
0 siblings, 0 replies; 2+ messages in thread
From: Kevin Traynor @ 2024-05-07 14:25 UTC (permalink / raw)
To: Ciara Power, stable; +Cc: brian.dooley, arkadiuszx.kusztal
On 07/05/2024 14:40, Ciara Power wrote:
> When doing a memcpy of the test vector into a local union variable,
> the size of the union was used. This meant extra bytes were being copied
> from the test vector address in the case the vector was smaller in size
> than the union. This caused a global buffer overflow error detected by
> Address Sanitizer.
>
> To fix this, the size of the test vector is also stored alongside the
> address, so when copying takes place, the minimum of the union and test
> vector can be used as the size reference.
>
> Fixes: 488f5a23c219 ("test/crypto: check asymmetric crypto")
>
> Signed-off-by: Ciara Power <ciara.power@intel.com>
> ---
> This issue was fixed by a rework in 2023, so this fix is only applicable
> to 21.11 and 22.11 LTS releases that are currently maintained.
> It is not applicable to 23.11 LTS, or current upstream releases.
> ---
> app/test/test_cryptodev_asym.c | 25 ++++++++++++++++---------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
Thanks Ciara, will apply to the tree and it will be part of the next
release.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-05-07 14:25 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-07 13:40 [PATCH 22.11 21.11] test/crypto: fix vector global buffer overflow Ciara Power
2024-05-07 14:25 ` Kevin Traynor
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).