EmbeddedPkg/VirtualRealTimeClockLib: Fix correctness issues

LibGetTime():
- Two variables were used for the epoch, where only one should have been [*].
- Also harmonize variable name to match the one used in LibSetTime.
LibSetTime():
- Address possible underflows if time is set to start of epoch.
- Ensure that time being read does actually match time that was manually
  set (plus the time elapsed since), by subtracting number of seconds
  since reset.

[*] This fixes a build breakage, since one of these variables was set but
    never used, triggering a compiler diagnostic at some optimization levels.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Pete Batard <pete@akeo.ie>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

1 files changed, 25 insertions, 9 deletions

diff --git a/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
index 4c354730d0..8ad2df90e7 100644
--- a/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
+++ b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
@@ -54,13 +54,12 @@ LibGetTime (
   )

 {

   EFI_STATUS  Status;

-  UINT32      EpochSeconds;

   INT16       TimeZone;

   UINT8       Daylight;

   UINT64      Freq;

   UINT64      Counter;

   UINT64      Remainder;

-  UINTN       ElapsedSeconds;

+  UINTN       EpochSeconds;

   UINTN       Size;

 

   if (Time == NULL) {

@@ -75,13 +74,13 @@ LibGetTime (
 

   // Get the epoch time from non-volatile storage

   Size = sizeof (UINTN);

-  ElapsedSeconds = 0;

+  EpochSeconds = 0;

   Status = EfiGetVariable (

              (CHAR16 *)mEpochVariableName,

              &gEfiCallerIdGuid,

              NULL,

              &Size,

-             (VOID *)&ElapsedSeconds

+             (VOID *)&EpochSeconds

              );

   // Fall back to compilation-time epoch if not set

   if (EFI_ERROR (Status)) {

@@ -93,7 +92,7 @@ LibGetTime (
     // If you are attempting to use this library on such an environment, please

     // contact the edk2 mailing list, so we can try to add support for it.

     //

-    ElapsedSeconds = BUILD_EPOCH;

+    EpochSeconds = BUILD_EPOCH;

     DEBUG ((

       DEBUG_INFO,

       "LibGetTime: %s non volatile variable was not found - Using compilation time epoch.\n",

@@ -101,7 +100,7 @@ LibGetTime (
       ));

   }

   Counter = GetPerformanceCounter ();

-  ElapsedSeconds += DivU64x64Remainder (Counter, Freq, &Remainder);

+  EpochSeconds += DivU64x64Remainder (Counter, Freq, &Remainder);

 

   // Get the current time zone information from non-volatile storage

   Size = sizeof (TimeZone);

@@ -204,7 +203,7 @@ LibGetTime (
     }

   }

 

-  EpochToEfiTime (ElapsedSeconds, Time);

+  EpochToEfiTime (EpochSeconds, Time);

 

   // Because we use the performance counter, we can fill the Nanosecond attribute

   // provided that the remainder doesn't overflow 64-bit during multiplication.

@@ -240,6 +239,9 @@ LibSetTime (
   )

 {

   EFI_STATUS  Status;

+  UINT64      Freq;

+  UINT64      Counter;

+  UINT64      Remainder;

   UINTN       EpochSeconds;

 

   if (!IsTimeValid (Time)) {

@@ -249,16 +251,30 @@ LibSetTime (
   EpochSeconds = EfiTimeToEpoch (Time);

 

   // Adjust for the correct time zone, i.e. convert to UTC time zone

-  if (Time->TimeZone != EFI_UNSPECIFIED_TIMEZONE) {

+  if ((Time->TimeZone != EFI_UNSPECIFIED_TIMEZONE)

+      && (EpochSeconds > Time->TimeZone * SEC_PER_MIN)) {

     EpochSeconds -= Time->TimeZone * SEC_PER_MIN;

   }

 

   // Adjust for the correct period

-  if ((Time->Daylight & EFI_TIME_IN_DAYLIGHT) == EFI_TIME_IN_DAYLIGHT) {

+  if (((Time->Daylight & EFI_TIME_IN_DAYLIGHT) == EFI_TIME_IN_DAYLIGHT)

+      && (EpochSeconds > SEC_PER_HOUR)) {

     // Convert to un-adjusted time, i.e. fall back one hour

     EpochSeconds -= SEC_PER_HOUR;

   }

 

+  // Use the performance counter to subtract the number of seconds

+  // since platform reset. Without this, setting time from the shell

+  // and immediately reading it back would result in a forward time

+  // offset, of the duration during which the platform has been up.

+  Freq = GetPerformanceCounterProperties (NULL, NULL);

+  if (Freq != 0) {

+    Counter = GetPerformanceCounter ();

+    if (EpochSeconds > DivU64x64Remainder (Counter, Freq, &Remainder)) {

+      EpochSeconds -= DivU64x64Remainder (Counter, Freq, &Remainder);

+    }

+  }

+

   // Save the current time zone information into non-volatile storage

   Status = EfiSetVariable (

              (CHAR16 *)mTimeZoneVariableName,