Re: [PATCH V2] SecurityPkg OpalPasswordDxe:Fix wrong BufferSize input to UnicodeSPrint

["Yao, Jiewen" <jiewen.yao@intel.com>]

Reviewed-by: Jiewen.yao@intel.com

> -----Original Message-----
> From: Zeng, Star
> Sent: Thursday, March 15, 2018 1:53 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Dong, Eric <eric.dong@intel.com>; Zhang, Chao B <chao.b.zhang@intel.com>
> Subject: [PATCH V2] SecurityPkg OpalPasswordDxe:Fix wrong BufferSize input to
> UnicodeSPrint
> 
> Current code uses string length as BufferSize input to UnicodeSPrint,
> it is wrong and makes the pop up string trimmed. The BufferSize input
> to UnicodeSPrint should be the size, in bytes, of the output buffer.
> 
> This is to use sizeof (mPopUpString) as the BufferSize input to
> UnicodeSPrint, it also updates array size of mPopUpString from 256 to
> 100 that is enough, otherwise the pop up string may be too long.
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
>  SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c | 15 +++++----------
>  1 file changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> index 1b55bbe4ecb8..6344deb86750 100644
> --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
> @@ -27,7 +27,7 @@ EFI_GUID mOpalDeviceNvmeGuid =
> OPAL_DEVICE_NVME_GUID;
>  BOOLEAN                 mOpalEndOfDxe = FALSE;
>  OPAL_REQUEST_VARIABLE   *mOpalRequestVariable = NULL;
>  UINTN                   mOpalRequestVariableSize = 0;
> -CHAR16                  mPopUpString[256];
> +CHAR16                  mPopUpString[100];
> 
>  typedef struct {
>    UINT32                   Address;
> @@ -908,9 +908,9 @@ OpalDriverPopUpPasswordInput (
>  }
> 
>  /**
> -  Check if disk is locked, show popup window and ask for password if it is.
> +  Get pop up string.
> 
> -  @param[in] Dev            The device which need to be unlocked.
> +  @param[in] Dev            The OPAL device.
>    @param[in] RequestString  Request string.
> 
>  **/
> @@ -920,15 +920,10 @@ OpalGetPopUpString (
>    IN CHAR16             *RequestString
>    )
>  {
> -  UINTN                 StrLength;
> -
> -  StrLength = StrLen (RequestString) + 1 + MAX (StrLen (Dev->Name16), StrLen
> (L"Disk"));
> -  ASSERT (StrLength < sizeof (mPopUpString) / sizeof (CHAR16));
> -
>    if (Dev->Name16 == NULL) {
> -    UnicodeSPrint (mPopUpString, StrLength + 1, L"%s Disk", RequestString);
> +    UnicodeSPrint (mPopUpString, sizeof (mPopUpString), L"%s Disk",
> RequestString);
>    } else {
> -    UnicodeSPrint (mPopUpString, StrLength + 1, L"%s %s", RequestString,
> Dev->Name16);
> +    UnicodeSPrint (mPopUpString, sizeof (mPopUpString), L"%s %s",
> RequestString, Dev->Name16);
>    }
> 
>    return mPopUpString;
> --
> 2.7.0.windows.1