From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail1.protonmail.ch (mail1.protonmail.ch [185.70.40.18]) by mx.groups.io with SMTP id smtpd.web09.2478.1571731379738539386 for ; Tue, 22 Oct 2019 01:03:01 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@protonmail.com header.s=default header.b=Kdm0z2qR; spf=pass (domain: protonmail.com, ip: 185.70.40.18, mailfrom: vit9696@protonmail.com) Date: Tue, 22 Oct 2019 08:02:54 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=default; t=1571731377; bh=TDIZ8kowCbGnT/UYnY4LSuEHib5a/mPSwuPh+GAIp2Q=; h=Date:To:From:Cc:Reply-To:Subject:In-Reply-To:References: Feedback-ID:From; b=Kdm0z2qROWIYcCZ0vMQShKvaPWnbUI4gxUQ9hfPfswwRBH+sK2Uj/7QhFQIqWdI2D /0O6vWagSEyqgKUuScuwGTK5bOFe71eEJa4TIMMr32fIy/fIEQp+wtmq6Q0R8EjsV9 qViVhRl6dNUCSMczfyhVJyBBxeNoa+BBN0BPFx5k= To: "Yao, Jiewen" From: "Vitaly Cheptsov" Cc: "devel@edk2.groups.io" , "Kinney, Michael D" Reply-To: vit9696 Subject: Re: [edk2-devel] [PATCH v3 1/1] MdePkg: Add PCD to disable safe string constraint assertions Message-ID: <4A3AB6A7-BD3F-42F3-97A4-1B893DC778AE@protonmail.com> In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C503F80F5D0@shsmsx102.ccr.corp.intel.com> References: <20191022072022.18826-1-vit9696@protonmail.com> <20191022072022.18826-2-vit9696@protonmail.com> <74D8A39837DF1E4DA445A8C0B3885C503F80F5D0@shsmsx102.ccr.corp.intel.com> Feedback-ID: p9QuX-L1wMgUm6nrSvNrf8juLupNs0VSnzXGVXuYDxlEahFdWtaedWDMB9zpwGDklGt7kzs1-RBc0cqz327Gcg==:Ext:ProtonMail MIME-Version: 1.0 X-Spam-Status: No, score=-0.7 required=7.0 tests=ALL_TRUSTED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM,FREEMAIL_REPLYTO_END_DIGIT autolearn=no autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on mail.protonmail.ch X-Groupsio-MsgNum: 49332 Content-Type: multipart/signed; protocol="application/pgp-signature"; micalg=pgp-sha256; boundary="---------------------5f0bbbd293a088f86f2f73a55deb1161"; charset=UTF-8 -----------------------5f0bbbd293a088f86f2f73a55deb1161 Cc: "devel@edk2.groups.io" , "Kinney, Michael D" Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 Date: Tue, 22 Oct 2019 11:02:51 +0300 From: vit9696 In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C503F80F5D0@shsmsx102.ccr.corp.intel.com> Message-Id: <4A3AB6A7-BD3F-42F3-97A4-1B893DC778AE@protonmail.com> Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.11\)) References: <20191022072022.18826-1-vit9696@protonmail.com> <20191022072022.18826-2-vit9696@protonmail.com> <74D8A39837DF1E4DA445A8C0B3885C503F80F5D0@shsmsx102.ccr.corp.intel.com> Subject: Re: [edk2-devel] [PATCH v3 1/1] MdePkg: Add PCD to disable safe string constraint assertions To: "Yao, Jiewen" X-Mailer: Apple Mail (2.3445.104.11) I agree that we want this PCD mentioned in BaseLib header, but most = likely it should be done in a shorter form. What about something like this? Unless PcdAssertOnSafeStringConstraints is set to FALSE: If String is NULL, then ASSERT(). If Data is NULL, then ASSERT(). If String is not aligned in a 16-bit boundary, then ASSERT(). If PcdMaximumUnicodeStringLength is not zero, and String contains more = than PcdMaximumUnicodeStringLength Unicode characters, not including the Null-terminator, then ASSERT(). Best wishes, Vitaly > 22 =D0=BE=D0=BA=D1=82. 2019 =D0=B3., =D0=B2 10:52, Yao, Jiewen = =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0=B0):= >=20 >=20 > In BaseLib.h, we have below comments in each safe string function = header. > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > If an error would be returned, then the function will also ASSERT(). > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >=20 > As I mentioned earlier, the original purpose is to let the caller = guarantee the correctness of the input. Similar to Microsoft Visual = Studio strcpy_s() behavior. >=20 >=20 > If we decide to change the purpose and change the design, I recommend = we add clarification in the function header as well. >=20 > For example: > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D > The PcdAssertOnSafeStringConstraints is used to control the behavior = of the runtime violation. > When PcdAssertOnSafeStringConstraints is TRUE, if an error would be = returned, then the function will also ASSERT(). > When PcdAssertOnSafeStringConstraints is FALSE, if an error would be = returned, then the function will NOT ASSERT(). > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D >=20 >=20 > Hi Michael D Kinney > Do you have some comment? >=20 > Thank you > Yao Jiewen >=20 >=20 >> -----Original Message----- >> From: devel@edk2.groups.io On Behalf Of Vitaly >> Cheptsov via Groups.Io >> Sent: Tuesday, October 22, 2019 3:20 PM >> To: devel@edk2.groups.io >> Subject: [edk2-devel] [PATCH v3 1/1] MdePkg: Add PCD to disable safe = string >> constraint assertions >>=20 >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2054 >>=20 >> Runtime data checks are not meant to cause debug assertions >> unless explicitly needed by some debug code (thus the PCD) >> as this breaks debug builds validating data with BaseLib. >>=20 >> Signed-off-by: Vitaly Cheptsov > >> --- >> MdePkg/MdePkg.dec | 6 ++++++ >> MdePkg/Library/BaseLib/BaseLib.inf | 11 ++++++----- >> MdePkg/Library/BaseLib/SafeString.c | 4 +++- >> MdePkg/MdePkg.uni | 6 ++++++ >> 4 files changed, 21 insertions(+), 6 deletions(-) >>=20 >> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec >> index 3fd7d1634c..c1671333f6 100644 >> --- a/MdePkg/MdePkg.dec >> +++ b/MdePkg/MdePkg.dec >> @@ -2221,6 +2221,12 @@ [PcdsFixedAtBuild,PcdsPatchableInModule] >> # @Prompt Memory Address of GuidedExtractHandler Table. >>=20 >> = gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|0x1000000 >> |UINT64|0x30001015 >>=20 >> + ## Indicates if safe string constraint violation should = assert.

