Re: [PATCH] block/vpc: Avoid dynamic stack allocation

[Francisco Iglesias]

On [2023 Aug 11] Fri 18:52:29, Peter Maydell wrote:
> From: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> Use autofree heap allocation instead of variable-length array on the
> stack. Here we don't expect the bitmap size to be enormous, and
> since we're about to read/write it to disk the overhead of the
> allocation should be fine.
> 
> 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>
> [PMM: expanded commit message]
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com>

> ---
>  block/vpc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/block/vpc.c b/block/vpc.c
> index 3810a601a38..ceb87dd3d8e 100644
> --- a/block/vpc.c
> +++ b/block/vpc.c
> @@ -510,7 +510,7 @@ get_image_offset(BlockDriverState *bs, uint64_t offset, 
> bool write, int *err)
>         miss sparse read optimization, but it's not a problem in terms of
>         correctness. */
>      if (write && (s->last_bitmap_offset != bitmap_offset)) {
> -        uint8_t bitmap[s->bitmap_size];
> +        g_autofree uint8_t *bitmap = g_malloc(s->bitmap_size);
>          int r;
>  
>          s->last_bitmap_offset = bitmap_offset;
> @@ -558,7 +558,7 @@ alloc_block(BlockDriverState *bs, int64_t offset)
>      int64_t bat_offset;
>      uint32_t index, bat_value;
>      int ret;
> -    uint8_t bitmap[s->bitmap_size];
> +    g_autofree uint8_t *bitmap = g_malloc(s->bitmap_size);
>  
>      /* Check if sector_num is valid */
>      if ((offset < 0) || (offset > bs->total_sectors * BDRV_SECTOR_SIZE)) {
> -- 
> 2.34.1
> 
>