From 1342d7679e10b45f0702679b521513804205cdaf Mon Sep 17 00:00:00 2001
From: Pete Batard <pete@akeo.ie>
Date: Mon, 25 Feb 2019 23:52:02 +0000
Subject: [PATCH] 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>
---
 .../VirtualRealTimeClockLib.c                 | 34 ++++++++++++++-----
 1 file 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,
-- 
2.39.2