From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f46.google.com (mail-pj1-f46.google.com [209.85.216.46]) by mx.groups.io with SMTP id smtpd.web10.27136.1688657227067074831 for ; Thu, 06 Jul 2023 08:27:07 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="signature has expired" header.i=@gmail.com header.s=20221208 header.b=rlQRrmO9; spf=pass (domain: gmail.com, ip: 209.85.216.46, mailfrom: joey.vagedes@gmail.com) Received: by mail-pj1-f46.google.com with SMTP id 98e67ed59e1d1-2640a8ceefdso626682a91.3 for ; Thu, 06 Jul 2023 08:27:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1688657226; x=1691249226; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=1Dm4259ilbgVTVUmrPFBgfCg1NDOdXAPUEHckz4JTGA=; b=rlQRrmO9zMrOVAcCySMsAu2ywrLwJzHzyUy+r6MO83SosYB6122881svenV5MxT7sp Lw9h1jBt98CZQ8lx4gIqklMPRBWctZyuQkMQZ/l5B+qmSEhSg6ez6LTOh6hRJKR0xzJY EWPXRs9oTb0e7c3tDVMvwugb4u1yMgEINOliLgDYNZjIyU5Fidm1csZUSbzBT3M3r/3I YrOeAEWaRfbujj2lwP6s4X4HzdgMvuRbb+duGUBU8FvdTnYZpi6XegIojzNNvBjD/aFS wNWLeVqRv6CxLSLjQnBOIH8dXe5Aoo7YHcz7zSHJYfCarNOlbA45gh4xk54oFpH+JD6b uLPQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1688657226; x=1691249226; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=1Dm4259ilbgVTVUmrPFBgfCg1NDOdXAPUEHckz4JTGA=; b=GkwM2OB1UM2RnOcyGKQuQwQWw0Eq3MAR1p2UOuZp0jCNoIjDgL+al5Cnn1OIpenRoJ A0RuUymJVA83KPkLoCGNLMIvS5GZWRwpiA/kimWGOsXDXkNpVnBtvffJTN5IueEBfFYC 1YmGFW/iBJK7Fz16mUr0Hes0LqCDCwwjTy1nuxzjCDGiPf5hG3MkETEgVecTxc3wnQsU +BH/QR/bBxFWCRG1ssc/RWavRvmAeNORFXhakbs3dzBjekdt1LAxN4Y6KdeTFEnWU59R MJgZpQjgxDkdUSQZdBJZhZglLBIFbhTE5cYFaTERwPsNy5Kz/zXFYk0jKoUC90j4p0oj TsNQ== X-Gm-Message-State: ABy/qLYF+ybfKmnb5T4LyfjHxW+FnP2w2u7L+sycbV/wDHK9u+Q73fAL FFLNW9J7DE2m/HIt7L8UCFMqf4sFj/Qv1edMRyN87/LoGyc+bw== X-Google-Smtp-Source: APBJJlFZuFCUbWxG8q5UEGv7F+HIN6fiiIdpPW2B1cf+ddpn/OQ1gVWOC8nrs++w34AnNaFbJIeRpqmd9WuA+4hQMH4= X-Received: by 2002:a17:90a:6b85:b0:263:5033:9f36 with SMTP id w5-20020a17090a6b8500b0026350339f36mr1942762pjj.24.1688657226080; Thu, 06 Jul 2023 08:27:06 -0700 (PDT) MIME-Version: 1.0 References: <20230623154442.799-1-joey.vagedes@gmail.com> <20230623154442.799-3-joey.vagedes@gmail.com> In-Reply-To: <20230623154442.799-3-joey.vagedes@gmail.com> From: "Joey Vagedes" Date: Thu, 6 Jul 2023 08:26:53 -0700 Message-ID: Subject: Re: [PATCH v1 2/2] BaseTools: GenFw: auto-set nxcompat flag To: devel@edk2.groups.io Cc: Rebecca Cran , Liming Gao , Bob Feng , Yuwei Chen Content-Type: multipart/alternative; boundary="0000000000001baa9d05ffd32666" --0000000000001baa9d05ffd32666 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi all, Do you have any concerns over the changes I've made to GenFw.c as seen above? Please let me know if you have any questions, concerns, or improvements; I would be happy to help! Thanks, Joey On Fri, Jun 23, 2023 at 8:44=E2=80=AFAM Joey Vagedes wrote: > Automatically set the nxcompat flag in the DLL Characteristics field of > the Optional Header of the PE32+ image. For this flag to be set > automatically, it must, the section alignment must be evenly divisible > by 4K (EFI_PAGE_SIZE) and no section must be executable and writable. > > Cc: Rebecca Cran > Cc: Liming Gao > Cc: Bob Feng > Cc: Yuwei Chen > Signed-off-by: Joey Vagedes > --- > BaseTools/Source/C/GenFw/GenFw.c | 59 ++++++++++++++++++++ > 1 file changed, 59 insertions(+) > > diff --git a/BaseTools/Source/C/GenFw/GenFw.c > b/BaseTools/Source/C/GenFw/GenFw.c > index 0289c8ef8a5c..4581c4233c14 100644 > --- a/BaseTools/Source/C/GenFw/GenFw.c > +++ b/BaseTools/Source/C/GenFw/GenFw.c > @@ -441,6 +441,60 @@ Returns: > return STATUS_SUCCESS; > } > > +STATIC > +BOOLEAN > +IsNxCompatCompliant ( > + EFI_IMAGE_OPTIONAL_HEADER_UNION *PeHdr > + ) > +/*++ > + > +Routine Description: > + > + Checks if the Pe image is nxcompat. i.e. PE is 64bit, section alignmen= t > is > + evenly divisible by 4k, and no section is writable and executable. > + > +Arguments: > + > + PeHdr The Pe header > + > +Returns: > + TRUE The PE is nx compat compliant > + FALSE The PE is not nx compat compliant > + > +--*/ > +{ > + EFI_IMAGE_SECTION_HEADER *SectionHeader; > + UINT32 Index; > + UINT32 Mask; > + > + // Must have an optional header to perform verification > + if (PeHdr->Pe32.FileHeader.SizeOfOptionalHeader =3D=3D 0) { > + return FALSE; > + } > + > + // Verify PE is 64 bit > + if (!(PeHdr->Pe32.OptionalHeader.Magic =3D=3D > EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC)) { > + return FALSE; > + } > + > + // Verify Section Alignment is divisible by 4K > + if (!((PeHdr->Pe32Plus.OptionalHeader.SectionAlignment % EFI_PAGE_SIZE= ) > =3D=3D 0)) { > + return FALSE; > + } > + > + // Verify sections are not Write & Execute > + Mask =3D EFI_IMAGE_SCN_MEM_EXECUTE | EFI_IMAGE_SCN_MEM_WRITE; > + SectionHeader =3D (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *) > &(PeHdr->Pe32Plus.OptionalHeader) + > PeHdr->Pe32Plus.FileHeader.SizeOfOptionalHeader); > + for (Index =3D 0; Index < PeHdr->Pe32Plus.FileHeader.NumberOfSections; > Index ++, SectionHeader ++) { > + if ((SectionHeader->Characteristics & Mask) =3D=3D Mask) { > + return FALSE; > + } > + } > + > + // Passed all requirements, return TRUE > + return TRUE; > +} > + > VOID > SetHiiResourceHeader ( > UINT8 *HiiBinData, > @@ -2458,6 +2512,11 @@ Returns: > TEImageHeader.BaseOfCode =3D Optional64->BaseOfCode; > TEImageHeader.ImageBase =3D (UINT64) (Optional64->ImageBas= e); > > + // Set NxCompat flag > + if (IsNxCompatCompliant (PeHdr)) { > + Optional64->DllCharacteristics |=3D > IMAGE_DLLCHARACTERISTICS_NX_COMPAT; > + } > + > if (Optional64->NumberOfRvaAndSizes > > EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC) { > > TEImageHeader.DataDirectory[EFI_TE_IMAGE_DIRECTORY_ENTRY_BASERELOC].Virt= ualAddress > =3D > Optional64->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC].VirtualAdd= ress; > > TEImageHeader.DataDirectory[EFI_TE_IMAGE_DIRECTORY_ENTRY_BASERELOC].Size= =3D > Optional64->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC].Size; > -- > 2.41.0.windows.1 > > --0000000000001baa9d05ffd32666 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi all,