>> + # TRUE - Safe string constraint violation causes assertion.
>> + # FALSE - Safe string constraint violation does not cause = assertion.
>> + # @Prompt Enable safe string constraint violation assertions. >> + >> gEfiMdePkgTokenSpaceGuid.PcdAssertOnSafeStringConstraints|TRUE|BOOLEA >> N|0x0000002e >> + >> [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx] >> ## This value is used to set the base address of PCI express = hierarchy. >> # @Prompt PCI Express Base Address. >> diff --git a/MdePkg/Library/BaseLib/BaseLib.inf >> b/MdePkg/Library/BaseLib/BaseLib.inf >> index 3586beb0ab..bc98bc6134 100644 >> --- a/MdePkg/Library/BaseLib/BaseLib.inf >> +++ b/MdePkg/Library/BaseLib/BaseLib.inf >> @@ -390,11 +390,12 @@ [LibraryClasses] >> BaseMemoryLib >>=20 >> [Pcd] >> - gEfiMdePkgTokenSpaceGuid.PcdMaximumLinkedListLength ## >> SOMETIMES_CONSUMES >> - gEfiMdePkgTokenSpaceGuid.PcdMaximumAsciiStringLength ## >> SOMETIMES_CONSUMES >> - gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength ## >> SOMETIMES_CONSUMES >> - gEfiMdePkgTokenSpaceGuid.PcdControlFlowEnforcementPropertyMask = ## >> SOMETIMES_CONSUMES >> - gEfiMdePkgTokenSpaceGuid.PcdSpeculationBarrierType ## >> SOMETIMES_CONSUMES >> + gEfiMdePkgTokenSpaceGuid.PcdAssertOnSafeStringConstraints ## >> SOMETIMES_CONSUMES >> + gEfiMdePkgTokenSpaceGuid.PcdMaximumLinkedListLength ## >> SOMETIMES_CONSUMES >> + gEfiMdePkgTokenSpaceGuid.PcdMaximumAsciiStringLength ## >> SOMETIMES_CONSUMES >> + gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength ## >> SOMETIMES_CONSUMES >> + gEfiMdePkgTokenSpaceGuid.PcdControlFlowEnforcementPropertyMask ## >> SOMETIMES_CONSUMES >> + gEfiMdePkgTokenSpaceGuid.PcdSpeculationBarrierType ## >> SOMETIMES_CONSUMES >>=20 >> [FeaturePcd] >> gEfiMdePkgTokenSpaceGuid.PcdVerifyNodeInList ## CONSUMES >> diff --git a/MdePkg/Library/BaseLib/SafeString.c >> b/MdePkg/Library/BaseLib/SafeString.c >> index 7dc03d2caa..56b5e34a8d 100644 >> --- a/MdePkg/Library/BaseLib/SafeString.c >> +++ b/MdePkg/Library/BaseLib/SafeString.c >> @@ -14,7 +14,9 @@ >>=20 >> #define SAFE_STRING_CONSTRAINT_CHECK(Expression, Status) \ >> do { \ >> - ASSERT (Expression); \ >> + if (PcdGetBool (PcdAssertOnSafeStringConstraints)) { \ >> + ASSERT (Expression); \ >> + } \ >> if (!(Expression)) { \ >> return Status; \ >> } \ >> diff --git a/MdePkg/MdePkg.uni b/MdePkg/MdePkg.uni >> index 5c1fa24065..425b66bb43 100644 >> --- a/MdePkg/MdePkg.uni >> +++ b/MdePkg/MdePkg.uni >> @@ -287,6 +287,12 @@ >>=20 >> #string >> STR_gEfiMdePkgTokenSpaceGuid_PcdGuidedExtractHandlerTableAddress_HELP >> #language en-US "This value is used to set the available memory = address to >> store Guided Extract Handlers. The required memory space is decided = by the >> value of PcdMaximumGuidedExtractHandler." >>=20 >> +#string >> STR_gEfiMdePkgTokenSpaceGuid_PcdAssertOnSafeStringConstraints_PROMPT >> #language en-US "Enable safe string constraint violation assertions" >> + >> +#string >> STR_gEfiMdePkgTokenSpaceGuid_PcdAssertOnSafeStringConstraints_HELP >> #language en-US "Indicates if safe string constraint violation should >> assert.

