[PATCH 1/2] tests/qtest/xlnx-versal-trng-test.c: Drop use of variable length array

Peter Maydell posted 2 patches 10 months ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Thomas Huth <thuth@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Laurent Vivier <lvivier@redhat.com>
[PATCH 1/2] tests/qtest/xlnx-versal-trng-test.c: Drop use of variable length array
Posted by Peter Maydell 10 months ago
This test program is the last use of any variable length array in the
codebase.  If we can get rid of all uses of VLAs we can make the
compiler error on new additions.  This is a defensive measure against
security bugs where an on-stack dynamic allocation isn't correctly
size-checked (e.g.  CVE-2021-3527).

In this case the test code didn't even want a variable-sized
array, it was just accidentally using syntax that gave it one.
(The array size for C has to be an actual constant expression,
not just something that happens to be known to be constant...)

Remove the VLA usage.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 tests/qtest/xlnx-versal-trng-test.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/tests/qtest/xlnx-versal-trng-test.c b/tests/qtest/xlnx-versal-trng-test.c
index cef4e575bba..ba86f39d13c 100644
--- a/tests/qtest/xlnx-versal-trng-test.c
+++ b/tests/qtest/xlnx-versal-trng-test.c
@@ -298,10 +298,13 @@ static size_t trng_collect(uint32_t *rnd, size_t cnt)
     return i;
 }
 
+/* These tests all generate 512 bits of random data with the device */
+#define TEST_DATA_WORDS (512 / 32)
+
 static void trng_test_autogen(void)
 {
-    const size_t cnt = 512 / 32;
-    uint32_t rng[cnt], prng[cnt];
+    const size_t cnt = TEST_DATA_WORDS;
+    uint32_t rng[TEST_DATA_WORDS], prng[TEST_DATA_WORDS];
     size_t n;
 
     trng_reset();
@@ -343,8 +346,8 @@ static void trng_test_autogen(void)
 
 static void trng_test_oneshot(void)
 {
-    const size_t cnt = 512 / 32;
-    uint32_t rng[cnt];
+    const size_t cnt = TEST_DATA_WORDS;
+    uint32_t rng[TEST_DATA_WORDS];
     size_t n;
 
     trng_reset();
@@ -370,8 +373,8 @@ static void trng_test_oneshot(void)
 
 static void trng_test_per_str(void)
 {
-    const size_t cnt = 512 / 32;
-    uint32_t rng[cnt], prng[cnt];
+    const size_t cnt = TEST_DATA_WORDS;
+    uint32_t rng[TEST_DATA_WORDS], prng[TEST_DATA_WORDS];
     size_t n;
 
     trng_reset();
@@ -415,8 +418,8 @@ static void trng_test_forced_prng(void)
     const char *prop = "forced-prng";
     const uint64_t seed = 0xdeadbeefbad1bad0ULL;
 
-    const size_t cnt = 512 / 32;
-    uint32_t rng[cnt], prng[cnt];
+    const size_t cnt = TEST_DATA_WORDS;
+    uint32_t rng[TEST_DATA_WORDS], prng[TEST_DATA_WORDS];
     size_t n;
 
     trng_reset();
-- 
2.34.1
Re: [PATCH 1/2] tests/qtest/xlnx-versal-trng-test.c: Drop use of variable length array
Posted by Zhao Liu 10 months ago
On Thu, Jan 25, 2024 at 05:32:10PM +0000, Peter Maydell wrote:
> Date: Thu, 25 Jan 2024 17:32:10 +0000
> From: Peter Maydell <peter.maydell@linaro.org>
> Subject: [PATCH 1/2] tests/qtest/xlnx-versal-trng-test.c: Drop use of
>  variable length array
> X-Mailer: git-send-email 2.34.1
> 
> This test program is the last use of any variable length array in the
> codebase.  If we can get rid of all uses of VLAs we can make the
> compiler error on new additions.  This is a defensive measure against
> security bugs where an on-stack dynamic allocation isn't correctly
> size-checked (e.g.  CVE-2021-3527).
> 
> In this case the test code didn't even want a variable-sized
> array, it was just accidentally using syntax that gave it one.
> (The array size for C has to be an actual constant expression,
> not just something that happens to be known to be constant...)
> 
> Remove the VLA usage.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  tests/qtest/xlnx-versal-trng-test.c | 19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

> 
> diff --git a/tests/qtest/xlnx-versal-trng-test.c b/tests/qtest/xlnx-versal-trng-test.c
> index cef4e575bba..ba86f39d13c 100644
> --- a/tests/qtest/xlnx-versal-trng-test.c
> +++ b/tests/qtest/xlnx-versal-trng-test.c
> @@ -298,10 +298,13 @@ static size_t trng_collect(uint32_t *rnd, size_t cnt)
>      return i;
>  }
>  
> +/* These tests all generate 512 bits of random data with the device */
> +#define TEST_DATA_WORDS (512 / 32)
> +
>  static void trng_test_autogen(void)
>  {
> -    const size_t cnt = 512 / 32;
> -    uint32_t rng[cnt], prng[cnt];
> +    const size_t cnt = TEST_DATA_WORDS;
> +    uint32_t rng[TEST_DATA_WORDS], prng[TEST_DATA_WORDS];
>      size_t n;
>  
>      trng_reset();
> @@ -343,8 +346,8 @@ static void trng_test_autogen(void)
>  
>  static void trng_test_oneshot(void)
>  {
> -    const size_t cnt = 512 / 32;
> -    uint32_t rng[cnt];
> +    const size_t cnt = TEST_DATA_WORDS;
> +    uint32_t rng[TEST_DATA_WORDS];
>      size_t n;
>  
>      trng_reset();
> @@ -370,8 +373,8 @@ static void trng_test_oneshot(void)
>  
>  static void trng_test_per_str(void)
>  {
> -    const size_t cnt = 512 / 32;
> -    uint32_t rng[cnt], prng[cnt];
> +    const size_t cnt = TEST_DATA_WORDS;
> +    uint32_t rng[TEST_DATA_WORDS], prng[TEST_DATA_WORDS];
>      size_t n;
>  
>      trng_reset();
> @@ -415,8 +418,8 @@ static void trng_test_forced_prng(void)
>      const char *prop = "forced-prng";
>      const uint64_t seed = 0xdeadbeefbad1bad0ULL;
>  
> -    const size_t cnt = 512 / 32;
> -    uint32_t rng[cnt], prng[cnt];
> +    const size_t cnt = TEST_DATA_WORDS;
> +    uint32_t rng[TEST_DATA_WORDS], prng[TEST_DATA_WORDS];
>      size_t n;
>  
>      trng_reset();
> -- 
> 2.34.1
> 
>
Re: [PATCH 1/2] tests/qtest/xlnx-versal-trng-test.c: Drop use of variable length array
Posted by Thomas Huth 10 months ago
On 25/01/2024 18.32, Peter Maydell wrote:
> This test program is the last use of any variable length array in the
> codebase.  If we can get rid of all uses of VLAs we can make the
> compiler error on new additions.  This is a defensive measure against
> security bugs where an on-stack dynamic allocation isn't correctly
> size-checked (e.g.  CVE-2021-3527).
> 
> In this case the test code didn't even want a variable-sized
> array, it was just accidentally using syntax that gave it one.
> (The array size for C has to be an actual constant expression,
> not just something that happens to be known to be constant...)
> 
> Remove the VLA usage.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   tests/qtest/xlnx-versal-trng-test.c | 19 +++++++++++--------
>   1 file changed, 11 insertions(+), 8 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>