From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 Cc: Ard Biesheuvel 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: [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: , X-List-Received-Date: Fri, 18 Aug 2017 13:00:27 -0000 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(-) 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 -- 2.11.0