From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by mx.groups.io with SMTP id smtpd.web11.28505.1688945066393741669 for ; Sun, 09 Jul 2023 16:24:26 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="signature has expired" header.i=@bsdio.com header.s=fm3 header.b=VJJr0nUY; spf=pass (domain: bsdio.com, ip: 66.111.4.26, mailfrom: rebecca@bsdio.com) Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.nyi.internal (Postfix) with ESMTP id E556B5C00A2; Sun, 9 Jul 2023 19:24:24 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute5.internal (MEProxy); Sun, 09 Jul 2023 19:24:24 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdio.com; h=cc :cc:content-transfer-encoding:content-type:content-type:date :date:from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:sender:subject:subject:to:to; s=fm3; t= 1688945064; x=1689031464; bh=VAEogvOyUi60klIqiGJUxao95pc2xKFHdQC RBpp0TOg=; b=VJJr0nUYpMkr/QMzDneNJZFTqbyyfJImSBM22Dl2jcOBOudQisT fw5bgHTsCyyjWxMwIlX5+44i6Z/b7PASCknyexHxFnhIq7l5zmY2HC+ND7KTTnAa VhqcZgQKEY+/F8Llzwg/rpohDs+oGeqkSW6FHCrTIIrSMoKSrHvXQb3buCMKK/Gj Wj3ew03QViaGGnzLaupHBAjgWlIG5EAS2L02mJEFixu5DlnLwYnCC0GaFytMM8n0 5Yl1oLyhkUnM8DAG1R9wQvSXOye1RuAkYTj24FR453B1tE1YSQ+yeHkEL+VUtxQW qFonoWqiLRRLSmEsXjAREcfgp+M17eqTQXQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:sender:subject:subject:to:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t= 1688945064; x=1689031464; bh=VAEogvOyUi60klIqiGJUxao95pc2xKFHdQC RBpp0TOg=; b=Xj6eApfZuHupVxTG1OCH7+Ik8kj4uzYMUzPhYNF2hwjRP/EBm0z LeqX58iwRfr94eIx5sECRl48Qeh+vy/K73KQSqgHxQG5JlaBxgsmLVigjFWi6Vy0 E/qUIbt/7SJgwg0YiZqQbWzJG1wa+yt1Yalh6HJ1badziJSkj6FQHqGTZ+dfquMT u+6yzprBRBMKuSzdJXBXD7B4u/Is0pi7oQ/ediNXJ+4RuD/2fXCQN+DJkcomOwHy MgyPnueflrD8DvPL6HzUs1dkicZDqONkodJkH/RiNPV3bCR7N7yXRMyT4iKpvMHq Ms93A0BG2nbA8e/XCmPY6PpiqCv/8D11ftw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedviedrvdeigddvfecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefkffggfgfuvfevfhfhjggtgfesthekredttddvjeenucfhrhhomheptfgvsggv tggtrgcuvehrrghnuceorhgvsggvtggtrgessghsughiohdrtghomheqnecuggftrfgrth htvghrnheplefhgeevleehieeuveduudfguedtieetteevhffgfffhhffhveffueegtdeg keeknecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomheprh gvsggvtggtrgessghsughiohdrtghomh X-ME-Proxy: Feedback-ID: i5b994698:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Sun, 9 Jul 2023 19:24:23 -0400 (EDT) Message-ID: <316ec4da-bfde-a491-27b8-3f85a5643ac5@bsdio.com> Date: Sun, 9 Jul 2023 17:24:18 -0600 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v1 2/2] BaseTools: GenFw: auto-set nxcompat flag To: Joey Vagedes , devel@edk2.groups.io Cc: Liming Gao , Bob Feng , Yuwei Chen References: <20230623154442.799-1-joey.vagedes@gmail.com> <20230623154442.799-3-joey.vagedes@gmail.com> From: "Rebecca Cran" In-Reply-To: Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Please fix the documentation block. It should go above the function, and use Doxygen style. Also, the commit message doesn't make sense to me - specifically ", it must," -- Rebecca Cran On 7/6/23 9:26 AM, Joey Vagedes wrote: > 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 AM 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 > alignment 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 == 0) { > +    return FALSE; > +  } > + > +  // Verify PE is 64 bit > +  if (!(PeHdr->Pe32.OptionalHeader.Magic == > EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC)) { > +    return FALSE; > +  } > + > +  // Verify Section Alignment is divisible by 4K > +  if (!((PeHdr->Pe32Plus.OptionalHeader.SectionAlignment % > EFI_PAGE_SIZE) == 0)) { > +    return FALSE; > +  } > + > +  // Verify sections are not Write & Execute > +  Mask = EFI_IMAGE_SCN_MEM_EXECUTE | EFI_IMAGE_SCN_MEM_WRITE; > +  SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *) > &(PeHdr->Pe32Plus.OptionalHeader) + > PeHdr->Pe32Plus.FileHeader.SizeOfOptionalHeader); > +  for (Index = 0; Index < > PeHdr->Pe32Plus.FileHeader.NumberOfSections; Index ++, > SectionHeader ++) { > +    if ((SectionHeader->Characteristics & Mask) == Mask) { > +      return FALSE; > +    } > +  } > + > +  // Passed all requirements, return TRUE > +  return TRUE; > +} > + >  VOID >  SetHiiResourceHeader ( >    UINT8   *HiiBinData, > @@ -2458,6 +2512,11 @@ Returns: >      TEImageHeader.BaseOfCode          = Optional64->BaseOfCode; >      TEImageHeader.ImageBase           = (UINT64) > (Optional64->ImageBase); > > +    // Set NxCompat flag > +    if (IsNxCompatCompliant (PeHdr)) { > +      Optional64->DllCharacteristics |= > IMAGE_DLLCHARACTERISTICS_NX_COMPAT; > +    } > + >      if (Optional64->NumberOfRvaAndSizes > > EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC) { >  TEImageHeader.DataDirectory[EFI_TE_IMAGE_DIRECTORY_ENTRY_BASERELOC].VirtualAddress > = > Optional64->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC].VirtualAddress; >  TEImageHeader.DataDirectory[EFI_TE_IMAGE_DIRECTORY_ENTRY_BASERELOC].Size > = Optional64->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC].Size; > -- > 2.41.0.windows.1 >