From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4864:20::d41; helo=mail-io1-xd41.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io1-xd41.google.com (mail-io1-xd41.google.com [IPv6:2607:f8b0:4864:20::d41]) (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 4AD832117D764 for ; Mon, 14 Jan 2019 06:59:20 -0800 (PST) Received: by mail-io1-xd41.google.com with SMTP id t24so17840329ioi.0 for ; Mon, 14 Jan 2019 06:59:20 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=F1aYBxO83Ic+Cj3YG59XIRujrQlavzvxKLomhRbkK/4=; b=OdmMXMulcdjeYhsxCD5x59yvrvEBlMNabMQ9tRu7B9S+C5A2tkqTAg1ALb/KAl359P T8H5CrwBNL5gfpBdnLuPnmRgIOV42W/uYiB5g316c7aIjxbV9s5ETQYFnmKEdVLJfkHx FRvf7eOeYz270JGqVnB1SvLECDQvYM8arRW0Y= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=F1aYBxO83Ic+Cj3YG59XIRujrQlavzvxKLomhRbkK/4=; b=cBR7sbjMeqCmrbWTA+YeXSto9heHWrey6TkgdXb55+W1dhwr6eNHbqmPTyfXoqTCNH 3QLG47OTBNqY7e/uQWRTFSPOojuvm78OpBhBKQkOoCCkJvXb6OkWx2QZNzp0ILY0XgO1 ui4ydXP8+ge00BTHxuvnfBio1Q7OZ9V1cWqHz/i41O4FLLB78moK2+KSXsxELX7cHhpT TmH4LoAIfIiuaSX8u2CTD0votMrgKsP9ZOJsX1prUcod5tOoFn1sMcqlyAuIsAiejutW wPZ3030gqiOTYEoAHHwAGylVRPiUKicaiL5w4Najv1uWZ2JQrxpy4QedFz5cCdqBcVxE Ksfg== X-Gm-Message-State: AJcUukdhSopyI+uZHLTZz+b1ge7FaNwJ75O7o+2XE+1SQFAFwvAbLFVu wlzf6WBmqMrxGA8DaXnKNdyXyTgqJ/4jw4hWxpJ1VsLJiV8= X-Google-Smtp-Source: ALg8bN6VNX+UpCJxaZn53CmbrLkL6iuIHPg4uTsw8uFhxh0kYCW6U6fWs6cJOTJ+/+dFxQpzzKz8t2gvEf0wFlQ3fmU= X-Received: by 2002:a6b:5d01:: with SMTP id r1mr15627205iob.170.1547477959508; Mon, 14 Jan 2019 06:59:19 -0800 (PST) MIME-Version: 1.0 References: <20190107071504.2431-1-ard.biesheuvel@linaro.org> <20190107071504.2431-4-ard.biesheuvel@linaro.org> <20190114142901.lkwgte6mlsv6bjyk@bivouac.eciton.net> In-Reply-To: <20190114142901.lkwgte6mlsv6bjyk@bivouac.eciton.net> From: Ard Biesheuvel Date: Mon, 14 Jan 2019 15:59:08 +0100 Message-ID: To: Leif Lindholm Cc: Charles Garcia-Tobin , "edk2-devel@lists.01.org" Subject: Re: [PATCH 3/5] ArmPkg/ArmMmuLib AARCH64: implement support for EFI_MEMORY_RP permissions X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 14 Jan 2019 14:59:20 -0000 Content-Type: text/plain; charset="UTF-8" On Mon, 14 Jan 2019 at 15:29, Leif Lindholm wrote: > > On Mon, Jan 07, 2019 at 08:15:02AM +0100, Ard Biesheuvel wrote: > > Wire up the access flag (AF) page table attribute to the EFI_MEMORY_RP > > permission attribute, so that attempts to read from such a region will > > trigger an access flag fault. > > > > Note that this is a stronger notion than just read protection, since > > it now implies that any write or execute attempt is trapped as well. > > However, this does not really matter in practice since we never assume > > that a read protected page is writable or executable, and StackGuard > > and HeapGuard (which are the primary users of this facility) certainly > > don't care. > > So ... I'm cautiously positive to this patch. > But this use does contradict the UEFI spec (2.7a, 2.3.6.1 Memory > types), which says EFI_MEMORY_RP is "not used or defined" for AArch64. > > Charles? > Not defined by the spec means we can use it do whatever we bloody want with it, at least that is what a typical compiler engineer will tell you :-) I think there was a pending ECR to update the AArch64 binding code to reflect reality, but I don't think we included EFI_MEMORY_RP. > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Ard Biesheuvel > > --- > > ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c | 5 +++-- > > ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 14 +++++++++++--- > > 2 files changed, 14 insertions(+), 5 deletions(-) > > > > diff --git a/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c b/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c > > index 3e216c7cb235..e62e3fa87112 100644 > > --- a/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c > > +++ b/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c > > @@ -223,8 +223,9 @@ EfiAttributeToArmAttribute ( > > ArmAttributes = TT_ATTR_INDX_MASK; > > } > > > > - // Set the access flag to match the block attributes > > - ArmAttributes |= TT_AF; > > + if ((EfiAttributes & EFI_MEMORY_RP) == 0) { > > + ArmAttributes |= TT_AF; > > + } > > > > // Determine protection attributes > > if (EfiAttributes & EFI_MEMORY_RO) { > > diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c > > index e1fabfcbea14..b59c081a7e49 100644 > > --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c > > +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c > > @@ -102,6 +102,10 @@ PageAttributeToGcdAttribute ( > > GcdAttributes |= EFI_MEMORY_XP; > > } > > > > + if ((PageAttributes & TT_AF) == 0) { > > + GcdAttributes |= EFI_MEMORY_RP; > > + } > > + > > return GcdAttributes; > > } > > > > @@ -451,7 +455,11 @@ GcdAttributeToPageAttribute ( > > PageAttributes |= TT_AP_RO_RO; > > } > > > > - return PageAttributes | TT_AF; > > + if ((GcdAttributes & EFI_MEMORY_RP) == 0) { > > + PageAttributes |= TT_AF; > > + } > > + > > + return PageAttributes; > > } > > > > EFI_STATUS > > @@ -474,9 +482,9 @@ ArmSetMemoryAttributes ( > > // No memory type was set in Attributes, so we are going to update the > > // permissions only. > > // > > - PageAttributes &= TT_AP_MASK | TT_UXN_MASK | TT_PXN_MASK; > > + PageAttributes &= TT_AP_MASK | TT_UXN_MASK | TT_PXN_MASK | TT_AF; > > PageAttributeMask = ~(TT_ADDRESS_MASK_BLOCK_ENTRY | TT_AP_MASK | > > - TT_PXN_MASK | TT_XN_MASK); > > + TT_PXN_MASK | TT_XN_MASK | TT_AF); > > } > > > > TranslationTable = ArmGetTTBR0BaseAddress (); > > -- > > 2.20.1 > >