Wu, Hao A
2015-06-24 06:30:11 UTC
Thanks a lot. I will update the patch and send out for review latter.
Best Regards,
Hao Wu
Best Regards,
Hao Wu
-----Original Message-----
Sent: Tuesday, June 23, 2015 11:55 PM
Subject: Re: [edk2] [PATCH 5/8] IntelFrameworkModulePkg BootMaint: Use
safe string functions
]Sent: Tuesday, June 23, 2015 12:24 AM
]Subject: [edk2] [PATCH 5/8] IntelFrameworkModulePkg BootMaint: Use
safe string functions
]
]Contributed-under: TianoCore Contribution Agreement 1.0
]---
] .../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
]
] 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
]
] 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;
] {
] UINTN Size1;
] UINTN Size2;
]+ UINTN MaxLen;
] CHAR16 *Str;
] CHAR16 *TmpStr;
] CHAR16 *Ptr;
]
] 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;
] //
]
] //
]- // 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) == '\\') {
] //
] //
]
] //
]- // 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
] 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
] #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
] 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
] //
] 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
] 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
] );
]
] if (*DescriptionData == 0x0000) {
]- StrCpy (DescriptionData, DriverString);
]+ StrCpyS (DescriptionData, DESCRIPTION_DATA_SIZE, DriverString);
] }
]
] BufferSize = sizeof (UINT32) + sizeof (UINT16) + StrSize (DescriptionData);
] 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-
]+ );
] }
]
] BufferSize = sizeof (UINT32) + sizeof (UINT16) + StrSize (NvRamMap-
]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
https://lists.sourceforge.net/lists/listinfo/edk2-devel
Sent: Tuesday, June 23, 2015 11:55 PM
Subject: Re: [edk2] [PATCH 5/8] IntelFrameworkModulePkg BootMaint: Use
safe string functions
]Sent: Tuesday, June 23, 2015 12:24 AM
]Subject: [edk2] [PATCH 5/8] IntelFrameworkModulePkg BootMaint: Use
safe string functions
]
]Contributed-under: TianoCore Contribution Agreement 1.0
]---
] .../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
]
] 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
]
] 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;
] {
] UINTN Size1;
] UINTN Size2;
]+ UINTN MaxLen;
] CHAR16 *Str;
] CHAR16 *TmpStr;
] CHAR16 *Ptr;
]
] 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;
] //
]
] //
]- // 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) == '\\') {
] //
] //
]
] //
]- // 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
] 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
] #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
] 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
] //
] 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
] 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
] );
]
] if (*DescriptionData == 0x0000) {
]- StrCpy (DescriptionData, DriverString);
]+ StrCpyS (DescriptionData, DESCRIPTION_DATA_SIZE, DriverString);
] }
]
] BufferSize = sizeof (UINT32) + sizeof (UINT16) + StrSize (DescriptionData);
] 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
https://lists.sourceforge.net/lists/listinfo/edk2-devel