\n" >> + = "TRUE - Safe string constraint >> violation causes assertion.
\n" >> + = "FALSE - Safe string constraint >> violation does not cause assertion.
" >> + >> #string STR_gEfiMdePkgTokenSpaceGuid_PcdPciExpressBaseAddress_PROMPT >> #language en-US "PCI Express Base Address" >>=20 >> #string STR_gEfiMdePkgTokenSpaceGuid_PcdPciExpressBaseAddress_HELP >> #language en-US "This value is used to set the base address of PCI = express >> hierarchy." >> -- >> 2.21.0 (Apple Git-122) >>=20 >>=20 >> -=3D-=3D-=3D-=3D-=3D-=3D >> Groups.io Links: You receive all messages sent to this group. >>=20 >> View/Reply Online (#49330): = https://edk2.groups.io/g/devel/message/49330 >> Mute This Topic: https://groups.io/mt/36392351/1772286 >> Group Owner: devel+owner@edk2.groups.io >> Unsubscribe: https://edk2.groups.io/g/devel/unsub = [jiewen.yao@intel.com] >> -=3D-=3D-=3D-=3D-=3D-=3D >=20 -----------------------5f0bbbd293a088f86f2f73a55deb1161 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: ProtonMail wsBmBAEBCAAQBQJdrresCRBPsoxt7Hy0xQAKCRBPsoxt7Hy0xZJ7CABwn5VF Sc0WapB8bg2MomTDgclnx+vkoncHaDgJvqSPZVOp94BYyki4xyggRlHIxvOm kDJCmMONY9/+rgkAJpOwUatIha8PrhAeQjt30yksgIG6gIw6skqN8P6aW177 APHLO/xaILkwo2DSS6TtoTD91BsMkzqO0qn5Vfzjp2x2+Ptnn29ZTVy96gyl WZosH5IxJEBsoPGA9aD1fuY/fL+G3NSnC6QZ3FxA2ATGUcF74UJAw4lRYMax 2Ub5iWw880/9+YnoR4dQe+qdev7rQ5hOBncw+DwXtbI3cRiGybvFlBrsFyYe gDr/wsX2muW2hhpx1D+DT+RS+6/r2hW+e6B3 =zAbd -----END PGP SIGNATURE----- -----------------------5f0bbbd293a088f86f2f73a55deb1161--