Re: [edk2-devel] [PATCH V6 0/1] Disable safe string constraint assertions

["Ard Biesheuvel" <ard.biesheuvel@arm.com>]

On 5/14/20 11:25 AM, Vitaly Cheptsov via groups.io wrote:
> CC: Andrew Fish <afish@apple.com>
> CC: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> CC: Bret Barkelew <bret.barkelew@microsoft.com>
> CC: Brian J. Johnson <brian.johnson@hpe.com>
> CC: Chasel Chiu <chasel.chiu@intel.com>
> CC: Jordan Justen <jordan.l.justen@intel.com>
> CC: Laszlo Ersek <lersek@redhat.com>
> CC: Leif Lindholm <leif@nuviainc.com>
> CC: Liming Gao <liming.gao@intel.com>
> CC: Marvin Häuser <mhaeuser@outlook.de>
> CC: Mike Kinney <michael.d.kinney@intel.com>
> CC: Vincent Zimmer <vincent.zimmer@intel.com>
> CC: Zhichao Gao <zhichao.gao@intel.com>
> 
> 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.
> 
> 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.
> 
> 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.
> 
> 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.
> 
> 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.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2054
> 
> Requesting to merge this into edk2-stable202005.
> 
> Vitaly Cheptsov (1):
>    MdePkg: Fix SafeString performing assertions on runtime checks
> 
>   MdePkg/Include/Library/BaseLib.h    | 120 ++------------------
>   MdePkg/Library/BaseLib/SafeString.c |  80 -------------
>   2 files changed, 7 insertions(+), 193 deletions(-)
> 

Acked-by: Ard Biesheuvel <ard.biesheuvel@arm.com>