hw/display/bcm2835_fb: Validate config settings

Validate the config settings that the guest tries to set.

The wiki page documentation is not really accurate here:
generally rather than failing requests to set bad parameters,
the hardware will just clip them to something sensible.

Validate the most important parameters: sizes and
the viewport offsets. This prevents the framebuffer
code from trying to read out-of-range memory.

In the property handling code, we validate the new parameters every
time we encounter a tag that sets them. This means we validate the
config multiple times if the request includes multiple config-setting
tags, but the code would require significant restructuring to do a
validation only once but still return the clipped settings for
get-parameter tags and the buffer allocation tag.

Validation of settings made via the older bcm2835_fb_mbox_push()
function will be done in the next commit.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20180814144436.679-8-peter.maydell@linaro.org

1 files changed, 46 insertions, 2 deletions

diff --git a/hw/display/bcm2835_fb.c b/hw/display/bcm2835_fb.c
index 76a10072b4..3edb8b5cfc 100644
--- a/hw/display/bcm2835_fb.c
+++ b/hw/display/bcm2835_fb.c
@@ -34,6 +34,13 @@
 #define DEFAULT_VCRAM_SIZE 0x4000000
 #define BCM2835_FB_OFFSET  0x00100000
 
+/* Maximum permitted framebuffer size; experimentally determined on an rpi2 */
+#define XRES_MAX 3840
+#define YRES_MAX 2560
+/* Framebuffer size used if guest requests zero size */
+#define XRES_SMALL 592
+#define YRES_SMALL 488
+
 static void fb_invalidate_display(void *opaque)
 {
     BCM2835FBState *s = BCM2835_FB(opaque);
@@ -202,6 +209,45 @@ static void fb_update_display(void *opaque)
     s->invalidate = false;
 }
 
+void bcm2835_fb_validate_config(BCM2835FBConfig *config)
+{
+    /*
+     * Validate the config, and clip any bogus values into range,
+     * as the hardware does. Note that fb_update_display() relies on
+     * this happening to prevent it from performing out-of-range
+     * accesses on redraw.
+     */
+    config->xres = MIN(config->xres, XRES_MAX);
+    config->xres_virtual = MIN(config->xres_virtual, XRES_MAX);
+    config->yres = MIN(config->yres, YRES_MAX);
+    config->yres_virtual = MIN(config->yres_virtual, YRES_MAX);
+
+    /*
+     * These are not minima: a 40x40 framebuffer will be accepted.
+     * They're only used as defaults if the guest asks for zero size.
+     */
+    if (config->xres == 0) {
+        config->xres = XRES_SMALL;
+    }
+    if (config->yres == 0) {
+        config->yres = YRES_SMALL;
+    }
+    if (config->xres_virtual == 0) {
+        config->xres_virtual = config->xres;
+    }
+    if (config->yres_virtual == 0) {
+        config->yres_virtual = config->yres;
+    }
+
+    if (fb_use_offsets(config)) {
+        /* Clip the offsets so the viewport is within the physical screen */
+        config->xoffset = MIN(config->xoffset,
+                              config->xres_virtual - config->xres);
+        config->yoffset = MIN(config->yoffset,
+                              config->yres_virtual - config->yres);
+    }
+}
+
 static void bcm2835_fb_mbox_push(BCM2835FBState *s, uint32_t value)
 {
     uint32_t pitch;
@@ -238,8 +284,6 @@ void bcm2835_fb_reconfigure(BCM2835FBState *s, BCM2835FBConfig *newconfig)
 {
     s->lock = true;
 
-    /* TODO: input validation! */
-
     s->config = *newconfig;
 
     s->invalidate = true;