MdeModulePkg/DxeCore: Add ASSERT to ensure no subtract underflow

[edk2] [PATCH] MdeModulePkg/DxeCore: Add ASSERT to ensure no subtract underflow

Posted by Hao Wu 7 years, 6 months ago

For function SplitRecord() in file PropertiesTable.c, there is a
potential subtract underflow case for line:

  return TotalNewRecordCount - 1;

However, such case will not happen since the logic in function
SplitTable() ensure that when calling SplitRecord(), the variable
'TotalNewRecordCount' will not be zero when performing the subtraction.
It will be handled in the previous if statement:

  if (MaxSplitRecordCount == 0) {
    CopyMem (NewRecord, OldRecord, DescriptorSize);
    return 0;
  }

Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
index e7c4a95712..6cf5edcbe5 100644
--- a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
+++ b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
@@ -576,6 +576,11 @@ SplitRecord (
     TempRecord.NumberOfPages = EfiSizeToPages (PhysicalEnd - PhysicalStart);
   } while ((ImageRecord != NULL) && (PhysicalStart < PhysicalEnd));
 
+  //
+  // The logic in function SplitTable() ensures that TotalNewRecordCount will not be zero if the
+  // code reaches here.
+  //
+  ASSERT (TotalNewRecordCount != 0);
   return TotalNewRecordCount - 1;
 }
 
-- 
2.12.0.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdeModulePkg/DxeCore: Add ASSERT to ensure no subtract underflow

Posted by Yao, Jiewen 7 years, 6 months ago

Reviewed-by: jiewen.yao@intel.com

> -----Original Message-----
> From: Wu, Hao A
> Sent: Monday, April 10, 2017 2:16 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: [PATCH] MdeModulePkg/DxeCore: Add ASSERT to ensure no subtract
> underflow
> 
> For function SplitRecord() in file PropertiesTable.c, there is a
> potential subtract underflow case for line:
> 
>   return TotalNewRecordCount - 1;
> 
> However, such case will not happen since the logic in function
> SplitTable() ensure that when calling SplitRecord(), the variable
> 'TotalNewRecordCount' will not be zero when performing the subtraction.
> It will be handled in the previous if statement:
> 
>   if (MaxSplitRecordCount == 0) {
>     CopyMem (NewRecord, OldRecord, DescriptorSize);
>     return 0;
>   }
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> ---
>  MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
> b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
> index e7c4a95712..6cf5edcbe5 100644
> --- a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
> +++ b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
> @@ -576,6 +576,11 @@ SplitRecord (
>      TempRecord.NumberOfPages = EfiSizeToPages (PhysicalEnd -
> PhysicalStart);
>    } while ((ImageRecord != NULL) && (PhysicalStart < PhysicalEnd));
> 
> +  //
> +  // The logic in function SplitTable() ensures that TotalNewRecordCount will
> not be zero if the
> +  // code reaches here.
> +  //
> +  ASSERT (TotalNewRecordCount != 0);
>    return TotalNewRecordCount - 1;
>  }
> 
> --
> 2.12.0.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel