[PATCH] hw/display/exynos4210_fimd: Account for zero length in fimd_update_memory_section()

[PATCH] hw/display/exynos4210_fimd: Account for zero length in fimd_update_memory_section()

[Peter Maydell]

In fimd_update_memory_section() we attempt ot find and map part of
the RAM MR which backs the framebuffer, based on guest-configurable
size and start address.

If the guest configures framebuffer settings which result in a
zero-sized framebuffer, we hit an assertion(), because
memory_region_find() will return a NULL mem_section.mr.

Explicitly check for the zero-size case and treat this as a
guest error.

Because we now have a code path which can reach error_return without
calling memory_region_find to set w->mem_section, we must NULL out
w->mem_section.mr after the unref of the old MR, so that error_return
does not incorrectly double-unref the old MR.

Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1407
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/display/exynos4210_fimd.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/display/exynos4210_fimd.c b/hw/display/exynos4210_fimd.c
index c61e0280a7c..eec874d0b1d 100644
--- a/hw/display/exynos4210_fimd.c
+++ b/hw/display/exynos4210_fimd.c
@@ -1147,6 +1147,13 @@ static void fimd_update_memory_section(Exynos4210fimdState *s, unsigned win)
     if (w->mem_section.mr) {
         memory_region_set_log(w->mem_section.mr, false, DIRTY_MEMORY_VGA);
         memory_region_unref(w->mem_section.mr);
+        w->mem_section.mr = NULL;
+    }
+
+    if (w->fb_len == 0) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "FIMD: Guest config means framebuffer is zero length\n");
+        goto error_return;
     }
 
     w->mem_section = memory_region_find(s->fbmem, fb_start_addr, w->fb_len);
-- 
2.43.0

Re: [PATCH] hw/display/exynos4210_fimd: Account for zero length in fimd_update_memory_section()

[Peter Maydell]

Ping for review?

thanks
-- PMM

On Fri, 7 Nov 2025 at 14:39, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> In fimd_update_memory_section() we attempt ot find and map part of
> the RAM MR which backs the framebuffer, based on guest-configurable
> size and start address.
>
> If the guest configures framebuffer settings which result in a
> zero-sized framebuffer, we hit an assertion(), because
> memory_region_find() will return a NULL mem_section.mr.
>
> Explicitly check for the zero-size case and treat this as a
> guest error.
>
> Because we now have a code path which can reach error_return without
> calling memory_region_find to set w->mem_section, we must NULL out
> w->mem_section.mr after the unref of the old MR, so that error_return
> does not incorrectly double-unref the old MR.
>
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1407
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/display/exynos4210_fimd.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/hw/display/exynos4210_fimd.c b/hw/display/exynos4210_fimd.c
> index c61e0280a7c..eec874d0b1d 100644
> --- a/hw/display/exynos4210_fimd.c
> +++ b/hw/display/exynos4210_fimd.c
> @@ -1147,6 +1147,13 @@ static void fimd_update_memory_section(Exynos4210fimdState *s, unsigned win)
>      if (w->mem_section.mr) {
>          memory_region_set_log(w->mem_section.mr, false, DIRTY_MEMORY_VGA);
>          memory_region_unref(w->mem_section.mr);
> +        w->mem_section.mr = NULL;
> +    }
> +
> +    if (w->fb_len == 0) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "FIMD: Guest config means framebuffer is zero length\n");
> +        goto error_return;
>      }
>
>      w->mem_section = memory_region_find(s->fbmem, fb_start_addr, w->fb_len);
> --
> 2.43.0

Re: [PATCH] hw/display/exynos4210_fimd: Account for zero length in fimd_update_memory_section()

[Philippe Mathieu-Daudé]

On 7/11/25 15:39, Peter Maydell wrote:
> In fimd_update_memory_section() we attempt ot find and map part of
> the RAM MR which backs the framebuffer, based on guest-configurable
> size and start address.
> 
> If the guest configures framebuffer settings which result in a
> zero-sized framebuffer, we hit an assertion(), because
> memory_region_find() will return a NULL mem_section.mr.

The datasheet is not clear about what to do here...

   41.3.10 Virtual Display

     The size of video buffer in which you store the image should
     be larger than the LCD panel screen size.

     OFFSIZE_F should have value that is multiple of 4byte size or 0.

     PAGEWIDTH should have bigger value than the burst size and you
     should align the size word boundary.

(virtpage_offsize is OFFSIZE_F and virtpage_width is PAGEWIDTH).

I couldn't find any info more useful, and your patch is safe, so:

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> Explicitly check for the zero-size case and treat this as a
> guest error.
> 
> Because we now have a code path which can reach error_return without
> calling memory_region_find to set w->mem_section, we must NULL out
> w->mem_section.mr after the unref of the old MR, so that error_return
> does not incorrectly double-unref the old MR.
> 
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1407
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/display/exynos4210_fimd.c | 7 +++++++
>   1 file changed, 7 insertions(+)