[PATCH 0/2] Add a checking step in MdePkg and Basetools

[PATCH 0/2] Add a checking step in MdePkg and Basetools

[Shenglei Zhang]

Add a checking step to verify DevicePath.

Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Shenglei Zhang (2):
  BaseTools/DevicePath: Add a checking step
  MdePkg/UefiDevicePathLib: Add a checking step

 BaseTools/Source/C/DevicePath/DevicePathUtilities.c  | 12 ++++++++++++
 .../Library/UefiDevicePathLib/DevicePathUtilities.c  | 12 ++++++++++++
 2 files changed, 24 insertions(+)

-- 
2.18.0.windows.1

[PATCH 1/2] BaseTools/DevicePath: Add a checking step

[Shenglei Zhang]

Add a checking step in DevicePathUtilities.c to verify DevicePath.

Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
---
 BaseTools/Source/C/DevicePath/DevicePathUtilities.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/BaseTools/Source/C/DevicePath/DevicePathUtilities.c b/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
index f8a41ff97d..e2efaeebf0 100644
--- a/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
+++ b/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
@@ -59,6 +59,18 @@ IsDevicePathValid (
   UINTN Size;
   UINTN NodeLength;
 
+/**
+  NULL device path is also invalid path.
+  
+  IsDevicePathValid() should return FALSE or TRUE, and not ASSERT().
+
+  This change needs to update IsDevicePathValid() API definition and implementation both.
+
+**/
+  if (DevicePath == NULL || (MaxSize > 0 && MaxSize < END_DEVICE_PATH_LENGTH)) {
+	return FALSE;
+  }
+
   ASSERT (DevicePath != NULL);
 
   if (MaxSize == 0) {
-- 
2.18.0.windows.1

[PATCH 2/2] MdePkg/UefiDevicePathLib: Add a checking step

[Shenglei Zhang]

Add a checking step in DevicePathUtilities.c to verify DevicePath.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
---
 .../Library/UefiDevicePathLib/DevicePathUtilities.c  | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c b/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c
index 665e5a4adc..f29e1e22f6 100644
--- a/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c
+++ b/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c
@@ -59,6 +59,18 @@ IsDevicePathValid (
   UINTN Size;
   UINTN NodeLength;
 
+/**
+  NULL device path is also invalid path.
+
+  IsDevicePathValid() should return FALSE or TRUE, and not ASSERT().
+
+  This change needs to update IsDevicePathValid() API definition and implementation both.
+
+**/
+  if (DevicePath == NULL || (MaxSize > 0 && MaxSize < END_DEVICE_PATH_LENGTH)) {
+    return FALSE;
+  }
+  
   ASSERT (DevicePath != NULL);
 
   if (MaxSize == 0) {
-- 
2.18.0.windows.1

Re: [PATCH 2/2] MdePkg/UefiDevicePathLib: Add a checking step

[Gao, Liming]

Shenglei:
  Please update IsDevicePathValid() API description in MdePkg\Include\Library\DevicePathLib.h, and also remove ASSERT (DevicePath != NULL); in function implementation.

Thanks
Liming
> -----Original Message-----
> From: Zhang, Shenglei
> Sent: Tuesday, December 11, 2018 2:31 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: [PATCH 2/2] MdePkg/UefiDevicePathLib: Add a checking step
> 
> Add a checking step in DevicePathUtilities.c to verify DevicePath.
> 
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
> ---
>  .../Library/UefiDevicePathLib/DevicePathUtilities.c  | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c b/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c
> index 665e5a4adc..f29e1e22f6 100644
> --- a/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c
> +++ b/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c
> @@ -59,6 +59,18 @@ IsDevicePathValid (
>    UINTN Size;
>    UINTN NodeLength;
> 
> +/**
> +  NULL device path is also invalid path.
> +
> +  IsDevicePathValid() should return FALSE or TRUE, and not ASSERT().
> +
> +  This change needs to update IsDevicePathValid() API definition and implementation both.
> +
> +**/
> +  if (DevicePath == NULL || (MaxSize > 0 && MaxSize < END_DEVICE_PATH_LENGTH)) {
> +    return FALSE;
> +  }
> +
>    ASSERT (DevicePath != NULL);
> 
>    if (MaxSize == 0) {
> --
> 2.18.0.windows.1