diff mbox series

util/iov: Avoid dynamic stack allocation

Message ID 20230824164706.2652277-1-peter.maydell@linaro.org
State New
Headers show
Series util/iov: Avoid dynamic stack allocation | expand

Commit Message

Peter Maydell Aug. 24, 2023, 4:47 p.m. UTC
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(-)

Comments

Eric Blake Aug. 24, 2023, 7:36 p.m. UTC | #1
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
>
Philippe Mathieu-Daudé Aug. 31, 2023, 8:26 a.m. UTC | #2
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;
>
Eric Blake Aug. 31, 2023, 1:34 p.m. UTC | #3
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).
> >
diff mbox series

Patch

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;