public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Jian J Wang <jian.j.wang@intel.com>
To: edk2-devel@lists.01.org
Cc: Star Zeng <star.zeng@intel.com>, Laszlo Ersek <lersek@redhat.com>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Ruiyu Ni <ruiyu.ni@intel.com>, Jiewen Yao <jiewen.yao@intel.com>
Subject: [PATCH v2 1/2] MdeModulePkg/MdeModulePkg.dec/.uni: clarify PCDs usage
Date: Thu, 20 Sep 2018 14:02:46 +0800	[thread overview]
Message-ID: <20180920060247.7764-2-jian.j.wang@intel.com> (raw)
In-Reply-To: <20180920060247.7764-1-jian.j.wang@intel.com>

> v2 changes:
>    Newly added patch to clarify PCDs usage.

BZ#1116: https://bugzilla.tianocore.org/show_bug.cgi?id=1116

The usage of following PCDs described in MdeModulePkg.dec don't match
the implementation exactly. This patch updates related description in
both .dec and .uni files to avoid confusion in platform configuration.

  PcdSetNxForStack
  PcdImageProtectionPolicy
  PcdDxeNxMemoryProtectionPolicy

The main change is at the statement on how to handle the FALSE or 0
setting value in those PCDs. Current statement says the implementation
should unset or disable related features but in fact the related code
just do nothing (leave it to AS-IS). That means the result might be
disabled, or might be not. It depends on other features or platform
policy.

For example, if one don't want to enforce NX onto stack memory, he/she
needs to set PcdSetNxForStack to FALSE as well as to clear BIT4 of
PcdDxeNxMemoryProtectionPolicy.

Cc: Star Zeng <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
---
 MdeModulePkg/MdeModulePkg.dec | 20 +++++++++++++++-----
 MdeModulePkg/MdeModulePkg.uni | 13 +++++++++----
 2 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 74a699cbb7..6566b57fd4 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1288,17 +1288,23 @@
   ## Set image protection policy. The policy is bitwise.
   #  If a bit is set, the image will be protected by DxeCore if it is aligned.
   #   The code section becomes read-only, and the data section becomes non-executable.
-  #  If a bit is clear, the image will not be protected.<BR><BR>
+  #  If a bit is clear, nothing will be done to image code/data sections.<BR><BR>
   #    BIT0       - Image from unknown device. <BR>
   #    BIT1       - Image from firmware volume.<BR>
+  #  <BR>
+  #  Note: If a bit is cleared, the data section could be still non-executable if
+  #  PcdDxeNxMemoryProtectionPolicy is enabled for EfiLoaderData, EfiBootServicesData
+  #  and/or EfiRuntimeServicesData.<BR>
+  #  <BR>
   # @Prompt Set image protection policy.
   # @ValidRange 0x80000002 | 0x00000000 - 0x0000001F
   gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x00000002|UINT32|0x00001047
 
   ## Set DXE memory protection policy. The policy is bitwise.
   #  If a bit is set, memory regions of the associated type will be mapped
-  #  non-executable.<BR><BR>
-  #
+  #  non-executable.<BR>
+  #  If a bit is cleared, nothing will be done to associated type of memory.<BR>
+  #  <BR>
   # Below is bit mask for this PCD: (Order is same as UEFI spec)<BR>
   #  EfiReservedMemoryType          0x0001<BR>
   #  EfiLoaderCode                  0x0002<BR>
@@ -1890,8 +1896,12 @@
   #  For the DxeIpl and the DxeCore are both X64, set NX for stack feature also require PcdDxeIplBuildPageTables be TRUE.<BR>
   #  For the DxeIpl and the DxeCore are both IA32 (PcdDxeIplSwitchToLongMode is FALSE), set NX for stack feature also require
   #  IA32 PAE is supported and Execute Disable Bit is available.<BR>
-  #   TRUE  - to set NX for stack.<BR>
-  #   FALSE - Not to set NX for stack.<BR>
+  #  <BR>
+  #  Note: If this PCD is set to FALSE, NX could be still applied to stack due to PcdDxeNxMemoryProtectionPolicy enabled for
+  #  EfiBootServicesData.<BR>
+  #  <BR>
+  #   TRUE  - Set NX for stack.<BR>
+  #   FALSE - Do nothing for stack.<BR>
   # @Prompt Set NX for stack.
   gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|FALSE|BOOLEAN|0x0001006f
 
diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni
index 080b8a62c0..61befdc1e4 100644
--- a/MdeModulePkg/MdeModulePkg.uni
+++ b/MdeModulePkg/MdeModulePkg.uni
@@ -345,8 +345,9 @@
                                                                                   "For the DxeIpl and the DxeCore are both X64, set NX for stack feature also require PcdDxeIplBuildPageTables be TRUE.<BR>"
                                                                                   "For the DxeIpl and the DxeCore are both IA32 (PcdDxeIplSwitchToLongMode is FALSE), set NX for stack feature also require"
                                                                                   "IA32 PAE is supported and Execute Disable Bit is available.<BR>"
-                                                                                  "TRUE  - to set NX for stack.<BR>"
-                                                                                  "FALSE - Not to set NX for stack.<BR>"
+                                                                                  "Note: If this PCD is set to FALSE, NX could be still applied to stack due to PcdDxeNxMemoryProtectionPolicy enabled for EfiBootServicesData.<BR>"
+                                                                                  "TRUE  - Set NX for stack.<BR>"
+                                                                                  "FALSE - Do nothing for stack.<BR>"
 
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdAcpiS3Enable_PROMPT  #language en-US "ACPI S3 Enable"
 
@@ -1098,15 +1099,19 @@
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdImageProtectionPolicy_HELP  #language en-US "Set image protection policy. The policy is bitwise.\n"
                                                                                           "If a bit is set, the image will be protected by DxeCore if it is aligned.\n"
                                                                                           "The code section becomes read-only, and the data section becomes non-executable.\n"
-                                                                                          "If a bit is clear, the image will not be protected.<BR><BR>\n"
+                                                                                          "If a bit is clear, nothing will be done to image code/data sections.<BR><BR>\n"
                                                                                           "BIT0       - Image from unknown device. <BR>\n"
                                                                                           "BIT1       - Image from firmware volume.<BR>"
+                                                                                          "Note: If a bit is cleared, the data section could be still non-executable if\n"
+                                                                                          "PcdDxeNxMemoryProtectionPolicy is enabled for EfiLoaderData, EfiBootServicesData\n"
+                                                                                          "and/or EfiRuntimeServicesData.<BR>"
 
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdDxeNxMemoryProtectionPolicy_PROMPT  #language en-US "Set DXE memory protection policy."
 
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdDxeNxMemoryProtectionPolicy_HELP  #language en-US "Set DXE memory protection policy. The policy is bitwise.\n"
                                                                                                 "If a bit is set, memory regions of the associated type will be mapped\n"
-                                                                                                "non-executable.<BR><BR>\n"
+                                                                                                "non-executable.<BR>\n"
+                                                                                                "If a bit is cleared, nothing will be done to associated type of memory.<BR><BR>\n"
                                                                                                 "\n"
                                                                                                 "Below is bit mask for this PCD: (Order is same as UEFI spec)<BR>\n"
                                                                                                 "EfiReservedMemoryType          0x0001<BR>\n"
-- 
2.16.2.windows.1



  reply	other threads:[~2018-09-20  6:03 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-20  6:02 [PATCH v2 0/2] clarify NXE enabling logic Jian J Wang
2018-09-20  6:02 ` Jian J Wang [this message]
2018-09-21  5:49   ` [PATCH v2 1/2] MdeModulePkg/MdeModulePkg.dec/.uni: clarify PCDs usage Zeng, Star
2018-09-20  6:02 ` [PATCH v2 2/2] MdeModulePkg/DxeIpl: support more NX related PCDs Jian J Wang
2018-09-21  6:00   ` Zeng, Star
2018-09-21  8:42     ` Zeng, Star
2018-09-21 10:14       ` Laszlo Ersek
2018-09-25  3:23         ` Wang, Jian J
2018-09-20 11:31 ` [PATCH v2 0/2] clarify NXE enabling logic Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180920060247.7764-2-jian.j.wang@intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox