Hi Eric, With this patch GetPerformanceCounterProperties() is called twice. I think you can use TimestampCounterFreq in the else clause.
Also, the comment blocks are no longer correct. The original comment block goes with the else clause, and you need a new comment block for the if statement that describes the check for an overflow. Mike > -----Original Message----- > From: Dong, Eric > Sent: Tuesday, August 22, 2017 10:30 PM > To: edk2-devel@lists.01.org > Cc: Kinney, Michael D <michael.d.kin...@intel.com>; Ni, Ruiyu > <ruiyu...@intel.com> > Subject: [Patch] UefiCpuPkg/MpLib: fix potential overflow > issue. > > Current calculate timeout logic may have overflow if the input > timeout value too large. This patch fix this potential overflow > issue. > > Cc: Michael Kinney <michael.d.kin...@intel.com> > Cc: Ruiyu Ni <ruiyu...@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Eric Dong <eric.d...@intel.com> > --- > UefiCpuPkg/Library/MpInitLib/MpLib.c | 30 > +++++++++++++++++++++++------- > 1 file changed, 23 insertions(+), 7 deletions(-) > > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c > b/UefiCpuPkg/Library/MpInitLib/MpLib.c > index ed1f55e..005dec4 100644 > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c > @@ -1001,6 +1001,9 @@ CalculateTimeout ( > OUT UINT64 *CurrentTime > ) > { > + UINT64 TimeoutInSeconds; > + UINT64 TimestampCounterFreq; > + > // > // Read the current value of the performance counter > // > @@ -1019,13 +1022,26 @@ CalculateTimeout ( > // in Hz. So multiply the return value with > TimeoutInMicroseconds and then divide > // it by 1,000,000, to get the number of ticks for the > timeout value. > // > - return DivU64x32 ( > - MultU64x64 ( > - GetPerformanceCounterProperties (NULL, NULL), > - TimeoutInMicroseconds > - ), > - 1000000 > - ); > + TimestampCounterFreq = GetPerformanceCounterProperties > (NULL, NULL); > + if (DivU64x64Remainder (MAX_UINT64, TimeoutInMicroseconds, > NULL) < TimestampCounterFreq) { > + // > + // Convert microseconds into seconds if direct > multiplication overflows > + // > + TimeoutInSeconds = DivU64x32 (TimeoutInMicroseconds, > 1000000); > + // > + // Assertion if the final tick count exceeds MAX_UINT64 > + // > + ASSERT (DivU64x64Remainder (MAX_UINT64, TimeoutInSeconds, > NULL) >= TimestampCounterFreq); > + return MultU64x64 (TimestampCounterFreq, > TimeoutInSeconds); > + } else { > + return DivU64x32 ( > + MultU64x64 ( > + GetPerformanceCounterProperties (NULL, NULL), Use TimestampCounterFreq instead. > + TimeoutInMicroseconds > + ), > + 1000000 > + ); > + } > } > > /** > -- > 2.7.0.windows.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel