From patchwork Mon Nov 19 16:26:58 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Peter Maydell X-Patchwork-Id: 151499 Delivered-To: patches@linaro.org Received: by 2002:a2e:299d:0:0:0:0:0 with SMTP id p29-v6csp2865708ljp; Mon, 19 Nov 2018 08:27:04 -0800 (PST) X-Google-Smtp-Source: AJdET5dX+Ff5IbJvZZ9n0qogjmXsjHmt8x0kKOM4ZPAPGHiAZwc2xuTqoDUimLcaEDVpGALEfe7Y X-Received: by 2002:a05:6000:d2:: with SMTP id q18mr19556042wrx.302.1542644824353; Mon, 19 Nov 2018 08:27:04 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1542644824; cv=none; d=google.com; s=arc-20160816; b=PYg1x16bBmVnRE9K0hq4oUGFv5ezwCOpbbAbZIm+CmjcWbwVlwmE6GSVWKSFD4iTBC VjyfXLoozF3c9VwlVVk5ojj2Q15WARUmHN1DW2IeR25RVtmWF6OejKiZks13T/abW5Kc cjYtWQgw0iO85fF71KoZjPRwYu3zTP8j5Lqn+VZBvBN+fq4ITk7algUvihPADhTnBWRQ OmpUYNzNIvVuue1/YXziT8kHoLvpT1ts78VgAVotYZ1UD8nK3mtRiFwhaK/gYJs6mUwv /g4TcwywS3LBHiZq/OO6WsIukGtTeVktYRB6Hg9x9HQmJY+SoglhGkQ95RP0az9FRS67 rvGQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from; bh=KrPJIDs43pjA5nUCzNV7AqaTADIvE2UNjq3hcHRvTA4=; b=P8CtxHsSKzVHIG/ATNN0H+7qMKdiUhQhlgmxhZnnW1gkyVg8TyYREcaO6YQro+OlV2 MfE4rv0avuhalHjCQJYIL3GGyWvvJko7XYjjUZFLQbuoHHnNRFN900AVJNejtbZBOxeZ jMzGOQSqCXkR3n2sVeNRHN0NLO4DX+wgVtxJRf0elmumhs4iUnzypS/UEpkRnsnCqW2D GVjD/Bu1tvZgrAId+l4pRlfO+kLTcWGBWq3E4SVEUbXbVGjd87vT4chxg+ufaYhu503a OQCpQNR/fJkptv8mAuYARZfVjgeLO9TX2a/RnIu3akhS2DA+oOcsoSwgO3AR07jiAoiF uzzg== 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 u4si10325322wmg.158.2018.11.19.08.27.04 for (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 19 Nov 2018 08:27:04 -0800 (PST) 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 1gOmO5-000683-9Y; Mon, 19 Nov 2018 16:27:01 +0000 From: Peter Maydell To: qemu-devel@nongnu.org Cc: patches@linaro.org, Stefano Stabellini , Anthony Perard , xen-devel@lists.xenproject.org Subject: [PATCH for-3.1] hw/xen/xen_pt_graphics: Don't trust the BIOS ROM contents so much Date: Mon, 19 Nov 2018 16:26:58 +0000 Message-Id: <20181119162658.30358-1-peter.maydell@linaro.org> X-Mailer: git-send-email 2.19.1 MIME-Version: 1.0 Coverity (CID 796599) points out that xen_pt_setup_vga() trusts the rom->size field in the BIOS ROM from a PCI passthrough VGA device, and uses it as an index into the memory which contains the BIOS image. A corrupt BIOS ROM could therefore cause us to index off the end of the buffer. Check that the size is within bounds before we use it. We are also trusting the pcioffset field, and assuming that the whole rom_header is present; Coverity doesn't notice these, but check them too. Signed-off-by: Peter Maydell --- Disclaimer: compile tested only, as I don't have a Xen setup, let alone one with pass-through PCI graphics. Note that https://xenbits.xen.org/xsa/advisory-124.html defines that bugs which are only exploitable by a malicious piece of hardware that is passed through to the guest are not security vulnerabilities as far as the Xen Project is concerned, and are treated like normal non-security-related bugs. So this is just a bugfix, not a security issue. Marked "for-3.1" because it would let us squash another Coverity issue, and it is a bug fix; on the other hand it's an obscure corner case and has been this way since forever. --- hw/xen/xen_pt_graphics.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) -- 2.19.1 Acked-by: Anthony PERARD diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c index 135c8df1e72..60d6b4a5563 100644 --- a/hw/xen/xen_pt_graphics.c +++ b/hw/xen/xen_pt_graphics.c @@ -185,8 +185,19 @@ void xen_pt_setup_vga(XenPCIPassthroughState *s, XenHostPCIDevice *dev, return; } + if (bios_size < sizeof(struct rom_header)) { + error_setg(errp, "VGA: VBIOS image corrupt (too small)"); + return; + } + /* Currently we fixed this address as a primary. */ rom = (struct rom_header *)bios; + + if (rom->pcioffset + sizeof(struct pci_data) > bios_size) { + error_setg(errp, "VGA: VBIOS image corrupt (bad pcioffset field)"); + return; + } + pd = (void *)(bios + (unsigned char)rom->pcioffset); /* We may need to fixup Device Identification. */ @@ -194,6 +205,11 @@ void xen_pt_setup_vga(XenPCIPassthroughState *s, XenHostPCIDevice *dev, pd->device = s->real_device.device_id; len = rom->size * 512; + if (len > bios_size) { + error_setg(errp, "VGA: VBIOS image corrupt (bad size field)"); + return; + } + /* Then adjust the bios checksum */ for (c = (char *)bios; c < ((char *)bios + len); c++) { checksum += *c;