From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from ma1-aaemail-dr-lapp01.apple.com (ma1-aaemail-dr-lapp01.apple.com [17.171.2.60]) by mx.groups.io with SMTP id smtpd.web12.15388.1583344409488926270 for ; Wed, 04 Mar 2020 09:53:29 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@apple.com header.s=20180706 header.b=YPIFMjkY; spf=pass (domain: apple.com, ip: 17.171.2.60, mailfrom: afish@apple.com) Received: from pps.filterd (ma1-aaemail-dr-lapp01.apple.com [127.0.0.1]) by ma1-aaemail-dr-lapp01.apple.com (8.16.0.27/8.16.0.27) with SMTP id 024HfsKP020846; Wed, 4 Mar 2020 09:53:28 -0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=apple.com; h=sender : content-type : mime-version : subject : from : in-reply-to : date : cc : content-transfer-encoding : message-id : references : to; s=20180706; bh=c9QP1pVB3iDN5O1B6DXU2rXfEm4tiBnCvranquTjpww=; b=YPIFMjkYuKRs36MdDDF2V3diKIaAvucp2g+qdBzcLypOiPOVqRUwjc0bQ+d67XoBflvk EohX6J220FSkuKZgk8PCua1t2Xf71fq6KQccEtorxzpGi2Xa6/AmeCLxDX4KKjfpokYV 9XtLbpSJH1ZYJM6FzvFHG4UpTSpN+7F+70gM2UqRipLrFGaVRBhgmLyX3mf7Tzm1FDBM FAxiz+PxTfs4+viZCjVaH5QI0heZRnzccsbMo87gMEyxJp2fDjXMujv0VVfVtWXhOtQS rdR7m4Aird5CU+gc2ie/snSrxYFGZ/yGYiTtCIQfa1ki36Rn6BXBctj0C4uwBeuW1PCp nw== Received: from rn-mailsvcp-mta-lapp04.rno.apple.com (rn-mailsvcp-mta-lapp04.rno.apple.com [10.225.203.152]) by ma1-aaemail-dr-lapp01.apple.com with ESMTP id 2yfqg6xpgv-2 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Wed, 04 Mar 2020 09:53:27 -0800 Received: from nwk-mmpp-sz10.apple.com (nwk-mmpp-sz10.apple.com [17.128.115.122]) by rn-mailsvcp-mta-lapp04.rno.apple.com (Oracle Communications Messaging Server 8.1.0.1.20190704 64bit (built Jul 4 2019)) with ESMTPS id <0Q6O00P4KKD2KG30@rn-mailsvcp-mta-lapp04.rno.apple.com>; Wed, 04 Mar 2020 09:53:26 -0800 (PST) Received: from process_milters-daemon.nwk-mmpp-sz10.apple.com by nwk-mmpp-sz10.apple.com (Oracle Communications Messaging Server 8.0.2.4.20190507 64bit (built May 7 2019)) id <0Q6O00E00K4W8N00@nwk-mmpp-sz10.apple.com>; Wed, 04 Mar 2020 09:53:26 -0800 (PST) X-Va-A: X-Va-T-CD: 07a9f7dd315dc6000695a0402a47d12d X-Va-E-CD: 27402815c5e31821051163b02fab88e9 X-Va-R-CD: 8bf71fd521e6e2c0a106a042b8cb9e5a X-Va-CD: 0 X-Va-ID: 71ba0989-ac87-4912-85c0-c385fe6dede5 X-V-A: X-V-T-CD: 07a9f7dd315dc6000695a0402a47d12d X-V-E-CD: 27402815c5e31821051163b02fab88e9 X-V-R-CD: 8bf71fd521e6e2c0a106a042b8cb9e5a X-V-CD: 0 X-V-ID: 4f0e65d6-3745-4482-9c31-4ddfeb9be15e X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.138,18.0.572 definitions=2020-03-04_07:2020-03-04,2020-03-04 signatures=0 Received: from [17.235.12.181] by nwk-mmpp-sz10.apple.com (Oracle Communications Messaging Server 8.0.2.4.20190507 64bit (built May 7 2019)) with ESMTPSA id <0Q6O00E3GKCYC520@nwk-mmpp-sz10.apple.com>; Wed, 04 Mar 2020 09:53:23 -0800 (PST) Sender: afish@apple.com MIME-version: 1.0 (Mac OS X Mail 13.0 \(3594.4.17\)) Subject: Re: Disabling safe string constraint assertions From: "Andrew Fish" In-reply-to: <86d328d2-e929-379a-e6dd-69968a064f13@redhat.com> Date: Wed, 04 Mar 2020 09:53:22 -0800 Cc: Vitaly Cheptsov , devel@edk2.groups.io, =?utf-8?Q?Marvin_H=C3=A4user?= , Mike Kinney , "Gao, Liming" , "Gao, Zhichao" Message-id: References: <17370CE3-AA53-4858-B497-1AB93DA709D3@ispras.ru> <86d328d2-e929-379a-e6dd-69968a064f13@redhat.com> To: Laszlo Ersek X-Mailer: Apple Mail (2.3594.4.17) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.138,18.0.572 definitions=2020-03-04_07:2020-03-04,2020-03-04 signatures=0 Content-type: text/plain; charset=utf-8 Content-transfer-encoding: quoted-printable > On Mar 4, 2020, at 5:33 AM, Laszlo Ersek wrote: >=20 > On 03/03/20 22:12, Vitaly Cheptsov wrote: >> Hello, >>=20 >> I am creating a new thread for this issue, as for some reason half of = my messages do not display in the web-interface and some of e-mail = clients in the previous one. It seems that somebody sent broken HTML = code to groups.io , and it did not like to be quoted. = It will be quite nice if all the messages sent are plain-text to avoid = such issues in the future. >>=20 >> Back to the issue, we want to make constraint assertions = (SAFE_STRING_CONSTRAINT_CHECK) in SafeString.c be optionally disabled in = DEBUG builds, as they break our ability to use some of BaseLib = interfaces to verify untrusted user data in UEFI Applications. The = background of this problem is covered in a bugzilla[1], and was also = extensively discussed in the last proposed patch discussion thread[2]. = We want the solution to the problem to land in the next stable tag: = edk2-stable202005, and can submit the patch. >>=20 >> Currently there is no clear decision on what approach to take, as = there are three different proposals to implement: >>=20 >> 1) Andrew Fish=E2=80=99s approach with C11-like constraint = handler[3]. >> 2) Marvin H=C3=A4user=E2=80=99s approach with return codes and = constraint assertions being moved to the caller side[4]. >> 3) My approach suggesting DebugLib interface simplification, = permitting us to resolve the problem and keep backwards = compatibility[5]. >>=20 >> My personal opinion is that: >>=20 >> =E2=80=94 Marvin=E2=80=99s way is very interesting, but it would = require implementing code on the caller side basically for every call, = and this would requires extra programming effort. It may also be uneasy = to teach people how to do this right, and this does not strictly provide = a good solution for situations, where nesting is more than 2 (i.e. when = user code calls library code, which itself calls library code). Because = of these downsides I am afraid it is impractical to adopt it in EDK II. >> =E2=80=94 Andrew=E2=80=99s way is reasonable from the C standard = point of view, but it does not work quite right with reentrancy and I am = not positive we need the dynamic handling, especially because of = DEBUG/RELEASE build confusion. I explained this in my previous e-mail, = which I attached to the end of this message as quote, since groups.io = ate the previous send. Other than that, I would say = that I can implement this approach if there is no other option. >> =E2=80=94 My own approach makes sense, as it reduces the amount of = code in every DebugLib and actually simplifies the development in the = future. I believe currently this is the most reasonable route we can = take, and suggest other parties to consider it. >>=20 >> Since the three of us each have their own suggestions, I ask Mike and = others to rejoin our discussions and give their opinions quickly, so = that we can choose, perhaps vote, and proceed with the implementation. = Thanks for your dedication. >=20 > I support the original proposal given in: >=20 > https://bugzilla.tianocore.org/show_bug.cgi?id=3D2054#c0 >=20 > also known as the v3 posting: >=20 > * [edk2-devel] [PATCH v3 1/1] > MdePkg: Add PCD to disable safe string constraint assertions >=20 > https://edk2.groups.io/g/devel/message/52838 > = http://mid.mail-archive.com/20200103171242.63839-2-vit9696@protonmail.com >=20 > With the following tweak: I think we should rather introduce a new bit > in "PcdDebugPropertyMask" than an entirely new BOOLEAN PCD. >=20 Laszlo, +1 > (Note that both PcdDebugPropertyMask and the propsed new PCD support > precisely the following PCD access methods: PcdsFixedAtBuild, > PcdsPatchableInModule.) >=20 > I don't feel too strongly about this part (i.e., whether we introduce = a > new PCD, or a new bit in PcdDebugPropertyMask), as long as we make = sure > that the access method can *never* become dynamic. >=20 > I do prefer quite strongly the original proposal, at a higher level. >=20 > Here's why I support the original proposal: >=20 > - In a brand new code base (or brand new set of APIs), I would fight > tooth and nail to keep such ASSERT()s out of the *base* APIs. It's up = to > the caller to check return values. Also, additional wrapper > functions/macros can be offered *on top* of the base APIs that > internalize the ASSERT()s for special use cases. >=20 > But this ship has sailed, for edk2. >=20 Crazy idea but we could add versions of APIs that don't do the checking = as new APIs? That would make life easier for untrusted code, Runtime = Drivers, code running on the AP, and other places that the ASSERTs could = have side effects.=20 > - I'm unfriendly towards callbacks. They make the behavior of code > implicit rather than explicit, and implicit is more difficult to = reason > about. IMO callbacks should be considered a last resort only. >=20 Not my 1st choice either as I agree with your first point about given = the caller control. I was just wanted to go through the exercise of what = it would take to make it like C11, and give the caller control.=20 > Just my two cents, because I've been asked to comment. Thanks for helping out. Thanks, Andrew Fish >=20 > Thanks > Laszlo >=20