public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH V7 0/1] Disable safe string constraint assertions
@ 2020-05-14 17:31 Vitaly Cheptsov
  2020-05-14 17:31 ` [PATCH V7 1/1] MdePkg: Fix SafeString performing assertions on runtime checks Vitaly Cheptsov
  0 siblings, 1 reply; 6+ messages in thread
From: Vitaly Cheptsov @ 2020-05-14 17:31 UTC (permalink / raw)
  To: devel
  Cc: Andrew Fish, Ard Biesheuvel, Bret Barkelew, Brian J . Johnson,
	Chasel Chiu, Jordan Justen, Laszlo Ersek, Leif Lindholm,
	Liming Gao, Marvin Häuser, Mike Kinney, Vincent Zimmer,
	Zhichao Gao

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>

V7 addressed review comments (only documentation changes).

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    | 111 -------------------
 MdePkg/Library/BaseLib/SafeString.c | 112 --------------------
 2 files changed, 223 deletions(-)

-- 
2.24.2 (Apple Git-127)


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-05-20  2:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-05-14 17:31 [PATCH V7 0/1] Disable safe string constraint assertions Vitaly Cheptsov
2020-05-14 17:31 ` [PATCH V7 1/1] MdePkg: Fix SafeString performing assertions on runtime checks Vitaly Cheptsov
2020-05-15 11:30   ` Laszlo Ersek
2020-05-18 17:04   ` [edk2-devel] " Michael D Kinney
2020-05-18 18:07     ` Vitaly Cheptsov
2020-05-20  2:48       ` Michael D Kinney

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox