[PATCH] tests/qtest/netdev-socket: Avoid variable-length array in inet_get_free_port_multiple()

Peter Maydell posted 1 patch 1 year, 3 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230824164535.2652070-1-peter.maydell@linaro.org
Maintainers: Thomas Huth <thuth@redhat.com>, Laurent Vivier <lvivier@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
tests/qtest/netdev-socket.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] tests/qtest/netdev-socket: Avoid variable-length array in inet_get_free_port_multiple()
Posted by Peter Maydell 1 year, 3 months ago
We use a variable-length array in inet_get_free_port_multiple().
This is only test code called at the start of a test, so switch to a
heap allocation instead.

The codebase has very few VLAs, and if we can get rid of them all 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).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 tests/qtest/netdev-socket.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qtest/netdev-socket.c b/tests/qtest/netdev-socket.c
index 097abc0230b..8eed54801f2 100644
--- a/tests/qtest/netdev-socket.c
+++ b/tests/qtest/netdev-socket.c
@@ -82,7 +82,7 @@ static int inet_get_free_port_socket_ipv6(int sock)
 
 static int inet_get_free_port_multiple(int nb, int *port, bool ipv6)
 {
-    int sock[nb];
+    g_autofree int *sock = g_new(int, nb);
     int i;
 
     for (i = 0; i < nb; i++) {
-- 
2.34.1
Re: [PATCH] tests/qtest/netdev-socket: Avoid variable-length array in inet_get_free_port_multiple()
Posted by Laurent Vivier 1 year, 3 months ago
On 8/24/23 18:45, Peter Maydell wrote:
> We use a variable-length array in inet_get_free_port_multiple().
> This is only test code called at the start of a test, so switch to a
> heap allocation instead.
> 
> The codebase has very few VLAs, and if we can get rid of them all 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).
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   tests/qtest/netdev-socket.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/netdev-socket.c b/tests/qtest/netdev-socket.c
> index 097abc0230b..8eed54801f2 100644
> --- a/tests/qtest/netdev-socket.c
> +++ b/tests/qtest/netdev-socket.c
> @@ -82,7 +82,7 @@ static int inet_get_free_port_socket_ipv6(int sock)
>   
>   static int inet_get_free_port_multiple(int nb, int *port, bool ipv6)
>   {
> -    int sock[nb];
> +    g_autofree int *sock = g_new(int, nb);
>       int i;
>   
>       for (i = 0; i < nb; i++) {

Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Re: [PATCH] tests/qtest/netdev-socket: Avoid variable-length array in inet_get_free_port_multiple()
Posted by Philippe Mathieu-Daudé 1 year, 3 months ago
On 24/8/23 18:45, Peter Maydell wrote:
> We use a variable-length array in inet_get_free_port_multiple().
> This is only test code called at the start of a test, so switch to a
> heap allocation instead.
> 
> The codebase has very few VLAs, and if we can get rid of them all 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).
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   tests/qtest/netdev-socket.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>


Re: [PATCH] tests/qtest/netdev-socket: Avoid variable-length array in inet_get_free_port_multiple()
Posted by Thomas Huth 1 year, 3 months ago
On 24/08/2023 18.45, Peter Maydell wrote:
> We use a variable-length array in inet_get_free_port_multiple().
> This is only test code called at the start of a test, so switch to a
> heap allocation instead.
> 
> The codebase has very few VLAs, and if we can get rid of them all 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).
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   tests/qtest/netdev-socket.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/netdev-socket.c b/tests/qtest/netdev-socket.c
> index 097abc0230b..8eed54801f2 100644
> --- a/tests/qtest/netdev-socket.c
> +++ b/tests/qtest/netdev-socket.c
> @@ -82,7 +82,7 @@ static int inet_get_free_port_socket_ipv6(int sock)
>   
>   static int inet_get_free_port_multiple(int nb, int *port, bool ipv6)
>   {
> -    int sock[nb];
> +    g_autofree int *sock = g_new(int, nb);
>       int i;
>   
>       for (i = 0; i < nb; i++) {

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