MdePkg/Include/Library/BaseLib.h | 120 ++------------------ MdePkg/Library/BaseLib/SafeString.c | 80 ------------- 2 files changed, 7 insertions(+), 193 deletions(-)
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(-) -- 2.24.2 (Apple Git-127) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#59524): https://edk2.groups.io/g/devel/message/59524 Mute This Topic: https://groups.io/mt/74201256/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
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> -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#59546): https://edk2.groups.io/g/devel/message/59546 Mute This Topic: https://groups.io/mt/74201256/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.