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

["Michael D Kinney" <michael.d.kinney@intel.com>]

Hao Wu,

Thanks for the detailed feedback on the size
Parameters.  I have fixed all these issues in
V2.  I also found some additional functions
that needed to be updated to safe string 
functions for the POSIX host module.

I have verified the EmulatorPkg builds and 
boots on Windows and Linux for IA32 and X64
with these patches applied.

Mike

> -----Original Message-----
> From: Wu, Hao A
> Sent: Wednesday, August 7, 2019 12:59 AM
> To: devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Justen, Jordan L <jordan.l.justen@intel.com>;
> Andrew Fish <afish@apple.com>; Ni, Ray
> <ray.ni@intel.com>
> Subject: RE: [edk2-devel] [Patch 3/3] EmulatorPkg: Add
> -D DISABLE_NEW_DEPRECATED_INTERFACES
> 
> 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/ThunkProtocolLi
> st.c
> >
> b/EmulatorPkg/Library/ThunkProtocolList/ThunkProtocolLi
> st.c
> > index b7aacc851c..3a7b6d1ceb 100644
> > ---
> a/EmulatorPkg/Library/ThunkProtocolList/ThunkProtocolLi
> st.c
> > +++
> b/EmulatorPkg/Library/ThunkProtocolList/ThunkProtocolLi
> st.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
> >
> >
> >