Do you have any concerns over t= he changes I've made to GenFw.c as seen above? Please let me know if yo= u have any questions, concerns, or improvements; I would be happy to help!<= /div>

Thanks,
Joey

On Fri, Jun 23, 2023= at 8:44=E2=80=AFAM Joey Vagedes <joey.vagedes@gmail.com> wrote:
Automatically set the nxcompat flag in the DLL Ch= aracteristics field of
the Optional Header of the PE32+ image. For this flag to be set
automatically, it must, the section alignment must be evenly divisible
by 4K (EFI_PAGE_SIZE) and no section must be executable and writable.

Cc: Rebecca Cran <rebecca@bsdio.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Bob Feng <= bob.c.feng@intel.com>
Cc: Yuwei Chen <yuwei.chen@intel.com>
Signed-off-by: Joey Vagedes <joeyvagedes@gmail.com>
---
=C2=A0BaseTools/Source/C/GenFw/GenFw.c | 59 ++++++++++++++++++++
=C2=A01 file changed, 59 insertions(+)

diff --git a/BaseTools/Source/C/GenFw/GenFw.c b/BaseTools/Source/C/GenFw/Ge= nFw.c
index 0289c8ef8a5c..4581c4233c14 100644
--- a/BaseTools/Source/C/GenFw/GenFw.c
+++ b/BaseTools/Source/C/GenFw/GenFw.c
@@ -441,6 +441,60 @@ Returns:
=C2=A0 =C2=A0return STATUS_SUCCESS;
=C2=A0}

