From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail4.protonmail.ch (mail4.protonmail.ch [185.70.40.27]) by mx.groups.io with SMTP id smtpd.web10.1493.1571642869461654077 for ; Mon, 21 Oct 2019 00:27:50 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@protonmail.com header.s=default header.b=AUNNPrMb; spf=pass (domain: protonmail.com, ip: 185.70.40.27, mailfrom: vit9696@protonmail.com) Date: Mon, 21 Oct 2019 07:27:43 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=default; t=1571642867; bh=K1T1ZOsxgjcPLytbpFYQk5uuJgbjcyNnObJRidbhc+4=; h=Date:To:From:Cc:Reply-To:Subject:In-Reply-To:References: Feedback-ID:From; b=AUNNPrMb6PNxl7xCjs2WNfdl/Bca/eRjiPIFNbZVe5tZsl+ClS4fIQQ2wYlWrKFcz uif+j9reINwHcOBieqL4WX9ZTV679A+/Euok+1MzHB/uVUGrTohLY/skbVuiSNQB1m eiRnNaQzKWFDF591w3h3p+saSkn+EhnLthGLAAMw= To: "Yao, Jiewen" , "Gao, Liming" From: "Vitaly Cheptsov" Cc: "devel@edk2.groups.io" , "Wang, Jian J" , "Kinney, Michael D" Reply-To: vit9696 Subject: Re: [edk2-devel] [PATCH v1 1/1] MdePkg: Add PCD to disable safe string constraint assertions Message-ID: <6FF18D79-65ED-4C0B-9B03-99C0B5DD539A@protonmail.com> In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C503F80B477@shsmsx102.ccr.corp.intel.com> References: <20191020130553.42851-1-vit9696@protonmail.com> <20191020130553.42851-2-vit9696@protonmail.com> <4A89E2EF3DFEDB4C8BFDE51014F606A14E51F6EB@SHSMSX104.ccr.corp.intel.com> <74D8A39837DF1E4DA445A8C0B3885C503F80B477@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: 49279 Content-Type: multipart/signed; protocol="application/pgp-signature"; micalg=pgp-sha256; boundary="---------------------4d5d22ad4fa19f32024828466272b323"; charset=UTF-8 -----------------------4d5d22ad4fa19f32024828466272b323 Cc: "devel@edk2.groups.io" , "Wang, Jian J" , "Kinney, Michael D" Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 Date: Mon, 21 Oct 2019 10:27:40 +0300 From: vit9696 In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C503F80B477@shsmsx102.ccr.corp.intel.com> Message-Id: <6FF18D79-65ED-4C0B-9B03-99C0B5DD539A@protonmail.com> Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.11\)) References: <20191020130553.42851-1-vit9696@protonmail.com> <20191020130553.42851-2-vit9696@protonmail.com> <4A89E2EF3DFEDB4C8BFDE51014F606A14E51F6EB@SHSMSX104.ccr.corp.intel.com> <74D8A39837DF1E4DA445A8C0B3885C503F80B477@shsmsx102.ccr.corp.intel.com> Subject: Re: [edk2-devel] [PATCH v1 1/1] MdePkg: Add PCD to disable safe string constraint assertions To: "Yao, Jiewen" , "Gao, Liming" X-Mailer: Apple Mail (2.3445.104.11) Liming, Jiewen,=20 I am personally fine to resubmit the patch changing the defaults to = TRUE, but does it actually make sense in any other environment but some = special testing platform? I cannot imagine any production platform that = would need it enabled, the only use case is to perform analysis on the = trusted data usage or something similar, as I explained on BZ. As for assertions, I would expect that all PCDs we introduce are meant = to control unexpected ASSERT behaviour. I.e. where ASSERTs are = potentially used to signalise about issues coming not only from trusted = sources (usage contract violation) but also from untrusted sources = (external data). Such assertions cannot be used in production = environments, as they may break software, and thus we have to implement = code to disable them. Best regards, Vitaly > 21 =D0=BE=D0=BA=D1=82. 2019 =D0=B3., =D0=B2 7:28, Yao, Jiewen = =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0=B0):= >=20 >=20 > Hi Mike > I remember we have discussed it before. >=20 > The general concern is that: how many additional PCD we need = introduce, to control different ASSERT in different modules ? >=20 > We may want to enable *some* assert in some modules, but disable *some = other* assert. > E.g. the assert for linkedlist in base lib is another example... >=20 > What is your thought? >=20 > Thank you > Yao Jiewen >=20 >> -----Original Message----- >> From: Gao, Liming >> Sent: Monday, October 21, 2019 11:17 AM >> To: devel@edk2.groups.io; vit9696@protonmail.com >> Cc: Yao, Jiewen ; Wang, Jian J = ; >> Gao, Liming ; Kinney, Michael D >> >> Subject: RE: [edk2-devel] [PATCH v1 1/1] MdePkg: Add PCD to disable = safe string >> constraint assertions >>=20 >> Include more people. >>=20 >> Basically, to keep the compatible behavior, = PcdAssertOnSafeStringConstraints >> default value should be TRUE. >> The different platform can configure it. >>=20 >> Thanks >> Liming >>> -----Original Message----- >>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf = Of >>> Vitaly Cheptsov via Groups.Io >>> Sent: Sunday, October 20, 2019 9:06 PM >>> To: devel@edk2.groups.io >>> Subject: [edk2-devel] [PATCH v1 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..dda2cdf401 100644 >>> --- a/MdePkg/MdePkg.dec >>> +++ b/MdePkg/MdePkg.dec >>> @@ -2221,6 +2221,12 @@ [PcdsFixedAtBuild,PcdsPatchableInModule] >>> # @Prompt Memory Address of GuidedExtractHandler Table. >>>=20 >>> gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|0x10000 >>> 00|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|FALSE|BOOL >>> EAN|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_H >>> ELP #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_PROM >>> PT #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 (#49255): = https://edk2.groups.io/g/devel/message/49255 >>> Mute This Topic: https://groups.io/mt/35943317/1759384 >>> Group Owner: devel+owner@edk2.groups.io >>> Unsubscribe: https://edk2.groups.io/g/devel/unsub = [liming.gao@intel.com] >>> -=3D-=3D-=3D-=3D-=3D-=3D >=20 -----------------------4d5d22ad4fa19f32024828466272b323 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: ProtonMail wsBmBAEBCAAQBQJdrV3tCRBPsoxt7Hy0xQAKCRBPsoxt7Hy0xdZzCAAGeunG M7EJdVo4/i5jxb37ejldKs3Q5NTu8htufbjbxvqnMldscYtoqeVlGkLGehci +GAKMSExR3EUl3KaTj1QMW1coS5GnFdbNQKH8yfjMaZHMhAYErk/m1wxlAOY Js7VUooORobOHTNQ0f5vv8nCD2sRzlKF10ZUnD2Lf6d2g5YjP3ec1XjXEOlm T8+SwGUhzAt63P1EOO/Gq3u8uyMczzWnt6K/Nl3fX8aYSm1kNzXhBikoGWQ4 IVK8xaR/L/8wQUKHt2c92Q55TRnBLDKyh08Ao90jf2QL0UGiYIHLcySTajtB zdJJRzsJIoFDcpXeGxqxRS7G9XluXr348INm =XkT2 -----END PGP SIGNATURE----- -----------------------4d5d22ad4fa19f32024828466272b323--