From patchwork Tue Feb 28 13:56:09 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ard Biesheuvel X-Patchwork-Id: 94614 Delivered-To: patch@linaro.org Received: by 10.140.20.113 with SMTP id 104csp1330915qgi; Tue, 28 Feb 2017 05:56:37 -0800 (PST) X-Received: by 10.98.147.10 with SMTP id b10mr2609482pfe.177.1488290197053; Tue, 28 Feb 2017 05:56:37 -0800 (PST) Return-Path: Received: from ml01.01.org (ml01.01.org. [2001:19d0:306:5::1]) by mx.google.com with ESMTPS id g6si1851208pln.179.2017.02.28.05.56.36 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 28 Feb 2017 05:56:37 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of edk2-devel-bounces@lists.01.org designates 2001:19d0:306:5::1 as permitted sender) client-ip=2001:19d0:306:5::1; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org; spf=pass (google.com: best guess record for domain of edk2-devel-bounces@lists.01.org designates 2001:19d0:306:5::1 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 7784881F56; Tue, 28 Feb 2017 05:56:36 -0800 (PST) X-Original-To: edk2-devel@lists.01.org Delivered-To: edk2-devel@lists.01.org Received: from mail-wr0-x234.google.com (mail-wr0-x234.google.com [IPv6:2a00:1450:400c:c0c::234]) (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 D514881F45 for ; Tue, 28 Feb 2017 05:56:34 -0800 (PST) Received: by mail-wr0-x234.google.com with SMTP id l37so9347103wrc.1 for ; Tue, 28 Feb 2017 05:56:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id; bh=0ggdoV7gNWxG4E+fQOHNQNyOYF6mdE68G2e3Cw1M9BA=; b=bET4gWoK3lMCNd3Wf9QlWblwokjEy5qqBDZ/FZc/VDtSxBwF6N5cd3TKUWCQUmJ6ZD k9wn+EDAOudD4e1thWKeoYKp0ZpDtamYcwxa2q9t0zAErc40Z58ZmwnCi3zx94YWxV0v N26chdEhJgzQMXLkkugp3fJwMW9KW2GcfnDOw= 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=0ggdoV7gNWxG4E+fQOHNQNyOYF6mdE68G2e3Cw1M9BA=; b=EG3W8v06xZQMN8yrjZ2w2yattjclChuDqzEJUvcBaDDSzVTGBpuwvY1cCJmy0Ft42f +/8RJiDEUx+2zIa4rhsSS01w5epSv8BoOSjZbC8UWbCKC9xDkKEAVIOT4HCyB9qvry5E Nlrz4VUTKw7myZGuU9zcizJUVNmTivY1fRVG28MVLpuf6CwlrX5C7VY9binZVgDXFPze ScNQZcr+UUuL+Vc1N0GDUnTVPSyXWqmO6agNWTCibWfWzjWqPfYWi6c45/oh7y5uRqpH ZHXnzj3aGDn7ElFwZaIIWpRxbsXT1EWQ/wGxzh7QM0S+mOH6RFVeiHpoxE8ULyMF+42a PHew== X-Gm-Message-State: AMke39lcjvHAcI6yo6DcfxTwA1o+j9Z96wIp/RRkzYZtYM7lqfP22RQykqClgjpyLZvVLuZ7 X-Received: by 10.223.152.83 with SMTP id v77mr2472183wrb.109.1488290193414; Tue, 28 Feb 2017 05:56:33 -0800 (PST) Received: from localhost.localdomain ([105.149.201.216]) by smtp.gmail.com with ESMTPSA id 186sm18600511wmw.24.2017.02.28.05.56.31 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 28 Feb 2017 05:56:32 -0800 (PST) From: Ard Biesheuvel To: edk2-devel@lists.01.org, lersek@redhat.com Date: Tue, 28 Feb 2017 13:56:09 +0000 Message-Id: <1488290169-10701-1-git-send-email-ard.biesheuvel@linaro.org> X-Mailer: git-send-email 2.7.4 Subject: [edk2] [PATCH] ArmVirtPkg/HighMemDxe: preserve non-exec permissions on newly added regions X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 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" Using DxeServices::SetMemorySpaceAttributes to set cacheability attributes has the side effect of stripping permission attributes, given that those are bits in the same bitfield, and so setting the Attributes argument to EFI_MEMORY_WB implies not setting EFI_MEMORY_XP or EFI_MEMORY_RO attributes. In fact, the situation is even worse, given that the descriptor returned by DxeServices::GetMemorySpaceDescriptor does not reflect the permission attributes that may have been set by the preceding call to DxeServices::AddMemorySpace if PcdDxeNxMemoryProtectionPolicy has been configured to map EfiConventionalMemory with non-executable permissions. Note that this applies equally to the non-executable stack and to PE/COFF sections that may have been mapped with R-X or RW- permissions. This is due to the ambiguity in the meaning of the EFI_MEMORY_RO/EFI_MEMORY_XP attributes when used in the GCD memory map, i.e., between signifying that an underlying RAM region has the controls to be configures as read-only or non-executable, and signifying that the contents of a certain UEFI memory region allow them to be mapped with certain restricted permissions. So let's check the policy in PcdDxeNxMemoryProtectionPolicy directly, and set the EFI_MEMORY_XP attribute if appropriate for EfiConventionalMemory regions. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel --- ArmVirtPkg/HighMemDxe/HighMemDxe.c | 18 ++++++++++++++++-- ArmVirtPkg/HighMemDxe/HighMemDxe.inf | 1 + 2 files changed, 17 insertions(+), 2 deletions(-) -- 2.7.4 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel Reviewed-by: Laszlo Ersek diff --git a/ArmVirtPkg/HighMemDxe/HighMemDxe.c b/ArmVirtPkg/HighMemDxe/HighMemDxe.c index 22f738279b20..853660437cb0 100644 --- a/ArmVirtPkg/HighMemDxe/HighMemDxe.c +++ b/ArmVirtPkg/HighMemDxe/HighMemDxe.c @@ -37,6 +37,7 @@ InitializeHighMemDxe ( UINTN AddressCells, SizeCells; UINT64 CurBase; UINT64 CurSize; + UINT64 Attributes; Status = gBS->LocateProtocol (&gFdtClientProtocolGuid, NULL, (VOID **)&FdtClient); @@ -77,8 +78,21 @@ InitializeHighMemDxe ( continue; } - Status = gDS->SetMemorySpaceAttributes (CurBase, CurSize, - EFI_MEMORY_WB); + // + // Take care not to strip any permission attributes that will have been + // set by DxeCore on the region we just added if a strict permission + // policy is in effect for EfiConventionalMemory regions. + // Unfortunately, we cannot interrogate the GCD memory space map for + // those permissions, since they are not recorded there (for historical + // reasons), so check the policy directly. + // + Attributes = EFI_MEMORY_WB; + if ((PcdGet64 (PcdDxeNxMemoryProtectionPolicy) & + (1UL << (UINT32)EfiConventionalMemory)) != 0) { + Attributes |= EFI_MEMORY_XP; + } + + Status = gDS->SetMemorySpaceAttributes (CurBase, CurSize, Attributes); if (EFI_ERROR (Status)) { DEBUG ((EFI_D_ERROR, diff --git a/ArmVirtPkg/HighMemDxe/HighMemDxe.inf b/ArmVirtPkg/HighMemDxe/HighMemDxe.inf index 3661cfd8c80c..89c743ebe058 100644 --- a/ArmVirtPkg/HighMemDxe/HighMemDxe.inf +++ b/ArmVirtPkg/HighMemDxe/HighMemDxe.inf @@ -45,6 +45,7 @@ [Protocols] [Pcd] gArmTokenSpaceGuid.PcdSystemMemoryBase + gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy [Depex] gEfiCpuArchProtocolGuid AND gFdtClientProtocolGuid