+STATIC
+BOOLEAN
+IsNxCompatCompliant (
+=C2=A0 EFI_IMAGE_OPTIONAL_HEADER_UNION=C2=A0 *PeHdr
+=C2=A0 )
+/*++
+
+Routine Description:
+
+=C2=A0 Checks if the Pe image is nxcompat. i.e. PE is 64bit, section align= ment is
+=C2=A0 evenly divisible by 4k, and no section is writable and executable.<= br> +
+Arguments:
+
+=C2=A0 PeHdr=C2=A0 =C2=A0 =C2=A0 The Pe header
+
+Returns:
+=C2=A0 TRUE=C2=A0 =C2=A0 =C2=A0 =C2=A0The PE is nx compat compliant
+=C2=A0 FALSE=C2=A0 =C2=A0 =C2=A0 The PE is not nx compat compliant
+
+--*/
+{
+=C2=A0 EFI_IMAGE_SECTION_HEADER=C2=A0 =C2=A0 =C2=A0*SectionHeader;
+=C2=A0 UINT32=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0Index;
+=C2=A0 UINT32=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0Mask;
+
+=C2=A0 // Must have an optional header to perform verification
+=C2=A0 if (PeHdr->Pe32.FileHeader.SizeOfOptionalHeader =3D=3D 0) {
+=C2=A0 =C2=A0 return FALSE;
+=C2=A0 }
+
+=C2=A0 // Verify PE is 64 bit
+=C2=A0 if (!(PeHdr->Pe32.OptionalHeader.Magic =3D=3D EFI_IMAGE_NT_OPTIO= NAL_HDR64_MAGIC)) {
+=C2=A0 =C2=A0 return FALSE;
+=C2=A0 }
+
+=C2=A0 // Verify Section Alignment is divisible by 4K
+=C2=A0 if (!((PeHdr->Pe32Plus.OptionalHeader.SectionAlignment % EFI_PAG= E_SIZE) =3D=3D 0)) {
+=C2=A0 =C2=A0 return FALSE;
+=C2=A0 }
+
+=C2=A0 // Verify sections are not Write & Execute
+=C2=A0 Mask =3D EFI_IMAGE_SCN_MEM_EXECUTE | EFI_IMAGE_SCN_MEM_WRITE;
+=C2=A0 SectionHeader =3D (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *) &(PeH= dr->Pe32Plus.OptionalHeader) + PeHdr->Pe32Plus.FileHeader.SizeOfOptio= nalHeader);
+=C2=A0 for (Index =3D 0; Index < PeHdr->Pe32Plus.FileHeader.NumberOf= Sections; Index ++, SectionHeader ++) {
+=C2=A0 =C2=A0 if ((SectionHeader->Characteristics & Mask) =3D=3D Ma= sk) {
+=C2=A0 =C2=A0 =C2=A0 return FALSE;
+=C2=A0 =C2=A0 }
+=C2=A0 }
+
+=C2=A0 // Passed all requirements, return TRUE
+=C2=A0 return TRUE;
+}
+
=C2=A0VOID
=C2=A0SetHiiResourceHeader (
=C2=A0 =C2=A0UINT8=C2=A0 =C2=A0*HiiBinData,
@@ -2458,6 +2512,11 @@ Returns:
=C2=A0 =C2=A0 =C2=A0TEImageHeader.BaseOfCode=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =3D Optional64->BaseOfCode;
=C2=A0 =C2=A0 =C2=A0TEImageHeader.ImageBase=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0=3D (UINT64) (Optional64->ImageBase);

+=C2=A0 =C2=A0 // Set NxCompat flag
+=C2=A0 =C2=A0 if (IsNxCompatCompliant (PeHdr)) {
+=C2=A0 =C2=A0 =C2=A0 Optional64->DllCharacteristics |=3D IMAGE_DLLCHARA= CTERISTICS_NX_COMPAT;
+=C2=A0 =C2=A0 }
+
=C2=A0 =C2=A0 =C2=A0if (Optional64->NumberOfRvaAndSizes > EFI_IMAGE_D= IRECTORY_ENTRY_BASERELOC) {
=C2=A0 =C2=A0 =C2=A0 =C2=A0TEImageHeader.DataDirectory[EFI_TE_IMAGE_DIRECTO= RY_ENTRY_BASERELOC].VirtualAddress =3D Optional64->DataDirectory[EFI_IMA= GE_DIRECTORY_ENTRY_BASERELOC].VirtualAddress;
=C2=A0 =C2=A0 =C2=A0 =C2=A0TEImageHeader.DataDirectory[EFI_TE_IMAGE_DIRECTO= RY_ENTRY_BASERELOC].Size =3D Optional64->DataDirectory[EFI_IMAGE_DIRECTO= RY_ENTRY_BASERELOC].Size;
--
2.41.0.windows.1

--0000000000001baa9d05ffd32666--