[PATCH 0/5] BaseTools/Common & DevicePath: Code refinements

[PATCH 0/5] BaseTools/Common & DevicePath: Code refinements

[Hao Wu]

The series refines the C/C++ source codes within
BaseTools/C/Source/Common/ and BaseTools/Source/C/DevicePath/ for the
below catagories:

* Resolve possible resource/memory leaks

* Resolve possible NULL pointer dereferences

Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Cc: Liming Gao <liming.gao@intel.com>

Hao Wu (5):
  BaseTools/C/Common: Fix code to be more readable
  BaseTools/C/Common: Fix potential memory leak
  BaseTools/DevicePath: Fix potential memory leak
  BaseTools/C/Common: Fix potential null pointer dereference
  BaseTools/DevicePath: Fix potential null pointer dereference

 BaseTools/Source/C/Common/CommonLib.c              |  2 +-
 BaseTools/Source/C/Common/PcdValueCommon.c         | 10 ++++++++++
 BaseTools/Source/C/DevicePath/DevicePath.c         |  6 ++++++
 BaseTools/Source/C/DevicePath/DevicePathFromText.c | 11 +++++++++++
 4 files changed, 28 insertions(+), 1 deletion(-)

-- 
2.12.0.windows.1

[PATCH 1/5] BaseTools/C/Common: Fix code to be more readable

[Hao Wu]

The change doesn't impact the functionality.

Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/Common/CommonLib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Source/C/Common/CommonLib.c b/BaseTools/Source/C/Common/CommonLib.c
index 90cc578e61..0e9aaa3735 100644
--- a/BaseTools/Source/C/Common/CommonLib.c
+++ b/BaseTools/Source/C/Common/CommonLib.c
@@ -2032,8 +2032,8 @@ StrToIpv6Address (
     return RETURN_UNSUPPORTED;
   }
   memcpy (&Address->Addr[0], &LocalAddress.Addr[0], CompressStart);
-  memset (&Address->Addr[CompressStart], 0,  ARRAY_SIZE (Address->Addr) - AddressIndex);
   if (AddressIndex > CompressStart) {
+    memset (&Address->Addr[CompressStart], 0,  ARRAY_SIZE (Address->Addr) - AddressIndex);
     memcpy (
       &Address->Addr[CompressStart + ARRAY_SIZE (Address->Addr) - AddressIndex],
       &LocalAddress.Addr[CompressStart],
-- 
2.12.0.windows.1

[PATCH 2/5] BaseTools/C/Common: Fix potential memory leak

[Hao Wu]

Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/Common/PcdValueCommon.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/BaseTools/Source/C/Common/PcdValueCommon.c b/BaseTools/Source/C/Common/PcdValueCommon.c
index 42f76ddbbc..611085cd2d 100644
--- a/BaseTools/Source/C/Common/PcdValueCommon.c
+++ b/BaseTools/Source/C/Common/PcdValueCommon.c
@@ -106,6 +106,9 @@ Returns:
   case 5:
     PcdList[PcdIndex].Value = Token;
     break;
+  default:
+    free (Token);
+    break;
   }
 }
 
-- 
2.12.0.windows.1

[PATCH 3/5] BaseTools/DevicePath: Fix potential memory leak

[Hao Wu]

Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/DevicePath/DevicePath.c         |  1 +
 BaseTools/Source/C/DevicePath/DevicePathFromText.c | 11 +++++++++++
 2 files changed, 12 insertions(+)

diff --git a/BaseTools/Source/C/DevicePath/DevicePath.c b/BaseTools/Source/C/DevicePath/DevicePath.c
index 76b8553b71..4f859a0e44 100644
--- a/BaseTools/Source/C/DevicePath/DevicePath.c
+++ b/BaseTools/Source/C/DevicePath/DevicePath.c
@@ -190,5 +190,6 @@ int main(int argc, CHAR8 *argv[])
   }
   PrintMem (DevicePath, DevicePath->Length[0] | DevicePath->Length[1] << 8);
   putchar('\n');
+  free(Str16);
   return STATUS_SUCCESS;
 }
