From patchwork Tue Aug 14 14:44:35 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Peter Maydell X-Patchwork-Id: 144181 Delivered-To: patches@linaro.org Received: by 2002:a2e:9754:0:0:0:0:0 with SMTP id f20-v6csp4450926ljj; Tue, 14 Aug 2018 07:44:46 -0700 (PDT) X-Google-Smtp-Source: AA+uWPyj/jAaBC7aq/Z+0NJrOMZvGa5tIAXdBFf1TceQTxfC0Mo7eRFkeZzMOZ2guYf3qD2St6oD X-Received: by 2002:a1c:6c03:: with SMTP id h3-v6mr10470826wmc.38.1534257886049; Tue, 14 Aug 2018 07:44:46 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1534257886; cv=none; d=google.com; s=arc-20160816; b=mraPJ8r37OP8/fVio+hIqBLKE9I+A92q99/nTv3HQJWm8COQvR1AHOS9wej8WRPamL onUOzLHwrLntvS5IoSZAvxkdPeByp88DTf0DIc9TVNv9KskYrp/PJ9A+Uhbsy1I0rTqg K+7MLLPRI3j62ek9QKN/hf/ehtgvXHHFmMgrNwcWtrWpW+/O4O6qkMsMq+lAodOow/jj HAlX+jrjsicKrexPP8/gUMfYjlQouclu+0BlcmDjexQefuz8RhdQdLF4iRNKXF8UzDpr EAc8bIa97MEHPaSxSGzjUyXAIbBTC0ri6RbQ8xukS6qpJwPm+6rHc794VQgkAnk2qN3J e90w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=references:in-reply-to:message-id:date:subject:cc:to:from :arc-authentication-results; bh=HJzEcUuMelrOgp0W/yse7R5pFuMoBHKJZ6WBYDOFTqQ=; b=CASD3DBRqVS6ClncWyfZO4G5KMTyMnoQw0WYuPZzBbUC3F5WXhfwA7FngZviXfDDaN RvlaCvh11ZnOIXw+nJYY46gHUHmRUOdFm6t+XW237/X+kzWy6+fblWp60K2phtE+8yPS ekTPggV1lcD7TEMGUUvuRTasJP0CtF8GqecbNbwQRTGZQfZbZ/mf9+2rgMM+zMYaZHps 0n9PE9qirBjDnEMCkBt1Tgt+gZkrmDsF1AekW/QaudoknYDiQ6JJX1lAVvLUAE5Zxjls HOGuGzlI/Wh3uRkHc37epf6yRuDc+L3BDtN+DzsUbw3eqcPE2Iz4aEmdg0CsMEpSP62Y pmOQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of pm215@archaic.org.uk designates 2001:8b0:1d0::2 as permitted sender) smtp.mailfrom=pm215@archaic.org.uk; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from orth.archaic.org.uk (orth.archaic.org.uk. [2001:8b0:1d0::2]) by mx.google.com with ESMTPS id f1-v6si16074856wrh.303.2018.08.14.07.44.45 for (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 14 Aug 2018 07:44:46 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of pm215@archaic.org.uk designates 2001:8b0:1d0::2 as permitted sender) client-ip=2001:8b0:1d0::2; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of pm215@archaic.org.uk designates 2001:8b0:1d0::2 as permitted sender) smtp.mailfrom=pm215@archaic.org.uk; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from pm215 by orth.archaic.org.uk with local (Exim 4.89) (envelope-from ) id 1fpaYv-000720-Gv; Tue, 14 Aug 2018 15:44:45 +0100 From: Peter Maydell To: qemu-arm@nongnu.org, qemu-devel@nongnu.org Cc: patches@linaro.org Subject: [PATCH 7/8] hw/display/bcm2835_fb: Validate config settings Date: Tue, 14 Aug 2018 15:44:35 +0100 Message-Id: <20180814144436.679-8-peter.maydell@linaro.org> X-Mailer: git-send-email 2.18.0 In-Reply-To: <20180814144436.679-1-peter.maydell@linaro.org> References: <20180814144436.679-1-peter.maydell@linaro.org> 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 --- include/hw/display/bcm2835_fb.h | 8 +++++ hw/display/bcm2835_fb.c | 48 +++++++++++++++++++++++++++-- hw/misc/bcm2835_property.c | 54 ++++++++++++++++----------------- 3 files changed, 81 insertions(+), 29 deletions(-) -- 2.18.0 Reviewed-by: Richard Henderson diff --git a/include/hw/display/bcm2835_fb.h b/include/hw/display/bcm2835_fb.h index d992c60c120..228988ba056 100644 --- a/include/hw/display/bcm2835_fb.h +++ b/include/hw/display/bcm2835_fb.h @@ -76,4 +76,12 @@ static inline uint32_t bcm2835_fb_get_size(BCM2835FBConfig *config) return yres * bcm2835_fb_get_pitch(config); } +/** + * bcm2835_fb_validate_config: check provided config + * + * Validates the configuration information provided by the guest and + * adjusts it if necessary. + */ +void bcm2835_fb_validate_config(BCM2835FBConfig *config); + #endif diff --git a/hw/display/bcm2835_fb.c b/hw/display/bcm2835_fb.c index 76a10072b46..3edb8b5cfcb 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; diff --git a/hw/misc/bcm2835_property.c b/hw/misc/bcm2835_property.c index e3ab677891b..145427ae0f8 100644 --- a/hw/misc/bcm2835_property.c +++ b/hw/misc/bcm2835_property.c @@ -155,16 +155,6 @@ static void bcm2835_property_mbox_push(BCM2835PropertyState *s, uint32_t value) case 0x00040002: /* Blank screen */ resplen = 4; break; - case 0x00040003: /* Get physical display width/height */ - stl_le_phys(&s->dma_as, value + 12, fbconfig.xres); - stl_le_phys(&s->dma_as, value + 16, fbconfig.yres); - resplen = 8; - break; - case 0x00040004: /* Get virtual display width/height */ - stl_le_phys(&s->dma_as, value + 12, fbconfig.xres_virtual); - stl_le_phys(&s->dma_as, value + 16, fbconfig.yres_virtual); - resplen = 8; - break; case 0x00044003: /* Test physical display width/height */ case 0x00044004: /* Test virtual display width/height */ resplen = 8; @@ -172,29 +162,35 @@ static void bcm2835_property_mbox_push(BCM2835PropertyState *s, uint32_t value) case 0x00048003: /* Set physical display width/height */ fbconfig.xres = ldl_le_phys(&s->dma_as, value + 12); fbconfig.yres = ldl_le_phys(&s->dma_as, value + 16); + bcm2835_fb_validate_config(&fbconfig); fbconfig_updated = true; + /* fall through */ + case 0x00040003: /* Get physical display width/height */ + stl_le_phys(&s->dma_as, value + 12, fbconfig.xres); + stl_le_phys(&s->dma_as, value + 16, fbconfig.yres); resplen = 8; break; case 0x00048004: /* Set virtual display width/height */ fbconfig.xres_virtual = ldl_le_phys(&s->dma_as, value + 12); fbconfig.yres_virtual = ldl_le_phys(&s->dma_as, value + 16); + bcm2835_fb_validate_config(&fbconfig); fbconfig_updated = true; + /* fall through */ + case 0x00040004: /* Get virtual display width/height */ + stl_le_phys(&s->dma_as, value + 12, fbconfig.xres_virtual); + stl_le_phys(&s->dma_as, value + 16, fbconfig.yres_virtual); resplen = 8; break; - case 0x00040005: /* Get depth */ - stl_le_phys(&s->dma_as, value + 12, fbconfig.bpp); - resplen = 4; - break; case 0x00044005: /* Test depth */ resplen = 4; break; case 0x00048005: /* Set depth */ fbconfig.bpp = ldl_le_phys(&s->dma_as, value + 12); + bcm2835_fb_validate_config(&fbconfig); fbconfig_updated = true; - resplen = 4; - break; - case 0x00040006: /* Get pixel order */ - stl_le_phys(&s->dma_as, value + 12, fbconfig.pixo); + /* fall through */ + case 0x00040005: /* Get depth */ + stl_le_phys(&s->dma_as, value + 12, fbconfig.bpp); resplen = 4; break; case 0x00044006: /* Test pixel order */ @@ -202,11 +198,11 @@ static void bcm2835_property_mbox_push(BCM2835PropertyState *s, uint32_t value) break; case 0x00048006: /* Set pixel order */ fbconfig.pixo = ldl_le_phys(&s->dma_as, value + 12); + bcm2835_fb_validate_config(&fbconfig); fbconfig_updated = true; - resplen = 4; - break; - case 0x00040007: /* Get alpha */ - stl_le_phys(&s->dma_as, value + 12, fbconfig.alpha); + /* fall through */ + case 0x00040006: /* Get pixel order */ + stl_le_phys(&s->dma_as, value + 12, fbconfig.pixo); resplen = 4; break; case 0x00044007: /* Test pixel alpha */ @@ -214,7 +210,11 @@ static void bcm2835_property_mbox_push(BCM2835PropertyState *s, uint32_t value) break; case 0x00048007: /* Set alpha */ fbconfig.alpha = ldl_le_phys(&s->dma_as, value + 12); + bcm2835_fb_validate_config(&fbconfig); fbconfig_updated = true; + /* fall through */ + case 0x00040007: /* Get alpha */ + stl_le_phys(&s->dma_as, value + 12, fbconfig.alpha); resplen = 4; break; case 0x00040008: /* Get pitch */ @@ -222,18 +222,18 @@ static void bcm2835_property_mbox_push(BCM2835PropertyState *s, uint32_t value) bcm2835_fb_get_pitch(&fbconfig)); resplen = 4; break; - case 0x00040009: /* Get virtual offset */ - stl_le_phys(&s->dma_as, value + 12, fbconfig.xoffset); - stl_le_phys(&s->dma_as, value + 16, fbconfig.yoffset); - resplen = 8; - break; case 0x00044009: /* Test virtual offset */ resplen = 8; break; case 0x00048009: /* Set virtual offset */ fbconfig.xoffset = ldl_le_phys(&s->dma_as, value + 12); fbconfig.yoffset = ldl_le_phys(&s->dma_as, value + 16); + bcm2835_fb_validate_config(&fbconfig); fbconfig_updated = true; + /* fall through */ + case 0x00040009: /* Get virtual offset */ + stl_le_phys(&s->dma_as, value + 12, fbconfig.xoffset); + stl_le_phys(&s->dma_as, value + 16, fbconfig.yoffset); resplen = 8; break; case 0x0004000a: /* Get/Test/Set overscan */