From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by mx.groups.io with SMTP id smtpd.web11.746.1590005424695289668 for ; Wed, 20 May 2020 13:10:25 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.65, mailfrom: michael.d.kinney@intel.com) IronPort-SDR: K9UIjhxt4z6xCNjDAHrKN17ZfIPLnS2or5SocAZqE6nfA44I8u0uWyEVaj4HMto9RlUj70DSw0 +v5J796smmYA== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 May 2020 13:10:24 -0700 IronPort-SDR: d+GWtAs/AljTpOW+/4ZnFQFjgdsrMYglrtQaFsgF4j44HOILG6XkwBDeEi9tYxVIJAN2iz0g0m bX3+rDdZptFQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.73,415,1583222400"; d="scan'208";a="308822880" Received: from mdkinney-mobl2.amr.corp.intel.com ([10.254.67.110]) by FMSMGA003.fm.intel.com with ESMTP; 20 May 2020 13:10:23 -0700 From: "Michael D Kinney" To: devel@edk2.groups.io Subject: [Patch v9 0/2] Disable safe string constraint assertions Date: Wed, 20 May 2020 13:10:20 -0700 Message-Id: <20200520201022.28196-1-michael.d.kinney@intel.com> X-Mailer: git-send-email 2.21.0.windows.1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2054 V9 Update unit test comment and use zero buffers to verify more behavior. V8 Add DEBUG_VERBOSE message and unit test. 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 behavior 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. Requesting to merge this into edk2-stable202005. 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: Michael D Kinney michael.d.kinney@intel.com Cc: Vincent Zimmer vincent.zimmer@intel.com Cc: Zhichao Gao zhichao.gao@intel.com Cc: Jiewen Yao jiewen.yao@intel.com Signed-off-by: Vitaly Cheptsov vit9696@protonmail.com Michael D Kinney (1): MdePkg/Test/BaseLib: Add SAFE_STRING_CONSTRAINT_CHECK unit test Vitaly Cheptsov (1): MdePkg: Fix SafeString performing assertions on runtime checks MdePkg/Include/Library/BaseLib.h | 111 ----------------- MdePkg/Library/BaseLib/SafeString.c | 115 +----------------- .../UnitTest/Library/BaseLib/Base64UnitTest.c | 107 ++++++++++++++++ 3 files changed, 110 insertions(+), 223 deletions(-) -- 2.21.0.windows.1