[PATCH] util/iov: Avoid dynamic stack allocation

[PATCH] util/iov: Avoid dynamic stack allocation

[Peter Maydell]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Use autofree heap allocation instead of variable-length array on the
stack.

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: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Usual "only tested with make check/make check-avocado" caveat.

 util/iov.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/iov.c b/util/iov.c
index 866fb577f30..7e73948f5e3 100644
--- a/util/iov.c
+++ b/util/iov.c
@@ -571,7 +571,7 @@ static int sortelem_cmp_src_index(const void *a, const void *b)
  */
 void qemu_iovec_clone(QEMUIOVector *dest, const QEMUIOVector *src, void *buf)
 {
-    IOVectorSortElem sortelems[src->niov];
+    g_autofree IOVectorSortElem *sortelems = g_new(IOVectorSortElem, src->niov);
     void *last_end;
     int i;
 
-- 
2.34.1

Re: [PATCH] util/iov: Avoid dynamic stack allocation

[Eric Blake]

On Thu, Aug 24, 2023 at 05:47:06PM +0100, Peter Maydell wrote:
> From: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> Use autofree heap allocation instead of variable-length array on the
> stack.
> 
> 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: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Usual "only tested with make check/make check-avocado" caveat.
> 
>  util/iov.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

> 
> diff --git a/util/iov.c b/util/iov.c
> index 866fb577f30..7e73948f5e3 100644
> --- a/util/iov.c
> +++ b/util/iov.c
> @@ -571,7 +571,7 @@ static int sortelem_cmp_src_index(const void *a, const void *b)
>   */
>  void qemu_iovec_clone(QEMUIOVector *dest, const QEMUIOVector *src, void *buf)
>  {
> -    IOVectorSortElem sortelems[src->niov];
> +    g_autofree IOVectorSortElem *sortelems = g_new(IOVectorSortElem, src->niov);
>      void *last_end;
>      int i;
>  
> -- 
> 2.34.1
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

Re: [PATCH] util/iov: Avoid dynamic stack allocation

[Philippe Mathieu-Daudé]

Cc'ing qemu-block@ (I suppose this will go via a block tree)

On 24/8/23 18:47, Peter Maydell wrote:
> From: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> Use autofree heap allocation instead of variable-length array on the
> stack.
> 
> 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: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Usual "only tested with make check/make check-avocado" caveat.
> 
>   util/iov.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/util/iov.c b/util/iov.c
> index 866fb577f30..7e73948f5e3 100644
> --- a/util/iov.c
> +++ b/util/iov.c
> @@ -571,7 +571,7 @@ static int sortelem_cmp_src_index(const void *a, const void *b)
>    */
>   void qemu_iovec_clone(QEMUIOVector *dest, const QEMUIOVector *src, void *buf)
>   {
> -    IOVectorSortElem sortelems[src->niov];
> +    g_autofree IOVectorSortElem *sortelems = g_new(IOVectorSortElem, src->niov);
>       void *last_end;
>       int i;
>   

Re: [PATCH] util/iov: Avoid dynamic stack allocation

[Eric Blake]

On Thu, Aug 31, 2023 at 10:26:17AM +0200, Philippe Mathieu-Daudé wrote:
> Cc'ing qemu-block@ (I suppose this will go via a block tree)

Makes sense.

I can queue it through my NBD tree if no one else beats me (since the
nbd code is a heavy user of iovs).

> 
> On 24/8/23 18:47, Peter Maydell wrote:
> > From: Philippe Mathieu-Daudé <philmd@redhat.com>
> > 
> > Use autofree heap allocation instead of variable-length array on the
> > stack.
> > 
> > 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).
> > 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org