Reviewed-by: Michael Kinney <[email protected]> Mike
> -----Original Message----- > From: Dong, Eric > Sent: Wednesday, August 23, 2017 7:57 PM > To: [email protected] > Cc: Kinney, Michael D <[email protected]>; Ni, Ruiyu > <[email protected]> > 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. > > V2: Use local variable instead of call > GetPerformanceCounterProperties > twice. Also correct some comments. > > Cc: Michael Kinney <[email protected]> > Cc: Ruiyu Ni <[email protected]> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Eric Dong <[email protected]> > --- > UefiCpuPkg/Library/MpInitLib/MpLib.c | 43 > +++++++++++++++++++++++++++--------- > 1 file changed, 33 insertions(+), 10 deletions(-) > > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c > b/UefiCpuPkg/Library/MpInitLib/MpLib.c > index ed1f55e..8394572 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 > // > @@ -1016,16 +1019,36 @@ CalculateTimeout ( > > // > // GetPerformanceCounterProperties () returns the timestamp > counter's frequency > - // 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 > - ); > + // in Hz. > + // > + TimestampCounterFreq = GetPerformanceCounterProperties > (NULL, NULL); > + > + // > + // Check the potential overflow before calculate the number > of ticks for the timeout value. > + // > + 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 { > + // > + // No overflow case, 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 ( > + TimestampCounterFreq, > + TimeoutInMicroseconds > + ), > + 1000000 > + ); > + } > } > > /** > -- > 2.7.0.windows.1 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
