Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
Because the Armada RTC uses a 32-bit counter for seconds,
the maximum time span is just over 136 years.
Time is stored in Unix Epoch format, so it starts in 1970,
Therefore it can not exceed the year 2106.
The issue emerged during ACS test case, which does not pass
Unix Epoch-relative time and caused EfiTimeToEpoch to assert.
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c
index a811fd368e..40ab01ed41 100644
--- a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c
+++ b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c
@@ -179,6 +179,16 @@ LibSetWakeupTime (
{
UINTN WakeupSeconds;
+ //
+ // Because the Armada RTC uses a 32-bit counter for seconds,
+ // the maximum time span is just over 136 years.
+ // Time is stored in Unix Epoch format, so it starts in 1970,
+ // Therefore it can not exceed the year 2106.
+ //
+ if ((Time->Year < 1970) || (Time->Year >= 2106)) {
+ return EFI_UNSUPPORTED;
+ }
+
// Convert time to raw seconds
WakeupSeconds = EfiTimeToEpoch (Time);
if (WakeupSeconds > MAX_UINT32) {
--
2.29.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69335): https://edk2.groups.io/g/devel/message/69335
Mute This Topic: https://groups.io/mt/79129320/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hi Marcin, On 12/21/20 17:37, Marcin Wojtas wrote: > Because the Armada RTC uses a 32-bit counter for seconds, > the maximum time span is just over 136 years. > Time is stored in Unix Epoch format, so it starts in 1970, > Therefore it can not exceed the year 2106. > > The issue emerged during ACS test case, which does not pass > Unix Epoch-relative time and caused EfiTimeToEpoch to assert. > > Signed-off-by: Marcin Wojtas <mw@semihalf.com> > --- > Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c > index a811fd368e..40ab01ed41 100644 > --- a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c > +++ b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c > @@ -179,6 +179,16 @@ LibSetWakeupTime ( > { > UINTN WakeupSeconds; > > + // > + // Because the Armada RTC uses a 32-bit counter for seconds, > + // the maximum time span is just over 136 years. > + // Time is stored in Unix Epoch format, so it starts in 1970, > + // Therefore it can not exceed the year 2106. > + // > + if ((Time->Year < 1970) || (Time->Year >= 2106)) { > + return EFI_UNSUPPORTED; > + } > + > // Convert time to raw seconds > WakeupSeconds = EfiTimeToEpoch (Time); > if (WakeupSeconds > MAX_UINT32) { > please see: - edk2-platforms commit fbdfe8c4100d ("Silicon/Marvell/RealTimeClockLib: make EpochSeconds, WakeupSeconds UINTN", 2020-12-21) -- part of this is already visible in the context above, - edk2 commit c06635ea3f4b ("EmbeddedPkg/TimeBaseLib: remove useless truncation to 32-bit", 2020-12-21). If you advance the edk2 submodule reference in edk2-platforms to or past c06635ea3f4b, then the "Time->Year >= 2106" condition will be covered already, by the (WakeupSeconds > MAX_UINT32) check. Preventing ASSERT (JulianDate >= EPOCH_JULIAN_DATE) from firing in EfiGetEpochDays() still makes sense, so the (Time->Year < 1970) check can still be added usefully, I think. Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#69340): https://edk2.groups.io/g/devel/message/69340 Mute This Topic: https://groups.io/mt/79129320/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 12/21/20 21:09, Laszlo Ersek wrote: > Hi Marcin, > > On 12/21/20 17:37, Marcin Wojtas wrote: >> Because the Armada RTC uses a 32-bit counter for seconds, >> the maximum time span is just over 136 years. >> Time is stored in Unix Epoch format, so it starts in 1970, >> Therefore it can not exceed the year 2106. >> >> The issue emerged during ACS test case, which does not pass >> Unix Epoch-relative time and caused EfiTimeToEpoch to assert. >> >> Signed-off-by: Marcin Wojtas <mw@semihalf.com> >> --- >> Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c >> index a811fd368e..40ab01ed41 100644 >> --- a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c >> +++ b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c >> @@ -179,6 +179,16 @@ LibSetWakeupTime ( >> { >> UINTN WakeupSeconds; >> >> + // >> + // Because the Armada RTC uses a 32-bit counter for seconds, >> + // the maximum time span is just over 136 years. >> + // Time is stored in Unix Epoch format, so it starts in 1970, >> + // Therefore it can not exceed the year 2106. >> + // >> + if ((Time->Year < 1970) || (Time->Year >= 2106)) { >> + return EFI_UNSUPPORTED; >> + } >> + >> // Convert time to raw seconds >> WakeupSeconds = EfiTimeToEpoch (Time); >> if (WakeupSeconds > MAX_UINT32) { >> > > please see: > > - edk2-platforms commit fbdfe8c4100d ("Silicon/Marvell/RealTimeClockLib: > make EpochSeconds, WakeupSeconds UINTN", 2020-12-21) -- part of this is > already visible in the context above, > > - edk2 commit c06635ea3f4b ("EmbeddedPkg/TimeBaseLib: remove useless > truncation to 32-bit", 2020-12-21). > > If you advance the edk2 submodule reference in edk2-platforms to or past > c06635ea3f4b, then the "Time->Year >= 2106" condition will be covered > already, by the (WakeupSeconds > MAX_UINT32) check. (Oops, sorry, I forgot that edk2-platforms consumes edk2 *only* through PACKAGES_PATH, and never through a submodule; so please adjust the above submodule statement accordingly.) BTW, is this library instance ever built for 32-bit ARM? Because in that case (== UINTN meaning UINT32), checking Year against 2106 makes sense as well. While build-testing the above-noted patches, I tried to build them for 32-bit ARM too, but that arch did not seem applicable: while the following command works: build \ -a AARCH64 \ -b NOOPT \ -p Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.dsc \ -t GCC5 \ -m EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf the same with "-a ARM" fails, with the following error message: edk2-platforms/Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.dsc(...): error 4000: Instance of library class [ArmSoftFloatLib] is not found in [edk2/CryptoPkg/Library/OpensslLib/OpensslLib.inf] [ARM] consumed by module [edk2/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareReportDxe.inf] which implies that "Armada80x0McBin.dsc" is never actually built for ARM. ... Hm, the following platforms: - Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc - Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc do build the library for ARM. So this patch seems justified after all, as-posted (i.e., including the year 2106 check). Reviewed-by: Laszlo Ersek <lersek@redhat.com> Thanks Laszlo > > Preventing ASSERT (JulianDate >= EPOCH_JULIAN_DATE) from firing in > EfiGetEpochDays() still makes sense, so the (Time->Year < 1970) check > can still be added usefully, I think. > > Thanks > Laszlo > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#69342): https://edk2.groups.io/g/devel/message/69342 Mute This Topic: https://groups.io/mt/79129320/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 12/21/20 9:32 PM, Laszlo Ersek wrote: > On 12/21/20 21:09, Laszlo Ersek wrote: >> Hi Marcin, >> >> On 12/21/20 17:37, Marcin Wojtas wrote: >>> Because the Armada RTC uses a 32-bit counter for seconds, >>> the maximum time span is just over 136 years. >>> Time is stored in Unix Epoch format, so it starts in 1970, >>> Therefore it can not exceed the year 2106. >>> >>> The issue emerged during ACS test case, which does not pass >>> Unix Epoch-relative time and caused EfiTimeToEpoch to assert. >>> >>> Signed-off-by: Marcin Wojtas <mw@semihalf.com> >>> --- >>> Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c | 10 ++++++++++ >>> 1 file changed, 10 insertions(+) >>> >>> diff --git a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c >>> index a811fd368e..40ab01ed41 100644 >>> --- a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c >>> +++ b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c >>> @@ -179,6 +179,16 @@ LibSetWakeupTime ( >>> { >>> UINTN WakeupSeconds; >>> >>> + // >>> + // Because the Armada RTC uses a 32-bit counter for seconds, >>> + // the maximum time span is just over 136 years. >>> + // Time is stored in Unix Epoch format, so it starts in 1970, >>> + // Therefore it can not exceed the year 2106. >>> + // >>> + if ((Time->Year < 1970) || (Time->Year >= 2106)) { >>> + return EFI_UNSUPPORTED; >>> + } >>> + >>> // Convert time to raw seconds >>> WakeupSeconds = EfiTimeToEpoch (Time); >>> if (WakeupSeconds > MAX_UINT32) { >>> >> >> please see: >> >> - edk2-platforms commit fbdfe8c4100d ("Silicon/Marvell/RealTimeClockLib: >> make EpochSeconds, WakeupSeconds UINTN", 2020-12-21) -- part of this is >> already visible in the context above, >> >> - edk2 commit c06635ea3f4b ("EmbeddedPkg/TimeBaseLib: remove useless >> truncation to 32-bit", 2020-12-21). >> >> If you advance the edk2 submodule reference in edk2-platforms to or past >> c06635ea3f4b, then the "Time->Year >= 2106" condition will be covered >> already, by the (WakeupSeconds > MAX_UINT32) check. > > (Oops, sorry, I forgot that edk2-platforms consumes edk2 *only* through PACKAGES_PATH, and never through a submodule; so please adjust the above submodule statement accordingly.) > > BTW, is this library instance ever built for 32-bit ARM? Because in that case (== UINTN meaning UINT32), checking Year against 2106 makes sense as well. While build-testing the above-noted patches, I tried to build them for 32-bit ARM too, but that arch did not seem applicable: while the following command works: > > build \ > -a AARCH64 \ > -b NOOPT \ > -p Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.dsc \ > -t GCC5 \ > -m EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf > > the same with "-a ARM" fails, with the following error message: > > edk2-platforms/Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.dsc(...): error 4000: Instance of library class [ArmSoftFloatLib] is not found > in [edk2/CryptoPkg/Library/OpensslLib/OpensslLib.inf] [ARM] > consumed by module [edk2/SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareReportDxe.inf] > > which implies that "Armada80x0McBin.dsc" is never actually built for ARM. > > ... Hm, the following platforms: > > - Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc > - Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc > > do build the library for ARM. So this patch seems justified after all, as-posted (i.e., including the year 2106 check). > > Reviewed-by: Laszlo Ersek <lersek@redhat.com> > Pushed as 94e9fba43d7e..794979b1ee14 Thanks! -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#69601): https://edk2.groups.io/g/devel/message/69601 Mute This Topic: https://groups.io/mt/79129320/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.