Replying as per Liming's request for this to be merged into edk2-stable202002.
On Mon, Feb 10, 2020 at 14:12, vit9696 <vit9696@protonmail.com> wrote: Hello,
It has been quite some time since we submitted the patch with so far no negative response. As I mentioned previously, my team will strongly benefit from its landing in EDK II mainline. Since it does not add any regressions and can be viewed as a feature implementation for the rest of EDK II users, I request this to be merged upstream in edk2-stable202002.
Best wishes,
Vitaly
> 27 янв. 2020 г., в 12:47, vit9696 <vit9696@protonmail.com> написал(а):
>
>
> Hi Mike,
>
> Any progress with this? We would really benefit from this landing in the next stable release.
>
> Best,
> Vitaly
>
>> 8 янв. 2020 г., в 19:35, Kinney, Michael D <michael.d.kinney@intel.com> написал(а):
>>
>>
>> Hi Vitaly,
>>
>> Thanks for the additional background. I would like
>> a couple extra day to review the PCD name and the places
>> the PCD might potentially be used.
>>
>> If we find other APIs where ASSERT() behavior is only
>> valuable during dev/debug to quickly identify misuse
>> with trusted data and the API provides predicable
>> return behavior when ASSERT() is disabled, then I would
>> like to have a pattern we can potentially apply to all
>> these APIs across all packages.
>>
>> Thanks,
>>
>> Mike
>>
>>> -----Original Message-----
>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On
>>> Behalf Of Vitaly Cheptsov via Groups.Io
>>> Sent: Monday, January 6, 2020 10:44 AM
>>> To: Kinney, Michael D <michael.d.kinney@intel.com>
>>> Cc: devel@edk2.groups.io
>>> Subject: Re: [edk2-devel] [PATCH v3 0/1] Add PCD to
>>> disable safe string constraint assertions
>>>
>>> Hi Mike,
>>>
>>> Yes, the primary use case is for UEFI Applications. We
>>> do not want to disable ASSERT’s completely, as
>>> assertions that make sense, i.e. the ones signalising
>>> about interface misuse, are helpful for debugging.
>>>
>>> I have already explained in the BZ that basically all
>>> safe string constraint assertions make no sense for
>>> handling untrusted data. We find this use case very
>>> logical, as these functions behave properly with
>>> assertions disabled and cover all these error
>>> conditions by the return statuses. In such situation is
>>> not useful for these functions to assert, as we end up
>>> inefficiently reimplementing the logic. I would have
>>> liked the approach of discussing the interfaces
>>> individually, but I struggle to find any that makes
>>> sense from this point of view.
>>>
>>> AsciiStrToGuid will ASSERT when the length of the
>>> passed string is odd. Functions that cannot, ahem,
>>> parse, for us are pretty much useless.
>>> AsciiStrCatS will ASSERT when the appended string does
>>> not fit the buffer. For us this logic makes this
>>> function pretty much equivalent to deprecated and thus
>>> unavailable AsciiStrCat, except it is also slower.
>>>
>>> My original suggestion was to remove the assertions
>>> entirely, but several people here said that they use
>>> them to verify usage errors when handling trusted data.
>>> This makes good sense to me, so we suggest to support
>>> both cases by introducing a PCD in this patch.
>>>
>>> Best wishes,
>>> Vitaly
>>>
>>>> 6 янв. 2020 г., в 21:28, Kinney, Michael D
>>> <michael.d.kinney@intel.com> написал(а):
>>>>
>>>>
>>>> Hi Vitaly,
>>>>
>>>> Is the use case for UEFI Applications?
>>>>
>>>> There is a different mechanism to disable all
>>> ASSERT()
>>>> statements within a UEFI Application.
>>>>
>>>> If a component is consuming data from an untrusted
>>> source,
>>>> then that component is required to verify the
>>> untrusted
>>>> data before passing it to a function that clearly
>>> documents
>>>> is input requirements. If this approach is followed,
>>> then
>>>> the BaseLib functions can be used "as is" as long as
>>> the
>>>> ASSERT() conditions are verified before calling.
>>>>
>>>> If there are some APIs that currently document their
>>> ASSERT()
>>>> behavior and we think that ASSERT() behavior is
>>> incorrect and
>>>> should be handled by an existing error return value,
>>> then we
>>>> should discuss each of those APIs individually.
>>>>
>>>> Mike
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On
>>>>> Behalf Of Vitaly Cheptsov via Groups.Io
>>>>> Sent: Friday, January 3, 2020 9:13 AM
>>>>> To: devel@edk2.groups.io
>>>>> Subject: [edk2-devel] [PATCH v3 0/1] Add PCD to
>>> disable
>>>>> safe string constraint assertions
>>>>>
>>>>> REF:
>>>>> https://bugzilla.tianocore.org/show_bug.cgi?id=2054
>>>>>
>>>>> Requesting for merge in edk2-stable202002.
>>>>>
>>>>> Changes since V1:
>>>>> - Enable assertions by default to preserve the
>>> original
>>>>> behaviour
>>>>> - Fix bugzilla reference link
>>>>> - Update documentation in BaseLib.h
>>>>>
>>>>> Vitaly Cheptsov (1):
>>>>> MdePkg: Add PCD to disable safe string constraint
>>>>> assertions
>>>>>
>>>>> MdePkg/MdePkg.dec | 6 ++
>>>>> MdePkg/Library/BaseLib/BaseLib.inf | 11 +--
>>>>> MdePkg/Include/Library/BaseLib.h | 74
>>>>> +++++++++++++-------
>>>>> MdePkg/Library/BaseLib/SafeString.c | 4 +-
>>>>> MdePkg/MdePkg.uni | 6 ++
>>>>> 5 files changed, 71 insertions(+), 30 deletions(-)
>>>>>
>>>>> --
>>>>> 2.21.0 (Apple Git-122.2)
>>>>>
>>>>>
>>>>> -=-=-=-=-=-=
>>>>> Groups.io Links: You receive all messages sent to
>>> this
>>>>> group.
>>>>>
>>>>> View/Reply Online (#52837):
>>>>> https://edk2.groups.io/g/devel/message/52837
>>>>> Mute This Topic:
>>> https://groups.io/mt/69401948/1643496
>>>>> Group Owner: devel+owner@edk2.groups.io
>>>>> Unsubscribe: https://edk2.groups.io/g/devel/unsub
>>>>> [michael.d.kinney@intel.com]
>>>>> -=-=-=-=-=-=
>>>>
>>>
>>>
>>>
>>
>