Thanks a lot. I will update the patch and send out for review latter. Best Regards, Hao Wu
> -----Original Message----- > From: Scott Duplichan [mailto:sc...@notabs.org] > Sent: Tuesday, June 23, 2015 11:55 PM > To: edk2-devel@lists.sourceforge.net > Subject: Re: [edk2] [PATCH 5/8] IntelFrameworkModulePkg BootMaint: Use > safe string functions > > Hao Wu [mailto:hao.a...@intel.com] wrote: > > ]Sent: Tuesday, June 23, 2015 12:24 AM > ]To: edk2-devel@lists.sourceforge.net > ]Subject: [edk2] [PATCH 5/8] IntelFrameworkModulePkg BootMaint: Use > safe string functions > ] > ]Contributed-under: TianoCore Contribution Agreement 1.0 > ]Signed-off-by: Hao Wu <hao.a...@intel.com> > ]Reviewed-by: Jeff Fan <jeff....@intel.com> > ]--- > ] .../Universal/BdsDxe/BootMaint/BootOption.c | 60 +++++++++++++++++- > ---- > ] .../Universal/BdsDxe/BootMaint/FormGuid.h | 16 ++++-- > ] .../Universal/BdsDxe/BootMaint/UpdatePage.c | 9 +++- > ] .../Universal/BdsDxe/BootMaint/Variable.c | 10 ++-- > ] 4 files changed, 72 insertions(+), 23 deletions(-) > ] > ]diff --git > a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootOption.c > b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootOption.c > ]index 0a6a445..a55dd89 100644 > ]--- > a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootOption.c > ]+++ > b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootOption.c > ]@@ -5,7 +5,7 @@ > ] > ] Boot option manipulation > ] > ]-Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.<BR> > ]+Copyright (c) 2004 - 2015, 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 > ]@@ -1012,7 +1012,11 @@ BOpt_GetBootOptions ( > ] > ] NewLoadContext->Description = AllocateZeroPool > (StrSize((UINT16*)LoadOptionPtr)); > ] ASSERT (NewLoadContext->Description != NULL); > ]- StrCpy (NewLoadContext->Description, (UINT16*)LoadOptionPtr); > ]+ StrCpyS ( > ]+ NewLoadContext->Description, > ]+ StrSize((UINT16*)LoadOptionPtr) / sizeof (UINT16), > ]+ (UINT16*)LoadOptionPtr > ]+ ); > ] > ] ASSERT (NewLoadContext->Description != NULL); > ] NewMenuEntry->DisplayString = NewLoadContext->Description; > ]@@ -1089,6 +1093,7 @@ BOpt_AppendFileName ( > ] { > ] UINTN Size1; > ] UINTN Size2; > ]+ UINTN MaxLen; > ] CHAR16 *Str; > ] CHAR16 *TmpStr; > ] CHAR16 *Ptr; > ]@@ -1096,18 +1101,31 @@ BOpt_AppendFileName ( > ] > ] Size1 = StrSize (Str1); > ] Size2 = StrSize (Str2); > ]- Str = AllocateZeroPool (Size1 + Size2 + sizeof (CHAR16)); > ]+ MaxLen = (Size1 + Size2 + sizeof (CHAR16)) / sizeof (CHAR16); > ]+ Str = AllocateZeroPool (MaxLen * sizeof (CHAR16)); > ] ASSERT (Str != NULL); > ] > ]- TmpStr = AllocateZeroPool (Size1 + Size2 + sizeof (CHAR16)); > ]+ TmpStr = AllocateZeroPool (MaxLen * sizeof (CHAR16)); > ] ASSERT (TmpStr != NULL); > ] > ]- StrCat (Str, Str1); > ]+ StrCatS ( > ]+ Str, > ]+ MaxLen, > ]+ Str1 > ]+ ); > ] if (!((*Str == '\\') && (*(Str + 1) == 0))) { > ]- StrCat (Str, L"\\"); > ]+ StrCatS ( > ]+ Str, > ]+ MaxLen, > ]+ L"\\" > ]+ ); > ] } > ] > ]- StrCat (Str, Str2); > ]+ StrCatS ( > ]+ Str, > ]+ MaxLen, > ]+ Str2 > ]+ ); > ] > ] Ptr = Str; > ] LastSlash = Str; > ]@@ -1120,11 +1138,19 @@ BOpt_AppendFileName ( > ] // > ] > ] // > ]- // Use TmpStr as a backup, as StrCpy in BaseLib does not handle copy > of > two strings > ]+ // Use TmpStr as a backup, as StrCpyS in BaseLib does not handle copy > of two strings > ] // that overlap. > ] // > ]- StrCpy (TmpStr, Ptr + 3); > ]- StrCpy (LastSlash, TmpStr); > ]+ StrCpyS ( > ]+ TmpStr, > ]+ MaxLen, > ]+ Ptr + 3 > ]+ ); > ]+ StrCpyS ( > ]+ LastSlash, > ]+ MaxLen - (UINTN) (LastSlash - Str) / sizeof (CHAR16), > > Does "/ sizeof (CHAR16)" belong here? I think the subtraction of > two CHAR16 pointers already gives a result in units of CHAR16. > > ]+ TmpStr > ]+ ); > ] Ptr = LastSlash; > ] } else if (*Ptr == '\\' && *(Ptr + 1) == '.' && *(Ptr + 2) == '\\') { > ] // > ]@@ -1132,11 +1158,19 @@ BOpt_AppendFileName ( > ] // > ] > ] // > ]- // Use TmpStr as a backup, as StrCpy in BaseLib does not handle copy > of > two strings > ]+ // Use TmpStr as a backup, as StrCpyS in BaseLib does not handle copy > of two strings > ] // that overlap. > ] // > ]- StrCpy (TmpStr, Ptr + 2); > ]- StrCpy (Ptr, TmpStr); > ]+ StrCpyS ( > ]+ TmpStr, > ]+ MaxLen, > ]+ Ptr + 2 > ]+ ); > ]+ StrCpyS ( > ]+ Ptr, > ]+ MaxLen - (UINTN) (Ptr - Str) / sizeof (CHAR16), > > Does "/ sizeof (CHAR16)" belong here? I think the subtraction of > two CHAR16 pointers already gives a result in units of CHAR16. > > ]+ TmpStr > ]+ ); > ] Ptr = LastSlash; > ] } else if (*Ptr == '\\') { > ] LastSlash = Ptr; > ]diff --git > a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/FormGuid.h > b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/FormGuid.h > ]index f2e1866..bf99999 100644 > ]--- > a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/FormGuid.h > ]+++ > b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/FormGuid.h > ]@@ -1,7 +1,7 @@ > ] /** @file > ] Formset guids, form id and VarStore data structure for Boot Maintenance > Manager. > ] > ]-Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.<BR> > ]+Copyright (c) 2004 - 2015, 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 > ]@@ -219,14 +219,20 @@ typedef struct { > ] #define KEY_VALUE_SAVE_AND_EXIT_DRIVER 0x1002 > ] #define KEY_VALUE_NO_SAVE_AND_EXIT_DRIVER 0x1003 > ] > ]+// > ]+// Description data and optional data size > ]+// > ]+#define DESCRIPTION_DATA_SIZE 75 > ]+#define OPTIONAL_DATA_SIZE 127 > ]+ > ] /// > ] /// This is the data structure used by File Explorer formset > ] /// > ] typedef struct { > ]- UINT16 BootDescriptionData[75]; > ]- UINT16 BootOptionalData[127]; > ]- UINT16 DriverDescriptionData[75]; > ]- UINT16 DriverOptionalData[127]; > ]+ UINT16 BootDescriptionData[DESCRIPTION_DATA_SIZE]; > ]+ UINT16 BootOptionalData[OPTIONAL_DATA_SIZE]; > ]+ UINT16 DriverDescriptionData[DESCRIPTION_DATA_SIZE]; > ]+ UINT16 DriverOptionalData[OPTIONAL_DATA_SIZE]; > ] BOOLEAN BootOptionChanged; > ] BOOLEAN DriverOptionChanged; > ] UINT8 Active; > ]diff --git > a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/UpdatePage.c > b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/UpdatePage.c > ]index 7d5861e..78380ad 100644 > ]--- > a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/UpdatePage.c > ]+++ > b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/UpdatePage.c > ]@@ -1,7 +1,7 @@ > ] /** @file > ] Dynamically update the pages. > ] > ]-Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.<BR> > ]+Copyright (c) 2004 - 2015, 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 > ]@@ -830,7 +830,12 @@ UpdateConModePage ( > ] // > ] UnicodeValueToString (ModeString, 0, Col, 0); > ] PStr = &ModeString[0]; > ]- StrnCat (PStr, L" x ", StrLen(L" x ") + 1); > ]+ StrnCatS ( > ]+ PStr, > ]+ sizeof (ModeString) / sizeof (ModeString[0]), > ]+ L" x ", > ]+ sizeof (ModeString) / sizeof (ModeString[0]) - StrLen (PStr) - 1 > ]+ ); > ] PStr = PStr + StrLen (PStr); > ] UnicodeValueToString (PStr , 0, Row, 0); > ] > ]diff --git > a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/Variable.c > b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/Variable.c > ]index e4299ff..616549e 100644 > ]--- a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/Variable.c > ]+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/Variable.c > ]@@ -1,7 +1,7 @@ > ] /** @file > ] Variable operation that will be used by bootmaint > ] > ]-Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.<BR> > ]+Copyright (c) 2004 - 2015, 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 > ]@@ -579,7 +579,7 @@ Var_UpdateDriverOption ( > ] ); > ] > ] if (*DescriptionData == 0x0000) { > ]- StrCpy (DescriptionData, DriverString); > ]+ StrCpyS (DescriptionData, DESCRIPTION_DATA_SIZE, DriverString); > ] } > ] > ] BufferSize = sizeof (UINT32) + sizeof (UINT16) + StrSize > (DescriptionData); > ]@@ -763,7 +763,11 @@ Var_UpdateBootOption ( > ] UnicodeSPrint (BootString, sizeof (BootString), L"Boot%04x", Index); > ] > ] if (NvRamMap->BootDescriptionData[0] == 0x0000) { > ]- StrCpy (NvRamMap->BootDescriptionData, BootString); > ]+ StrCpyS ( > ]+ NvRamMap->BootDescriptionData, > ]+ sizeof (NvRamMap->BootDescriptionData) / sizeof (NvRamMap- > >BootDescriptionData[0]), > ]+ BootString > ]+ ); > ] } > ] > ] BufferSize = sizeof (UINT32) + sizeof (UINT16) + StrSize (NvRamMap- > >BootDescriptionData); > ]-- > ]1.9.5.msysgit.0 > > > > ------------------------------------------------------------------------------ > Monitor 25 network devices or servers for free with OpManager! > OpManager is web-based network management software that monitors > network devices and physical & virtual servers, alerts via email & sms > for fault. Monitor 25 devices for free with no restriction. Download now > http://ad.doubleclick.net/ddm/clk/292181274;119417398;o > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-devel ------------------------------------------------------------------------------ Monitor 25 network devices or servers for free with OpManager! OpManager is web-based network management software that monitors network devices and physical & virtual servers, alerts via email & sms for fault. Monitor 25 devices for free with no restriction. Download now http://ad.doubleclick.net/ddm/clk/292181274;119417398;o _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