diff --git a/BaseTools/Source/C/DevicePath/DevicePathFromText.c b/BaseTools/Source/C/DevicePath/DevicePathFromText.c
index 3d2f5a811c..120f2a38a2 100644
--- a/BaseTools/Source/C/DevicePath/DevicePathFromText.c
+++ b/BaseTools/Source/C/DevicePath/DevicePathFromText.c
@@ -3277,6 +3277,17 @@ UefiDevicePathLibConvertTextToDeviceNode (
     //
     FromText = DevPathFromTextFilePath;
     DeviceNode = FromText (DeviceNodeStr);
+    //
+    // According to above logic, if 'FromText' is NULL in the 'if' statement,
+    // then 'ParamStr' must be NULL as well. No memory allocation has been made
+    // in this case.
+    //
+    // The below check is for addressing a false positive potential memory leak
+    // issue raised from static analysis.
+    //
+    if (ParamStr != NULL) {
+      free (ParamStr);
+    }
   } else {
     DeviceNode = FromText (ParamStr);
     free (ParamStr);
-- 
2.12.0.windows.1

[PATCH 4/5] BaseTools/C/Common: Fix potential null pointer dereference

[Hao Wu]

Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/Common/PcdValueCommon.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/BaseTools/Source/C/Common/PcdValueCommon.c b/BaseTools/Source/C/Common/PcdValueCommon.c
index 611085cd2d..4b4f137ecf 100644
--- a/BaseTools/Source/C/Common/PcdValueCommon.c
+++ b/BaseTools/Source/C/Common/PcdValueCommon.c
@@ -72,6 +72,9 @@ Returns:
   CHAR8  *Token;
 
   Token = malloc (TokenEnd - TokenStart + 1);
+  if (Token == NULL) {
+    return;
+  }
   memcpy (Token, &FileBuffer[TokenStart], TokenEnd - TokenStart);
   Token[TokenEnd - TokenStart] = 0;
   switch (TokenIndex) {
@@ -333,6 +336,10 @@ Returns:
       Value = End + 1;
     }
     Buffer = malloc(*Size);
+    if (Buffer == NULL) {
+      *Size = 0;
+      return NULL;
+    }
     Value = &PcdList[Index].Value[1];
     for (*Size = 0, Buffer[*Size] = (UINT8) strtoul(Value, &End, 16); Value != End; *Size = *Size + 1, Buffer[*Size] = (UINT8) strtoul(Value, &End, 16)) {
       Value = End + 1;
-- 
2.12.0.windows.1

[PATCH 5/5] BaseTools/DevicePath: Fix potential null pointer dereference

[Hao Wu]

Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 BaseTools/Source/C/DevicePath/DevicePath.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/BaseTools/Source/C/DevicePath/DevicePath.c b/BaseTools/Source/C/DevicePath/DevicePath.c
index 4f859a0e44..b67ff412eb 100644
--- a/BaseTools/Source/C/DevicePath/DevicePath.c
+++ b/BaseTools/Source/C/DevicePath/DevicePath.c
@@ -183,6 +183,11 @@ int main(int argc, CHAR8 *argv[])
   }
   Ascii2UnicodeString(Str, Str16);
   DevicePath = UefiDevicePathLibConvertTextToDevicePath(Str16);
+  if (DevicePath == NULL) {
+    fprintf(stderr, "Convert fail, Cannot convert text to a device path");
+    free(Str16);
+    return STATUS_ERROR;
+  }
   while (!((DevicePath->Type == END_DEVICE_PATH_TYPE) && (DevicePath->SubType == END_ENTIRE_DEVICE_PATH_SUBTYPE)) )
   {
     PrintMem (DevicePath, DevicePath->Length[0] | DevicePath->Length[1] << 8);
-- 
2.12.0.windows.1

Re: [PATCH 0/5] BaseTools/Common & DevicePath: Code refinements

[Zhu, Yonghong]

Please help to update copyright year when push it.

Reviewed-by: Yonghong Zhu <yonghong.zhu@intel.com>

Best Regards,
Zhu Yonghong


-----Original Message-----
From: Wu, Hao A 
Sent: Monday, January 08, 2018 1:31 PM
To: edk2-devel@lists.01.org
Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: [PATCH 0/5] BaseTools/Common & DevicePath: Code refinements

The series refines the C/C++ source codes within BaseTools/C/Source/Common/ and BaseTools/Source/C/DevicePath/ for the below catagories:

* Resolve possible resource/memory leaks

* Resolve possible NULL pointer dereferences

Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Cc: Liming Gao <liming.gao@intel.com>

Hao Wu (5):
  BaseTools/C/Common: Fix code to be more readable
  BaseTools/C/Common: Fix potential memory leak
  BaseTools/DevicePath: Fix potential memory leak
  BaseTools/C/Common: Fix potential null pointer dereference
  BaseTools/DevicePath: Fix potential null pointer dereference

 BaseTools/Source/C/Common/CommonLib.c              |  2 +-
 BaseTools/Source/C/Common/PcdValueCommon.c         | 10 ++++++++++
 BaseTools/Source/C/DevicePath/DevicePath.c         |  6 ++++++
 BaseTools/Source/C/DevicePath/DevicePathFromText.c | 11 +++++++++++
 4 files changed, 28 insertions(+), 1 deletion(-)

--
2.12.0.windows.1

Re: [PATCH 0/5] BaseTools/Common & DevicePath: Code refinements

[Gao, Liming]

Reviewed-by: Liming Gao <liming.gao@intel.com>

> -----Original Message-----
> From: Wu, Hao A
> Sent: Monday, January 8, 2018 1:31 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>; Gao, Liming
> <liming.gao@intel.com>
> Subject: [PATCH 0/5] BaseTools/Common & DevicePath: Code refinements
> 
> The series refines the C/C++ source codes within
> BaseTools/C/Source/Common/ and BaseTools/Source/C/DevicePath/ for the
> below catagories:
> 
> * Resolve possible resource/memory leaks
> 
> * Resolve possible NULL pointer dereferences
> 
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Yonghong Zhu <yonghong.zhu@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> 
> Hao Wu (5):
>   BaseTools/C/Common: Fix code to be more readable
>   BaseTools/C/Common: Fix potential memory leak
>   BaseTools/DevicePath: Fix potential memory leak
>   BaseTools/C/Common: Fix potential null pointer dereference
>   BaseTools/DevicePath: Fix potential null pointer dereference
> 
>  BaseTools/Source/C/Common/CommonLib.c              |  2 +-
>  BaseTools/Source/C/Common/PcdValueCommon.c         | 10 ++++++++++
>  BaseTools/Source/C/DevicePath/DevicePath.c         |  6 ++++++
>  BaseTools/Source/C/DevicePath/DevicePathFromText.c | 11 +++++++++++
>  4 files changed, 28 insertions(+), 1 deletion(-)
> 
> --
> 2.12.0.windows.1