Re: [PATCH] MdePkg/SafeString: Directly return when length of source string is 0

[Laszlo Ersek <lersek@redhat.com>]

On 02/02/18 11:47, Ruiyu Ni wrote:
> Today's implementation of [Ascii]StrnCpyS/[Ascii]StrnCatS doesn't
> directly return the the length of source string is 0.
> 
> When length of source string is 0, it means the Source points to
> a memory that shouldn't be deferenced at all.
> So it's not proper to call StrnLenS() in such situation.
> In a pool guard enabled environment, when using shell to edit an
> existing file which contains empty line, the page fault is met.
> 
> The patch fixes the four library functions to align to the behavior
> of non-safe version: directly return when length of source string
> is 0.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> ---
>  MdePkg/Library/BaseLib/SafeString.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/MdePkg/Library/BaseLib/SafeString.c b/MdePkg/Library/BaseLib/SafeString.c
> index 68c33e9b7b..fed818ef33 100644
> --- a/MdePkg/Library/BaseLib/SafeString.c
> +++ b/MdePkg/Library/BaseLib/SafeString.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Safe String functions.
>  
> -  Copyright (c) 2014 - 2017, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.<BR>
>    This program and the accompanying materials
>    are licensed and made available under the terms and conditions of the BSD License
>    which accompanies this distribution.  The full text of the license may be found at
> @@ -317,6 +317,10 @@ StrnCpyS (
>  {
>    UINTN            SourceLen;
>  
> +  if (Length == 0) {
> +    return RETURN_SUCCESS;
> +  }
> +
>    ASSERT (((UINTN) Destination & BIT0) == 0);
>    ASSERT (((UINTN) Source & BIT0) == 0);
>  
> @@ -515,6 +519,10 @@ StrnCatS (
>    UINTN               CopyLen;
>    UINTN               SourceLen;
>  
> +  if (Length == 0) {
> +    return RETURN_SUCCESS;
> +  }
> +
>    ASSERT (((UINTN) Destination & BIT0) == 0);
>    ASSERT (((UINTN) Source & BIT0) == 0);
>  
> @@ -1894,6 +1902,10 @@ AsciiStrnCpyS (
>  {
>    UINTN            SourceLen;
>  
> +  if (Length == 0) {
> +    return RETURN_SUCCESS;
> +  }
> +
>    //
>    // 1. Neither Destination nor Source shall be a null pointer.
>    //
> @@ -2082,6 +2094,10 @@ AsciiStrnCatS (
>    UINTN               CopyLen;
>    UINTN               SourceLen;
>  
> +  if (Length == 0) {
> +    return RETURN_SUCCESS;
> +  }
> +
>    //
>    // Let CopyLen denote the value DestMax - AsciiStrnLenS(Destination, DestMax) upon entry to AsciiStrnCatS.
>    //
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>