From patchwork Fri Aug 18 13:02:43 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ard Biesheuvel X-Patchwork-Id: 110387 Delivered-To: patch@linaro.org Received: by 10.140.95.78 with SMTP id h72csp887165qge; Fri, 18 Aug 2017 06:02:57 -0700 (PDT) X-Received: by 10.99.139.66 with SMTP id j63mr686412pge.180.1503061377519; Fri, 18 Aug 2017 06:02:57 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1503061377; cv=none; d=google.com; s=arc-20160816; b=0stsgcqzYPDCUfB1fCZg0toaj9IQaZA4nNZ8+4d1+c3d5WG+TbtdXnYqs9pQlrZGHS Fb0kaLX6S1r4ma7rKRG007Xy8Jb3WM2prqus3skJJtwzO4mXMNcqFPoTlDc3k+JfUKZ1 SZQ9RAkbje5Pr+BLQPlQ/E7glGtRVHhkSzpokxxXG+m7iistgSBfsNWJn04kGt7yyGyg ULMVBA9np6bUnR4lhVGkK0JQVw2di/yxQBSOQRowgCRZoFkt9wKFtUtQA0GqM43aFfLn /bTtpjOyx301nWK16V0aH1taqaPKZcZTNV7hTId//OLpbHQ+zrBDuNy/2w6hu2yXbtuk voEg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:content-transfer-encoding:mime-version:cc :list-subscribe:list-help:list-post:list-archive:list-unsubscribe :list-id:precedence:subject:message-id:date:to:from:dkim-signature :delivered-to:arc-authentication-results; bh=6t88ugumAJkxVyUomlgxqNJalLb5LnQZQjaY4zs8VP4=; b=HefxCWsacbgm8fIyt9N1scGUE9Nt1hQLgZE1hAXkMKAcg4i2kSWjGeORSYTmCTu8Hb 2EgAbOD2kozb584O1h3NSRZhv+/0ysT9adHIUfsxcttfMy0LSregoTwLXYyem2DUW5KG fG5B3d9ult1tqEz2LgLdCkUfOmo8IM44OXBELKI3UMbe88zjNn6mQFybLWOrW0tb6Eqy g3D9OZFTt+tRnYL3bh7KlUcg76iSrlsRCqYjrAMqpgMO78wvXL2A4s9nrfnCOdlVhUx1 k8fD5l40/puVT2BFZsMKI3blxoLYymqhWv43uMstmw2mjb0KXThJnPuF3ZqodDvQgeKX Kd/A== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org header.s=google header.b=H36RXSyC; spf=pass (google.com: best guess record for domain of edk2-devel-bounces@lists.01.org designates 198.145.21.10 as permitted sender) smtp.mailfrom=edk2-devel-bounces@lists.01.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from ml01.01.org (ml01.01.org. [198.145.21.10]) by mx.google.com with ESMTPS id g6si1649192pgu.47.2017.08.18.06.02.57 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 18 Aug 2017 06:02:57 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of edk2-devel-bounces@lists.01.org designates 198.145.21.10 as permitted sender) client-ip=198.145.21.10; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org header.s=google header.b=H36RXSyC; spf=pass (google.com: best guess record for domain of edk2-devel-bounces@lists.01.org designates 198.145.21.10 as permitted sender) smtp.mailfrom=edk2-devel-bounces@lists.01.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id 38C5E2095D8D3; Fri, 18 Aug 2017 06:00:28 -0700 (PDT) X-Original-To: edk2-devel@lists.01.org Delivered-To: edk2-devel@lists.01.org Received: from mail-wr0-x22c.google.com (mail-wr0-x22c.google.com [IPv6:2a00:1450:400c:c0c::22c]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id BB10321B0E537 for ; Fri, 18 Aug 2017 06:00:26 -0700 (PDT) Received: by mail-wr0-x22c.google.com with SMTP id z91so61749339wrc.4 for ; Fri, 18 Aug 2017 06:02:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id; bh=8ylspPw1lhWopHZBJDasuICisyQK3tRxFSxnlEek7pM=; b=H36RXSyCNiK9DREsmA5toHLVMtOYn56HcyMXprQv771dvNJsAab6zrtGp5FWx/c3XB JQar4l2uHxkLfU5VdiZrGv6WxD48zNSLMbnyG25/vPc/3xzb/42W39Qnghc+SO7fz7Y6 Zl7ASHF4GMvbXXoiDJYoyrtJIUcDcllTjnbq4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=8ylspPw1lhWopHZBJDasuICisyQK3tRxFSxnlEek7pM=; b=fIztJ6S8gqdDUUkgA0epVzo8MDC7dDd8oZvueiAcHgtT3Oqqtrxe4b6PueyNO48s+g V4318SF2YVUNcT4ci9wrob5qxAjJyjBOWvr6Ep54maxCKFGWYe0aFhmTtQbtBoEroAlh viAxKdFD9/+HtRejVHepKHk2qHaCAxMQm349zmx964icO59Nu7JdfQsYTT8W5OQoDhS6 CpgR4KJCw+NF/omYP1mqCTbH8qr05vI9wH/q1nIQx0f+0QZd+XMzod16dE/cYzpWImu/ SEz3J4a+LkTeJU6rqCggSfLAybTgjxB/B+Dn6KYy6g0Lll9Vu//DRtOB/A0LP/eysiTF XBXw== X-Gm-Message-State: AHYfb5iZvxEBDeChdoGA7Cfbx0yjkrcGHzUMrhbEwRdCDKjt14cpujT2 GZAoF2622fBiRzavXyWlbQ== X-Received: by 10.223.197.139 with SMTP id m11mr6316786wrg.217.1503061372828; Fri, 18 Aug 2017 06:02:52 -0700 (PDT) Received: from localhost.localdomain ([154.146.161.128]) by smtp.gmail.com with ESMTPSA id 16sm8179525wrx.26.2017.08.18.06.02.50 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 18 Aug 2017 06:02:51 -0700 (PDT) From: Ard Biesheuvel To: edk2-devel@lists.01.org, lersek@redhat.com, leif.lindholm@linaro.org, jordan.l.justen@intel.com Date: Fri, 18 Aug 2017 14:02:43 +0100 Message-Id: <20170818130243.4645-1-ard.biesheuvel@linaro.org> X-Mailer: git-send-email 2.11.0 Subject: [edk2] [PATCH] OvmfPkg/QemuVideoDxe: map framebuffer as write-combining/non-executable X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Ard Biesheuvel MIME-Version: 1.0 Errors-To: edk2-devel-bounces@lists.01.org Sender: "edk2-devel" When QemuVideoDxe takes control of the framebuffer, it is already mapped EFI_MEMORY_UC by core code, and QemuVideoDxe simply records the base and size from the PCI BAR. On x86 systems, this is sufficient, but on ARM systems, the semantics of EFI_MEMORY_UC regions are quite different from EFI_MEMORY_WC regions, and treating a region like memory (i.e., dereferencing pointers into it or using ordinary CopyMem()/SetMem() functions on it) requires that it be mapped with memory semantics, i.e., EFI_MEMORY_WC, EFI_MEMORY_WT or EFI_MEMORY_WB. Since caching is not appropriate for regions where we rely on side effects, remap the frame buffer EFI_MEMORY_WT. Given that the ARM architecture requires device mappings to be non-executable (to avoid inadvertent speculative instruction fetches from device registers), retain the non-executable nature by adding the EFI_MEMORY_XP attribute as well. Note that the crashes that this patch aims to prevent can currently only occur under KVM, in which case the VGA driver does not operate correctly in the first place. However, this is an implementation detail of QEMU while running under KVM, and given that the ARM architecture simply does not permit unaligned accesses to device memory, it is best to conform to this in the code. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel --- OvmfPkg/QemuVideoDxe/Driver.c | 5 +++++ OvmfPkg/QemuVideoDxe/Gop.c | 18 ++++++++++++++++-- OvmfPkg/QemuVideoDxe/Qemu.h | 2 ++ OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf | 1 + 4 files changed, 24 insertions(+), 2 deletions(-) -- 2.11.0 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel Reviewed-by: Leif Lindholm Reviewed-by: Jordan Justen diff --git a/OvmfPkg/QemuVideoDxe/Driver.c b/OvmfPkg/QemuVideoDxe/Driver.c index 0dce80e59ba8..d81be49d91f3 100644 --- a/OvmfPkg/QemuVideoDxe/Driver.c +++ b/OvmfPkg/QemuVideoDxe/Driver.c @@ -69,6 +69,8 @@ QEMU_VIDEO_CARD gQemuVideoCardList[] = { } }; +EFI_CPU_ARCH_PROTOCOL *gCpu; + static QEMU_VIDEO_CARD* QemuVideoDetect( IN UINT16 VendorId, @@ -1103,5 +1105,8 @@ InitializeQemuVideo ( ); ASSERT_EFI_ERROR (Status); + Status = gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **)&gCpu); + ASSERT_EFI_ERROR(Status); + return Status; } diff --git a/OvmfPkg/QemuVideoDxe/Gop.c b/OvmfPkg/QemuVideoDxe/Gop.c index 512fd27acbda..a820524db293 100644 --- a/OvmfPkg/QemuVideoDxe/Gop.c +++ b/OvmfPkg/QemuVideoDxe/Gop.c @@ -53,7 +53,8 @@ QemuVideoCompleteModeData ( { EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *Info; EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *FrameBufDesc; - QEMU_VIDEO_MODE_DATA *ModeData; + QEMU_VIDEO_MODE_DATA *ModeData; + EFI_STATUS Status; ModeData = &Private->ModeData[Mode->Mode]; Info = Mode->Info; @@ -72,8 +73,21 @@ QemuVideoCompleteModeData ( DEBUG ((EFI_D_INFO, "FrameBufferBase: 0x%Lx, FrameBufferSize: 0x%Lx\n", Mode->FrameBufferBase, (UINT64)Mode->FrameBufferSize)); + // + // Remap the framebuffer region as write combining. On x86 systems, this is + // merely a performance optimization, but on ARM systems, it prevents + // crashes that may result from unaligned accesses, given that we treat the + // frame buffer as ordinary memory by using CopyMem()/SetMem() on it. While + // we're at it, set the non-exec attribute so the framebuffer is not + // exploitable by malware. + // + Status = gCpu->SetMemoryAttributes (gCpu, Mode->FrameBufferBase, + ALIGN_VALUE (Mode->FrameBufferSize, EFI_PAGE_SIZE), + EFI_MEMORY_WC | EFI_MEMORY_XP); + ASSERT_EFI_ERROR (Status); + FreePool (FrameBufDesc); - return EFI_SUCCESS; + return Status; } STATIC diff --git a/OvmfPkg/QemuVideoDxe/Qemu.h b/OvmfPkg/QemuVideoDxe/Qemu.h index 7fbb25b3efd3..2966c77c78b3 100644 --- a/OvmfPkg/QemuVideoDxe/Qemu.h +++ b/OvmfPkg/QemuVideoDxe/Qemu.h @@ -21,6 +21,7 @@ #include +#include #include #include #include @@ -164,6 +165,7 @@ extern EFI_DRIVER_BINDING_PROTOCOL gQemuVideoDriverBinding; extern EFI_COMPONENT_NAME_PROTOCOL gQemuVideoComponentName; extern EFI_COMPONENT_NAME2_PROTOCOL gQemuVideoComponentName2; extern EFI_DRIVER_SUPPORTED_EFI_VERSION_PROTOCOL gQemuVideoDriverSupportedEfiVersion; +extern EFI_CPU_ARCH_PROTOCOL *gCpu; // // Io Registers defined by VGA diff --git a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf index 346a5aed94fa..bbe11257c002 100644 --- a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf +++ b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf @@ -68,6 +68,7 @@ [LibraryClasses] UefiLib [Protocols] + gEfiCpuArchProtocolGuid # PROTOCOL ALWAYS_CONSUMED gEfiDriverSupportedEfiVersionProtocolGuid # PROTOCOL ALWAYS_PRODUCED gEfiGraphicsOutputProtocolGuid # PROTOCOL BY_START gEfiDevicePathProtocolGuid # PROTOCOL BY_START