[PATCH] util/iov: Avoid dynamic stack allocation

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/20230824164706.2652277-1-peter.maydell@linaro.org
util/iov.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] util/iov: Avoid dynamic stack allocation
Posted by Peter Maydell 1 year, 3 months ago
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
Posted by Philippe Mathieu-Daudé 1 year, 2 months ago
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
Posted by Eric Blake 1 year, 2 months ago
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


Re: [PATCH] util/iov: Avoid dynamic stack allocation
Posted by Eric Blake 1 year, 3 months ago
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