Re: [edk2-devel] [Patch 3/3] EmulatorPkg: Add -D DISABLE_NEW_DEPRECATED_INTERFACES

["Wu, Hao A" <hao.a.wu@intel.com>]

Hello Mike

Some inline comments below:


> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Michael D Kinney
> Sent: Wednesday, August 07, 2019 12:20 PM
> To: devel@edk2.groups.io
> Cc: Justen, Jordan L; Andrew Fish; Ni, Ray
> Subject: [edk2-devel] [Patch 3/3] EmulatorPkg: Add -D
> DISABLE_NEW_DEPRECATED_INTERFACES
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=162
> 
> Update EmulatorPkg specific modules and libraries to use
> safe string functions in BaseLib and safe PcdSetxx()
> functions in PcdLib.  With these updates, the define
> DISABLE_NEW_DEPRECATED_INTERFACES is enabled in the DSC
> file.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Andrew Fish <afish@apple.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> ---
>  EmulatorPkg/EmuBusDriverDxe/EmuBusDriverDxe.c |   9 +-
>  EmulatorPkg/EmulatorPkg.dsc                   |   6 +-
>  EmulatorPkg/FlashMapPei/FlashMapPei.c         |   8 +-
>  EmulatorPkg/Library/SmbiosLib/SmbiosLib.c     |   4 +-
>  .../ThunkProtocolList/ThunkProtocolList.c     |  11 +-
>  EmulatorPkg/Unix/Host/BerkeleyPacketFilter.c  |   8 +-
>  EmulatorPkg/Unix/Host/PosixFileSystem.c       |  30 ++++-
>  EmulatorPkg/Unix/Host/X11GraphicsWindow.c     |   4 +-
>  EmulatorPkg/Win/Host/WinFileSystem.c          | 116 ++++++++++++------
>  9 files changed, 138 insertions(+), 58 deletions(-)
> 
> diff --git a/EmulatorPkg/EmuBusDriverDxe/EmuBusDriverDxe.c
> b/EmulatorPkg/EmuBusDriverDxe/EmuBusDriverDxe.c
> index 0bf6e723a1..d8380f2be9 100644
> --- a/EmulatorPkg/EmuBusDriverDxe/EmuBusDriverDxe.c
> +++ b/EmulatorPkg/EmuBusDriverDxe/EmuBusDriverDxe.c
> @@ -1,7 +1,7 @@
>  /** @file
>   Emu Bus driver
> 
> -Copyright (c) 2006 - 2011, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>  Portions copyright (c) 2011, Apple Inc. All rights reserved.
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -256,7 +256,12 @@ EmuBusDriverBindingStart (
> 
>        EmuDevice->ControllerNameTable = NULL;
> 
> -      StrnCpy (ComponentName, EmuIoThunk->ConfigString, sizeof
> (ComponentName)/sizeof (CHAR16));
> +      StrnCpyS (
> +        ComponentName,
> +        sizeof (ComponentName) / sizeof (CHAR16),
> +        EmuIoThunk->ConfigString,
> +        sizeof (ComponentName) / sizeof (CHAR16)
> +        );
> 
>        EmuDevice->DevicePath = EmuBusCreateDevicePath (
>                                    ParentDevicePath,
> diff --git a/EmulatorPkg/EmulatorPkg.dsc b/EmulatorPkg/EmulatorPkg.dsc
> index c9e4a5b34d..39a6658427 100644
> --- a/EmulatorPkg/EmulatorPkg.dsc
> +++ b/EmulatorPkg/EmulatorPkg.dsc
> @@ -412,10 +412,14 @@ [Components]
>  !include NetworkPkg/Network.dsc.inc
> 
>  [BuildOptions]
> +  #
> +  # Disable deprecated APIs.
> +  #
> +  *_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES
> +
>    MSFT:DEBUG_*_*_CC_FLAGS = /Od /Oy-
>    MSFT:NOOPT_*_*_CC_FLAGS = /Od /Oy-
> 
>    MSFT:*_*_*_DLINK_FLAGS     = /ALIGN:4096 /FILEALIGN:4096
> /SUBSYSTEM:CONSOLE
>    MSFT:DEBUG_*_*_DLINK_FLAGS =
> /EXPORT:InitializeDriver=$(IMAGE_ENTRY_POINT) /BASE:0x10000
>    MSFT:NOOPT_*_*_DLINK_FLAGS =
> /EXPORT:InitializeDriver=$(IMAGE_ENTRY_POINT) /BASE:0x10000
> -
> diff --git a/EmulatorPkg/FlashMapPei/FlashMapPei.c
> b/EmulatorPkg/FlashMapPei/FlashMapPei.c
> index 2a468e43ac..7744065dd6 100644
> --- a/EmulatorPkg/FlashMapPei/FlashMapPei.c
> +++ b/EmulatorPkg/FlashMapPei/FlashMapPei.c
> @@ -1,7 +1,7 @@
>  /*++ @file
>    PEIM to build GUIDed HOBs for platform specific flash map
> 
> -Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>  Portions copyright (c) 2011, Apple Inc. All rights reserved.
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -69,9 +69,9 @@ Returns:
>      return Status;
>    }
> 
> -  PcdSet64 (PcdFlashNvStorageVariableBase64, PcdGet64
> (PcdEmuFlashNvStorageVariableBase) + FdFixUp);
> -  PcdSet64 (PcdFlashNvStorageFtwWorkingBase64, PcdGet64
> (PcdEmuFlashNvStorageFtwWorkingBase) + FdFixUp);
> -  PcdSet64 (PcdFlashNvStorageFtwSpareBase64, PcdGet64
> (PcdEmuFlashNvStorageFtwSpareBase) + FdFixUp);
> +  PcdSet64S (PcdFlashNvStorageVariableBase64, PcdGet64
> (PcdEmuFlashNvStorageVariableBase) + FdFixUp);
> +  PcdSet64S (PcdFlashNvStorageFtwWorkingBase64, PcdGet64
> (PcdEmuFlashNvStorageFtwWorkingBase) + FdFixUp);
> +  PcdSet64S (PcdFlashNvStorageFtwSpareBase64, PcdGet64
> (PcdEmuFlashNvStorageFtwSpareBase) + FdFixUp);
> 
>    return EFI_SUCCESS;
>  }
> diff --git a/EmulatorPkg/Library/SmbiosLib/SmbiosLib.c
> b/EmulatorPkg/Library/SmbiosLib/SmbiosLib.c
> index 331122e200..3acbb23644 100644
> --- a/EmulatorPkg/Library/SmbiosLib/SmbiosLib.c
> +++ b/EmulatorPkg/Library/SmbiosLib/SmbiosLib.c
> @@ -4,7 +4,7 @@
> 
> 
>  Copyright (c) 2012, Apple Inc. All rights reserved.
> -Portitions Copyright (c) 2006 - 2012, Intel Corporation. All rights
> reserved.<BR>
> +Portitions Copyright (c) 2006 - 2019, Intel Corporation. All rights
> reserved.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -223,7 +223,7 @@ SmbiosLibUpdateUnicodeString (
>    if (Ascii == NULL) {
>      return EFI_OUT_OF_RESOURCES;
>    }
> -  UnicodeStrToAsciiStr (String, Ascii);
> +  UnicodeStrToAsciiStrS (String, Ascii, StrSize (String));
> 
>    StringIndex = StringNumber;
>    Status = gSmbios->UpdateString (gSmbios, &SmbiosHandle, &StringIndex,
> Ascii);
> diff --git a/EmulatorPkg/Library/ThunkProtocolList/ThunkProtocolList.c
> b/EmulatorPkg/Library/ThunkProtocolList/ThunkProtocolList.c
> index b7aacc851c..3a7b6d1ceb 100644
> --- a/EmulatorPkg/Library/ThunkProtocolList/ThunkProtocolList.c
> +++ b/EmulatorPkg/Library/ThunkProtocolList/ThunkProtocolList.c
> @@ -2,7 +2,7 @@
>    Emulator Thunk to abstract OS services from pure EFI code
> 
>    Copyright (c) 2008 - 2011, Apple Inc. All rights reserved.<BR>
> -  Copyright (c) 2011 - 2018, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2011 - 2019, Intel Corporation. All rights reserved.<BR>
> 
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -37,6 +37,7 @@ AddThunkProtocol (
>    IN  BOOLEAN                 EmuBusDriver
>    )
>  {
> +  UINTN                       Size;
>    CHAR16                      *StartString;
>    CHAR16                      *SubString;
>    UINTN                       Instance;
> @@ -47,8 +48,12 @@ AddThunkProtocol (
>    }
> 
>    Instance = 0;
> -  StartString = AllocatePool (StrSize (ConfigString));
> -  StrCpy (StartString, ConfigString);
> +  Size = StrSize (ConfigString);
> +  StartString = AllocatePool (Size);
> +  if (StartString == NULL) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +  StrCpyS (StartString, Size / sizeof (CHAR16), ConfigString);
>    while (*StartString != '\0') {
> 
>      //
> diff --git a/EmulatorPkg/Unix/Host/BerkeleyPacketFilter.c
> b/EmulatorPkg/Unix/Host/BerkeleyPacketFilter.c
> index e318a90740..18cb3831a4 100644
> --- a/EmulatorPkg/Unix/Host/BerkeleyPacketFilter.c
> +++ b/EmulatorPkg/Unix/Host/BerkeleyPacketFilter.c
> @@ -4,7 +4,7 @@
> 
>   Tested on Mac OS X.
> 
> -Copyright (c) 2004 - 2009, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2004 - 2019, Intel Corporation. All rights reserved.<BR>
>  Portitions copyright (c) 2011, Apple Inc. All rights reserved.
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -1016,7 +1016,11 @@ GetInterfaceMacAddr (
>      goto Exit;
>    }
> 
> -  UnicodeStrToAsciiStr (Private->Thunk->ConfigString, Private-
> >InterfaceName);
> +  UnicodeStrToAsciiStrS (
> +    Private->Thunk->ConfigString,
> +    Private->InterfaceName,
> +    StrSize (Private->Thunk->ConfigString)
> +    );
> 
>    Status = EFI_NOT_FOUND;
>    If = IfAddrs;
> diff --git a/EmulatorPkg/Unix/Host/PosixFileSystem.c
> b/EmulatorPkg/Unix/Host/PosixFileSystem.c
> index 6ba3b59d7a..b2b2d011c9 100644
> --- a/EmulatorPkg/Unix/Host/PosixFileSystem.c
> +++ b/EmulatorPkg/Unix/Host/PosixFileSystem.c
> @@ -1017,7 +1017,11 @@ PosixFileGetInfo (
>      FileSystemInfoBuffer->BlockSize   = buf.f_bsize;
> 
> 
> -    StrCpy ((CHAR16 *) FileSystemInfoBuffer->VolumeLabel, PrivateRoot-
> >VolumeLabel);
> +    StrCpyS (
> +      (CHAR16 *) FileSystemInfoBuffer->VolumeLabel,
> +      StrSize (PrivateRoot->VolumeLabel) / sizeof (CHAR16),


I think it will be better to use:

(*BufferSize - SIZE_OF_EFI_FILE_SYSTEM_INFO) / sizeof (CHAR16)

here. Even though the usage of function PosixFileGetInfo() would ensures
that:

StrSize (PrivateRoot->VolumeLabel) == *BufferSize - SIZE_OF_EFI_FILE_SYSTEM_INFO


> +      PrivateRoot->VolumeLabel
> +      );
>      *BufferSize = SIZE_OF_EFI_FILE_SYSTEM_INFO + StrSize (PrivateRoot-
> >VolumeLabel);
> 
>    } else if (CompareGuid (InformationType,
> &gEfiFileSystemVolumeLabelInfoIdGuid)) {
> @@ -1026,7 +1030,11 @@ PosixFileGetInfo (
>        return EFI_BUFFER_TOO_SMALL;
>      }
> 
> -    StrCpy ((CHAR16 *) Buffer, PrivateRoot->VolumeLabel);
> +    StrCpyS (
> +      (CHAR16 *) Buffer,
> +      StrSize (PrivateRoot->VolumeLabel) / sizeof (CHAR16),


Similarly, I would suggest using:
*BufferSize / sizeof (CHAR16)

instead of:
StrSize (PrivateRoot->VolumeLabel) / sizeof (CHAR16)


> +      PrivateRoot->VolumeLabel
> +      );
>      *BufferSize = StrSize (PrivateRoot->VolumeLabel);
> 
>    }
> @@ -1110,7 +1118,11 @@ PosixFileSetInfo (
>        goto Done;
>      }
> 
> -    StrCpy (PrivateRoot->VolumeLabel, NewFileSystemInfo->VolumeLabel);
> +    StrCpyS (
> +      PrivateRoot->VolumeLabel,
> +      StrSize (NewFileSystemInfo->VolumeLabel) / sizeof (CHAR16),
> +      NewFileSystemInfo->VolumeLabel
> +      );
> 
>      Status = EFI_SUCCESS;
>      goto Done;
> @@ -1125,7 +1137,11 @@ PosixFileSetInfo (
>        goto Done;
>      }
> 
> -    StrCpy (PrivateRoot->VolumeLabel, (CHAR16 *) Buffer);
> +    StrCpyS (
> +      PrivateRoot->VolumeLabel,
> +      StrSize (PrivateRoot->VolumeLabel) / sizeof (CHAR16),


I think the size for 'PrivateRoot->VolumeLabel' is good here.

Since within this driver, 'PrivateRoot->VolumeLabel' is always allocated
with the size to just hold the current string it stores.


> +      (CHAR16 *) Buffer
> +      );
> 
>      Status = EFI_SUCCESS;
>      goto Done;
> @@ -1493,7 +1509,11 @@ PosixFileSystmeThunkOpen (
>      free (Private);
>      return EFI_OUT_OF_RESOURCES;
>    }
> -  StrCpy (Private->VolumeLabel, L"EFI_EMULATED");
> +  StrCpyS (
> +    Private->VolumeLabel,
> +    StrSize (L"EFI_EMULATED") / sizeof (CHAR16),
> +    L"EFI_EMULATED"
> +    );
> 
>    Private->Signature = EMU_SIMPLE_FILE_SYSTEM_PRIVATE_SIGNATURE;
>    Private->Thunk     = This;
> diff --git a/EmulatorPkg/Unix/Host/X11GraphicsWindow.c
> b/EmulatorPkg/Unix/Host/X11GraphicsWindow.c
> index 9d03c13011..5325a0e35b 100644
> --- a/EmulatorPkg/Unix/Host/X11GraphicsWindow.c
> +++ b/EmulatorPkg/Unix/Host/X11GraphicsWindow.c
> @@ -1,6 +1,6 @@
>  /*++ @file
> 
> -Copyright (c) 2004 - 2011, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2004 - 2019, Intel Corporation. All rights reserved.<BR>
>  Portions copyright (c) 2008 - 2011, Apple Inc. All rights reserved.<BR>
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -957,7 +957,7 @@ X11GraphicsWindowOpen (
>    XDefineCursor (Drv->display, Drv->win, XCreateFontCursor (Drv->display,
> XC_pirate));
> 
>    Drv->Title = malloc (StrSize (This->ConfigString));
> -  UnicodeStrToAsciiStr (This->ConfigString, Drv->Title);
> +  UnicodeStrToAsciiStrS (This->ConfigString, Drv->Title, StrSize (This-
> >ConfigString));
>    XStoreName (Drv->display, Drv->win, Drv->Title);
> 
>  //  XAutoRepeatOff (Drv->display);
> diff --git a/EmulatorPkg/Win/Host/WinFileSystem.c
> b/EmulatorPkg/Win/Host/WinFileSystem.c
> index da6595228d..bb64439007 100644
> --- a/EmulatorPkg/Win/Host/WinFileSystem.c
> +++ b/EmulatorPkg/Win/Host/WinFileSystem.c
> @@ -1,7 +1,7 @@
>  /*++ @file
>    Support OS native directory access.
> 
> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
> @@ -205,8 +205,14 @@ WinNtOpenVolume (
>      goto Done;
>    }
> 
> -  StrCpy (PrivateFile->FilePath, Private->FilePath);
> -  StrCpy (PrivateFile->FileName, PrivateFile->FilePath);
> +  StrCpyS (PrivateFile->FilePath,
> +    StrSize (Private->FilePath) / sizeof (CHAR16),
> +    Private->FilePath
> +    );
> +  StrCpyS (PrivateFile->FileName,
> +    StrSize (Private->FilePath) / sizeof (CHAR16),
> +    PrivateFile->FilePath
> +    );
>    PrivateFile->Signature = WIN_NT_EFI_FILE_PRIVATE_SIGNATURE;
>    PrivateFile->Thunk = Private->Thunk;
>    PrivateFile->SimpleFileSystem = This;
> @@ -243,8 +249,8 @@ WinNtOpenVolume (
>    if (TempFileName == NULL) {
>      goto Done;
>    }
> -  StrCpy (TempFileName, PrivateFile->FilePath);
> -  StrCat (TempFileName, L"\\*");
> +  StrCpyS (TempFileName, Size / sizeof (CHAR16), PrivateFile->FilePath);
> +  StrCatS (TempFileName, Size / sizeof (CHAR16), L"\\*");
> 
>    PrivateFile->LHandle = FindFirstFile (TempFileName, &PrivateFile->FindBuf);
>    FreePool (TempFileName);
> @@ -362,7 +368,7 @@ GetNextFileNameToken (
>    } else {
>      Offset = SlashPos - *FileName;
>      Token = AllocateZeroPool ((Offset + 1) * sizeof (CHAR16));
> -    StrnCpy (Token, *FileName, Offset);
> +    StrnCpyS (Token, Offset + 1, *FileName, Offset);
>      //
>      // Point *FileName to the next character after L'\'.
>      //
> @@ -496,7 +502,7 @@ WinNtFileOpen (
>    if (TempFileName == NULL) {
>      return EFI_OUT_OF_RESOURCES;
>    }
> -  StrCpy (TempFileName, FileName);
> +  StrCpyS (TempFileName, StrSize (FileName) / sizeof (CHAR16), FileName);
>    FileName = TempFileName;
> 
>    if (FileName[StrLen (FileName) - 1] == L'\\') {
> @@ -548,9 +554,17 @@ WinNtFileOpen (
>    }
> 
>    if (PrivateFile->IsDirectoryPath) {
> -    StrCpy (NewPrivateFile->FilePath, PrivateFile->FileName);
> +    StrCpyS (
> +      NewPrivateFile->FilePath,
> +      StrSize (PrivateFile->FileName) / sizeof (CHAR16),
> +      PrivateFile->FileName
> +      );
>    } else {
> -    StrCpy (NewPrivateFile->FilePath, PrivateFile->FilePath);
> +    StrCpyS (
> +      NewPrivateFile->FilePath,
> +      StrSize (PrivateFile->FileName) / sizeof (CHAR16),
> +      PrivateFile->FilePath
> +      );
>    }
> 
>    Size = StrSize (NewPrivateFile->FilePath);
> @@ -563,17 +577,17 @@ WinNtFileOpen (
>    }
> 
>    if (*FileName == L'\\') {
> -    StrCpy (NewPrivateFile->FileName, PrivateRoot->FilePath);
> -    StrCat (NewPrivateFile->FileName, L"\\");
> -    StrCat (NewPrivateFile->FileName, FileName + 1);
> +    StrCpyS (NewPrivateFile->FileName, Size / sizeof (CHAR16), PrivateRoot-
> >FilePath);
> +    StrCatS (NewPrivateFile->FileName, Size / sizeof (CHAR16), L"\\");
> +    StrCatS (NewPrivateFile->FileName, Size / sizeof (CHAR16), FileName + 1);
>    } else {
> -    StrCpy (NewPrivateFile->FileName, NewPrivateFile->FilePath);
> +    StrCpyS (NewPrivateFile->FileName, Size / sizeof (CHAR16),
> NewPrivateFile->FilePath);
>      if (StrCmp (FileName, L"") != 0) {
>        //
>        // In case the filename becomes empty, especially after trimming dots
> and blanks
>        //
> -      StrCat (NewPrivateFile->FileName, L"\\");
> -      StrCat (NewPrivateFile->FileName, FileName);
> +      StrCatS (NewPrivateFile->FileName, Size, L"\\");
> +      StrCatS (NewPrivateFile->FileName, Size, FileName);


For the above 2 lines, the 2nd parameter for StrCatS() should be:
Size / sizeof (CHAR16)


>      }
>    }
> 
> @@ -657,7 +671,11 @@ WinNtFileOpen (
>      goto Done;
>    }
> 
> -  StrCpy (NewPrivateFile->FilePath, NewPrivateFile->FileName);
> +  StrCpyS (
> +    NewPrivateFile->FilePath,
> +    StrSize (NewPrivateFile->FileName) / sizeof (CHAR16),
> +    NewPrivateFile->FileName
> +    );
>    if (TempChar != 0) {
>      *(RealFileName - 1) = TempChar;
>    }
> @@ -715,7 +733,7 @@ WinNtFileOpen (
>        goto Done;
>      }
> 
> -    StrCpy (TempFileName, NewPrivateFile->FileName);
> +    StrCpyS (TempFileName, Size / sizeof (CHAR16), NewPrivateFile-
> >FileName);
> 
>      if ((OpenMode & EFI_FILE_MODE_CREATE)) {
>        //
> @@ -769,7 +787,7 @@ WinNtFileOpen (
>      //
>      // Find the first file under it
>      //
> -    StrCat (TempFileName, L"\\*");
> +    StrCatS (TempFileName, Size, L"\\*");


Should be:
StrCatS (TempFileName, Size / sizeof (CHAR16), L"\\*");


>      NewPrivateFile->LHandle = FindFirstFile (TempFileName,
> &NewPrivateFile->FindBuf);
>      FreePool (TempFileName);
> 
> @@ -1330,8 +1348,8 @@ WinNtFileSetPossition (
>        goto Done;
>      }
> 
> -    StrCpy (FileName, PrivateFile->FileName);
> -    StrCat (FileName, L"\\*");
> +    StrCpyS (FileName, Size / sizeof (CHAR16), PrivateFile->FileName);
> +    StrCatS (FileName, Size / sizeof (CHAR16), L"\\*");
> 
>      if (PrivateFile->LHandle != INVALID_HANDLE_VALUE) {
>        FindClose (PrivateFile->LHandle);
> @@ -1599,7 +1617,11 @@ WinNtFileGetInfo (
>        goto Done;
>      }
> 
> -    StrCpy (DriveName, PrivateFile->FilePath);
> +    StrCpyS (
> +      DriveName,
> +      (StrSize (PrivateFile->FilePath) + 1) / sizeof (CHAR16),
> +      PrivateFile->FilePath
> +      );
>      for (Index = 0; DriveName[Index] != 0 && DriveName[Index] != ':';
> Index++) {
>        ;
>      }
> @@ -1664,7 +1686,11 @@ WinNtFileGetInfo (
>        }
>      }
> 
> -    StrCpy ((CHAR16 *)FileSystemInfoBuffer->VolumeLabel, PrivateRoot-
> >VolumeLabel);
> +    StrCpyS (
> +      (CHAR16 *)FileSystemInfoBuffer->VolumeLabel,
> +      (StrSize (PrivateRoot->VolumeLabel) + 1) / sizeof (CHAR16),


I would suggest here using:
(*BufferSize - SIZE_OF_EFI_FILE_SYSTEM_INFO) / sizeof (CHAR16)


> +      PrivateRoot->VolumeLabel
> +      );
>      *BufferSize = SIZE_OF_EFI_FILE_SYSTEM_INFO + StrSize (PrivateRoot-
> >VolumeLabel);
>      Status = EFI_SUCCESS;
>    }
> @@ -1676,7 +1702,11 @@ WinNtFileGetInfo (
>        goto Done;
>      }
> 
> -    StrCpy ((CHAR16 *)Buffer, PrivateRoot->VolumeLabel);
> +    StrCpyS (
> +      (CHAR16 *)Buffer,
> +      StrSize (PrivateRoot->VolumeLabel) / sizeof (CHAR16),


I would suggest here using:
*BufferSize / sizeof (CHAR16)


> +      PrivateRoot->VolumeLabel
> +      );
>      *BufferSize = StrSize (PrivateRoot->VolumeLabel);
>      Status = EFI_SUCCESS;
>    }
> @@ -1768,7 +1798,11 @@ WinNtFileSetInfo (
>        goto Done;
>      }
> 
> -    StrCpy (PrivateRoot->VolumeLabel, NewFileSystemInfo->VolumeLabel);
> +    StrCpyS (
> +      PrivateRoot->VolumeLabel,
> +      StrSize (NewFileSystemInfo->VolumeLabel) / sizeof (CHAR16),
> +      NewFileSystemInfo->VolumeLabel
> +      );
> 
>      Status = EFI_SUCCESS;
>      goto Done;
> @@ -1783,7 +1817,11 @@ WinNtFileSetInfo (
>        goto Done;
>      }
> 
> -    StrCpy (PrivateRoot->VolumeLabel, (CHAR16 *)Buffer);
> +    StrCpyS (
> +      PrivateRoot->VolumeLabel,
> +      StrSize (PrivateRoot->VolumeLabel) / sizeof (CHAR16),


Similar to the PosixFileSetInfo() case above, I think the size for
'PrivateRoot->VolumeLabel' is good here.


> +      (CHAR16 *)Buffer
> +      );
> 
>      Status = EFI_SUCCESS;
>      goto Done;
> @@ -1852,7 +1890,11 @@ WinNtFileSetInfo (
>      goto Done;
>    }
> 
> -  StrCpy (OldFileName, PrivateFile->FileName);
> +  StrCpyS (
> +    OldFileName,
> +    StrSize (PrivateFile->FileName) / sizeof (CHAR16),
> +    PrivateFile->FileName
> +    );
> 
>    //
>    // Make full pathname from new filename and rootpath.
> @@ -1867,9 +1909,9 @@ WinNtFileSetInfo (
>        goto Done;
>      }
> 
> -    StrCpy (NewFileName, PrivateRoot->FilePath);
> -    StrCat (NewFileName, L"\\");
> -    StrCat (NewFileName, NewFileInfo->FileName + 1);
> +    StrCpyS (NewFileName, Size / sizeof (CHAR16), PrivateRoot->FilePath);
> +    StrCatS (NewFileName, Size / sizeof (CHAR16), L"\\");
> +    StrCatS (NewFileName, Size / sizeof (CHAR16), NewFileInfo->FileName +
> 1);
>    } else {
>      Size = StrSize (PrivateFile->FilePath);
>      Size += StrSize (L"\\");
> @@ -1880,9 +1922,9 @@ WinNtFileSetInfo (
>        goto Done;
>      }
> 
> -    StrCpy (NewFileName, PrivateFile->FilePath);
> -    StrCat (NewFileName, L"\\");
> -    StrCat (NewFileName, NewFileInfo->FileName);
> +    StrCpyS (NewFileName, Size, PrivateFile->FilePath);
> +    StrCatS (NewFileName, Size, L"\\");
> +    StrCatS (NewFileName, Size, NewFileInfo->FileName);


The 2nd parameter for the above StrCatS() calls should be:
Size / sizeof (CHAR16)


>    }
> 
>    //
> @@ -1990,13 +2032,13 @@ WinNtFileSetInfo (
>          goto Done;
>        }
> 
> -      StrCpy (PrivateFile->FileName, NewFileName);
> +      StrCpyS (PrivateFile->FileName, StrSize (NewFileName) / sizeof
> (CHAR16), NewFileName);
> 
>        Size = StrSize (NewFileName);
>        Size += StrSize (L"\\*");
>        TempFileName = AllocatePool (Size);
> 
> -      StrCpy (TempFileName, NewFileName);
> +      StrCpyS (TempFileName, Size / sizeof (CHAR16), NewFileName);
> 
>        if (!PrivateFile->IsDirectoryPath) {
>          PrivateFile->LHandle = CreateFile (
> @@ -2029,7 +2071,7 @@ WinNtFileSetInfo (
>            NULL
>          );
> 
> -        StrCat (TempFileName, L"\\*");
> +        StrCatS (TempFileName, Size, L"\\*");


Should be:
StrCatS (TempFileName, Size / sizeof (CHAR16), L"\\*");


>          PrivateFile->LHandle = FindFirstFile (TempFileName, &FindBuf);
> 
>          FreePool (TempFileName);
> @@ -2048,7 +2090,7 @@ WinNtFileSetInfo (
>        Size += StrSize (L"\\*");
>        TempFileName = AllocatePool (Size);
> 
> -      StrCpy (TempFileName, OldFileName);
> +      StrCpyS (TempFileName, Size / sizeof (CHAR16), OldFileName);
> 
>        if (!PrivateFile->IsDirectoryPath) {
>          PrivateFile->LHandle = CreateFile (
> @@ -2071,7 +2113,7 @@ WinNtFileSetInfo (
>            NULL
>          );
> 
> -        StrCat (TempFileName, L"\\*");
> +        StrCatS (TempFileName, Size, L"\\*");


Should be:
StrCatS (TempFileName, Size / sizeof (CHAR16), L"\\*");

Best Regards,
Hao Wu


>          PrivateFile->LHandle = FindFirstFile (TempFileName, &FindBuf);
>        }
> 
> --
> 2.21.0.windows.1
> 
> 
>