From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-40134.protonmail.ch (mail-40134.protonmail.ch [185.70.40.134]) by mx.groups.io with SMTP id smtpd.web12.20509.1581333154924347819 for ; Mon, 10 Feb 2020 03:12:35 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@protonmail.com header.s=default header.b=uLxkGTli; spf=pass (domain: protonmail.com, ip: 185.70.40.134, mailfrom: vit9696@protonmail.com) Date: Mon, 10 Feb 2020 11:12:28 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=default; t=1581333152; bh=ShhnwTXU+G9Bg34+4IVj5dW87a6asNZa/D0PvplseXQ=; h=Date:To:From:Cc:Reply-To:Subject:In-Reply-To:References: Feedback-ID:From; b=uLxkGTli9leHvGvdsce2+zzAEQ25kuFlGnJHqHRYk5A2Zum7R+wGM6M6Lk0g4vLs4 ZdL0czQe6+s5/YSfwJ1k0ELKqjvSKrCrek8MyvI5mB5NkWy15cFGjbKYPHCWxyBv9T sj6pVUga41EuQNXxLvsWgYO5pIzOnTnUjQPp+gHU= To: "Kinney, Michael D" , "Gao, Liming" , "Gao, Zhichao" , "devel@edk2.groups.io" From: "Vitaly Cheptsov" Cc: =?UTF-8?Q?Marvin_H=C3=A4user?= , Laszlo Ersek Reply-To: vit9696 Subject: Re: [edk2-devel] [PATCH v3 0/1] Add PCD to disable safe string constraint assertions Message-ID: In-Reply-To: <492A300B-1CED-4FF0-98F8-20D1E116DCC0@protonmail.com> References: <20200103171242.63839-1-vit9696@protonmail.com> <492A300B-1CED-4FF0-98F8-20D1E116DCC0@protonmail.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,BAYES_50, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF, FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,FREEMAIL_REPLYTO_END_DIGIT shortcircuit=no 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: 54133 Content-Type: multipart/signed; protocol="application/pgp-signature"; micalg=pgp-sha256; boundary="---------------------f727994f9a5f37ce9f6a179a4261ccfa"; charset=UTF-8 -----------------------f727994f9a5f37ce9f6a179a4261ccfa Cc: =?utf-8?Q?Marvin_H=C3=A4user?= , Laszlo Ersek Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 Date: Mon, 10 Feb 2020 14:11:23 +0300 From: vit9696 In-Reply-To: <492A300B-1CED-4FF0-98F8-20D1E116DCC0@protonmail.com> Message-Id: Mime-Version: 1.0 (Mac OS X Mail 13.0 \(3608.60.0.2.5\)) References: <20200103171242.63839-1-vit9696@protonmail.com> <9MoxdJ8nml1WYF1JViU2YrwRwoNEP8Bu7Ax0wps7KatP9SiQyoly1LXHuozlAZdVMTsXi1-rcHMh1E5UZN8VyQ==@protonmail.conversationid> <492A300B-1CED-4FF0-98F8-20D1E116DCC0@protonmail.com> Subject: Re: [edk2-devel] [PATCH v3 0/1] Add PCD to disable safe string constraint assertions To: "Kinney, Michael D" , "Gao, Liming" , "Gao, Zhichao" , "devel@edk2.groups.io" X-Mailer: Apple Mail (2.3608.60.0.2.5) Hello, It has been quite some time since we submitted the patch with so far no ne= gative response. As I mentioned previously, my team will strongly benefit f= rom its landing in EDK II mainline. Since it does not add any regressions a= nd can be viewed as a feature implementation for the rest of EDK II users, = I request this to be merged upstream in edk2-stable202002. Best wishes, Vitaly > 27 =D1=8F=D0=BD=D0=B2. 2020 =D0=B3., =D0=B2 12:47, vit9696 =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0=B0): >=20 >=20 > Hi Mike, >=20 > Any progress with this? We would really benefit from this landing in the= next stable release. >=20 > Best, > Vitaly >=20 >> 8 =D1=8F=D0=BD=D0=B2. 2020 =D0=B3., =D0=B2 19:35, Kinney, Michael D =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0=B0= ): >>=20 >>=20 >> Hi Vitaly, >>=20 >> Thanks for the additional background. I would like >> a couple extra day to review the PCD name and the places >> the PCD might potentially be used. >>=20 >> If we find other APIs where ASSERT() behavior is only >> valuable during dev/debug to quickly identify misuse >> with trusted data and the API provides predicable >> return behavior when ASSERT() is disabled, then I would >> like to have a pattern we can potentially apply to all >> these APIs across all packages. >>=20 >> Thanks, >>=20 >> Mike >>=20 >>> -----Original Message----- >>> From: devel@edk2.groups.io On >>> Behalf Of Vitaly Cheptsov via Groups.Io >>> Sent: Monday, January 6, 2020 10:44 AM >>> To: Kinney, Michael D >>> Cc: devel@edk2.groups.io >>> Subject: Re: [edk2-devel] [PATCH v3 0/1] Add PCD to >>> disable safe string constraint assertions >>>=20 >>> Hi Mike, >>>=20 >>> Yes, the primary use case is for UEFI Applications. We >>> do not want to disable ASSERT=E2=80=99s completely, as >>> assertions that make sense, i.e. the ones signalising >>> about interface misuse, are helpful for debugging. >>>=20 >>> I have already explained in the BZ that basically all >>> safe string constraint assertions make no sense for >>> handling untrusted data. We find this use case very >>> logical, as these functions behave properly with >>> assertions disabled and cover all these error >>> conditions by the return statuses. In such situation is >>> not useful for these functions to assert, as we end up >>> inefficiently reimplementing the logic. I would have >>> liked the approach of discussing the interfaces >>> individually, but I struggle to find any that makes >>> sense from this point of view. >>>=20 >>> AsciiStrToGuid will ASSERT when the length of the >>> passed string is odd. Functions that cannot, ahem, >>> parse, for us are pretty much useless. >>> AsciiStrCatS will ASSERT when the appended string does >>> not fit the buffer. For us this logic makes this >>> function pretty much equivalent to deprecated and thus >>> unavailable AsciiStrCat, except it is also slower. >>>=20 >>> My original suggestion was to remove the assertions >>> entirely, but several people here said that they use >>> them to verify usage errors when handling trusted data. >>> This makes good sense to me, so we suggest to support >>> both cases by introducing a PCD in this patch. >>>=20 >>> Best wishes, >>> Vitaly >>>=20 >>>> 6 =D1=8F=D0=BD=D0=B2. 2020 =D0=B3., =D0=B2 21:28, Kinney, Michael D >>> =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0= =BB(=D0=B0): >>>>=20 >>>>=20 >>>> Hi Vitaly, >>>>=20 >>>> Is the use case for UEFI Applications? >>>>=20 >>>> There is a different mechanism to disable all >>> ASSERT() >>>> statements within a UEFI Application. >>>>=20 >>>> If a component is consuming data from an untrusted >>> source, >>>> then that component is required to verify the >>> untrusted >>>> data before passing it to a function that clearly >>> documents >>>> is input requirements. If this approach is followed, >>> then >>>> the BaseLib functions can be used "as is" as long as >>> the >>>> ASSERT() conditions are verified before calling. >>>>=20 >>>> If there are some APIs that currently document their >>> ASSERT() >>>> behavior and we think that ASSERT() behavior is >>> incorrect and >>>> should be handled by an existing error return value, >>> then we >>>> should discuss each of those APIs individually. >>>>=20 >>>> Mike >>>>=20 >>>>=20 >>>>> -----Original Message----- >>>>> From: devel@edk2.groups.io On >>>>> Behalf Of Vitaly Cheptsov via Groups.Io >>>>> Sent: Friday, January 3, 2020 9:13 AM >>>>> To: devel@edk2.groups.io >>>>> Subject: [edk2-devel] [PATCH v3 0/1] Add PCD to >>> disable >>>>> safe string constraint assertions >>>>>=20 >>>>> REF: >>>>> https://bugzilla.tianocore.org/show_bug.cgi?id=3D2054 >>>>>=20 >>>>> Requesting for merge in edk2-stable202002. >>>>>=20 >>>>> Changes since V1: >>>>> - Enable assertions by default to preserve the >>> original >>>>> behaviour >>>>> - Fix bugzilla reference link >>>>> - Update documentation in BaseLib.h >>>>>=20 >>>>> Vitaly Cheptsov (1): >>>>> MdePkg: Add PCD to disable safe string constraint >>>>> assertions >>>>>=20 >>>>> MdePkg/MdePkg.dec | 6 ++ >>>>> MdePkg/Library/BaseLib/BaseLib.inf | 11 +-- >>>>> MdePkg/Include/Library/BaseLib.h | 74 >>>>> +++++++++++++------- >>>>> MdePkg/Library/BaseLib/SafeString.c | 4 +- >>>>> MdePkg/MdePkg.uni | 6 ++ >>>>> 5 files changed, 71 insertions(+), 30 deletions(-) >>>>>=20 >>>>> -- >>>>> 2.21.0 (Apple Git-122.2) >>>>>=20 >>>>>=20 >>>>> -=3D-=3D-=3D-=3D-=3D-=3D >>>>> Groups.io Links: You receive all messages sent to >>> this >>>>> group. >>>>>=20 >>>>> View/Reply Online (#52837): >>>>> https://edk2.groups.io/g/devel/message/52837 >>>>> Mute This Topic: >>> https://groups.io/mt/69401948/1643496 >>>>> Group Owner: devel+owner@edk2.groups.io >>>>> Unsubscribe: https://edk2.groups.io/g/devel/unsub >>>>> [michael.d.kinney@intel.com] >>>>> -=3D-=3D-=3D-=3D-=3D-=3D >>>>=20 >>>=20 >>>=20 >>>=20 >>=20 >=20 -----------------------f727994f9a5f37ce9f6a179a4261ccfa Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: ProtonMail wsBmBAEBCAAQBQJeQTqYCRBPsoxt7Hy0xQAKCRBPsoxt7Hy0xbipB/0RczGC KKGf5PnAa3kQF4HI2hFW+wsCKjm33pvs59/XBiqquFoVVE/yrFmjZ/F3SH0S bITf8MsAZlKloArGrYHx3LmQZJaH28fa4/KEwTr+soisTLkPY8Q0yDkPLB9Y pZArDBot5LHchkW68il2BWWTu0477mky5EHBMghU9ya9CbI144ZCUNouRPj9 qyrwWstNC7PxH5DQvldmN6Fot5AKkkVfazf0y0ltB2rwSoiDBgv7KdHDO8cB HNVJtTOvCi4OOHAku3RPZlvht7Nb+Uh4CgUsK1zTaJwIM/cxc5EECOo+auWP SUQvU0uRXgMygjM4I0f64gE1CQlmcsoQfTP7 =fQaO -----END PGP SIGNATURE----- -----------------------f727994f9a5f37ce9f6a179a4261ccfa--