| Message ID | 20230818151057.1541189-2-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ui: avoid dynamic stack allocations | expand |
On 18/8/23 17:10, Peter Maydell wrote: > Use an autofree heap allocation instead of a variable-length > array on the stack in qemu_spice_create_update(). > > 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: Peter Maydell <peter.maydell@linaro.org> > --- > I was a little unsure about this allocation given that it's > in the display_refresh callback, but the code already does > a g_malloc() every time it calls qemu_spice_create_one_update() > so one more presumably won't hurt. > --- > ui/spice-display.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
diff --git a/ui/spice-display.c b/ui/spice-display.c index 3f3f8013d86..0e2fbfb17c1 100644 --- a/ui/spice-display.c +++ b/ui/spice-display.c @@ -189,7 +189,7 @@ static void qemu_spice_create_update(SimpleSpiceDisplay *ssd) { static const int blksize = 32; int blocks = DIV_ROUND_UP(surface_width(ssd->ds), blksize); - int dirty_top[blocks]; + g_autofree int *dirty_top = NULL; int y, yoff1, yoff2, x, xoff, blk, bw; int bpp = surface_bytes_per_pixel(ssd->ds); uint8_t *guest, *mirror; @@ -198,6 +198,7 @@ static void qemu_spice_create_update(SimpleSpiceDisplay *ssd) return; }; + dirty_top = g_new(int, blocks); for (blk = 0; blk < blocks; blk++) { dirty_top[blk] = -1; }
Use an autofree heap allocation instead of a variable-length array on the stack in qemu_spice_create_update(). 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: Peter Maydell <peter.maydell@linaro.org> --- I was a little unsure about this allocation given that it's in the display_refresh callback, but the code already does a g_malloc() every time it calls qemu_spice_create_one_update() so one more presumably won't hurt. --- ui/spice-display.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)