From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web11.10240.1589456010211543735 for ; Thu, 14 May 2020 04:33:30 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: ard.biesheuvel@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id DDDAC30E; Thu, 14 May 2020 04:33:28 -0700 (PDT) Received: from [192.168.1.81] (unknown [10.37.8.255]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id E56783F305; Thu, 14 May 2020 04:33:25 -0700 (PDT) Subject: Re: [edk2-devel] [PATCH V6 0/1] Disable safe string constraint assertions To: devel@edk2.groups.io, cheptsov@ispras.ru Cc: Andrew Fish , Ard Biesheuvel , Bret Barkelew , "Brian J . Johnson" , Chasel Chiu , Jordan Justen , Laszlo Ersek , Leif Lindholm , Liming Gao , =?UTF-8?Q?Marvin_H=c3=a4user?= , Mike Kinney , Vincent Zimmer , Zhichao Gao References: <20200514092537.29609-1-cheptsov@ispras.ru> From: "Ard Biesheuvel" Message-ID: <6b0ff798-db01-f29b-271c-0a61ae37ea41@arm.com> Date: Thu, 14 May 2020 13:33:19 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: <20200514092537.29609-1-cheptsov@ispras.ru> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: quoted-printable On 5/14/20 11:25 AM, Vitaly Cheptsov via groups.io wrote: > CC: Andrew Fish > CC: Ard Biesheuvel > CC: Bret Barkelew > CC: Brian J. Johnson > CC: Chasel Chiu > CC: Jordan Justen > CC: Laszlo Ersek > CC: Leif Lindholm > CC: Liming Gao > CC: Marvin H=C3=A4user > CC: Mike Kinney > CC: Vincent Zimmer > CC: Zhichao Gao >=20 > Current implementation of SafeString does not let one parse untrusted > data with its interfaces as they ASSERT on failing runtime checks and > require one to effectively reimplement the function on the caller side. >=20 > All the former proposals made it clear that the attempts to introduce > a solution preserving this behaviour require a lot of changes > throughout the codebase including platform code (e.g. introducing > constraint assertions and updating all DebugLib implementations) > or require all sorts of hacks. >=20 > Since the original code has roughly no benefit except in some very > special cases and the effort required to preserve it is very high, > I propose to remove it as agreed on with several other parties. >=20 > Please note, that this patch does not change the behaviour of the > functions in RELEASE builds. I.e. they will still check for NULL > and return RETURN_INVALID_PARAMETER. In the future we may (or may > not) want them to simply ASSERT in this case. Regardless this should > be done in a separate BZ entry and a separate commit. For this reason > I ask everyone not to touch this subject. >=20 > Due to the amount of discussion this has already undergone after > almost a year I kindly request everyone to reduce the communication > to the minimum and abstain from proposing another approach. >=20 > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2054 >=20 > Requesting to merge this into edk2-stable202005. >=20 > Vitaly Cheptsov (1): > MdePkg: Fix SafeString performing assertions on runtime checks >=20 > MdePkg/Include/Library/BaseLib.h | 120 ++------------------ > MdePkg/Library/BaseLib/SafeString.c | 80 ------------- > 2 files changed, 7 insertions(+), 193 deletions(-) >=20 Acked-by: Ard Biesheuvel