| Message ID | 20230811175229.808139-1-peter.maydell@linaro.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | block/vpc: Avoid dynamic stack allocation | expand |
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 > >
Am 11.08.2023 um 19:52 hat Peter Maydell geschrieben: > 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> Thanks, applied to the block branch. Kevin
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)) {