[PATCH] efi: cper: avoid using get_seconds()

[PATCH] efi: cper: avoid using get_seconds()

[Arnd Bergmann]

get_seconds() is deprecated because of the 32-bit time overflow
in y2038/y2106 on 32-bit architectures. The way it is used in
cper_next_record_id() causes an overflow in 2106 when unsigned UTC
seconds overflow, even on 64-bit architectures.

This starts using ktime_get_real_seconds() to give us more than 32 bits
of timestamp on all architectures, and then changes the algorithm to use
39 bits for the timestamp after the y2038 wrap date, plus an always-1
bit at the top. This gives us another 127 epochs of 136 years, with
strictly monotonically increasing sequence numbers across boots.

This is almost certainly overkill, but seems better than just extending
the deadline from 2038 to 2106.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/firmware/efi/cper.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
index 3bf0dca378a6..b73fc4cab083 100644
--- a/drivers/firmware/efi/cper.c
+++ b/drivers/firmware/efi/cper.c
@@ -48,8 +48,21 @@ u64 cper_next_record_id(void)
 {
 	static atomic64_t seq;
 
-	if (!atomic64_read(&seq))
-		atomic64_set(&seq, ((u64)get_seconds()) << 32);
+	if (!atomic64_read(&seq)) {
+		time64_t time = ktime_get_real_seconds();
+
+		/*
+		 * This code is unlikely to still be needed in year 2106,
+		 * but just in case, let's use a few more bits for timestamps
+		 * after y2038 to be sure they keep increasing monotonically
+		 * for the next few hundred years...
+		 */
+		if (time < 0x80000000)
+			atomic64_set(&seq, (ktime_get_real_seconds()) << 32);
+		else
+			atomic64_set(&seq, 0x8000000000000000ull |
+					   ktime_get_real_seconds() << 24);
+	}
 
 	return atomic64_inc_return(&seq);
 }
-- 
2.9.0

Re: [PATCH] efi: cper: avoid using get_seconds()

[Ard Biesheuvel]

On 18 June 2018 at 16:17, Arnd Bergmann <arnd@arndb.de> wrote:
> get_seconds() is deprecated because of the 32-bit time overflow
> in y2038/y2106 on 32-bit architectures. The way it is used in
> cper_next_record_id() causes an overflow in 2106 when unsigned UTC
> seconds overflow, even on 64-bit architectures.
>
> This starts using ktime_get_real_seconds() to give us more than 32 bits
> of timestamp on all architectures, and then changes the algorithm to use
> 39 bits for the timestamp after the y2038 wrap date, plus an always-1
> bit at the top. This gives us another 127 epochs of 136 years, with
> strictly monotonically increasing sequence numbers across boots.
>
> This is almost certainly overkill, but seems better than just extending
> the deadline from 2038 to 2106.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/firmware/efi/cper.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
> index 3bf0dca378a6..b73fc4cab083 100644
> --- a/drivers/firmware/efi/cper.c
> +++ b/drivers/firmware/efi/cper.c
> @@ -48,8 +48,21 @@ u64 cper_next_record_id(void)
>  {
>         static atomic64_t seq;
>
> -       if (!atomic64_read(&seq))
> -               atomic64_set(&seq, ((u64)get_seconds()) << 32);
> +       if (!atomic64_read(&seq)) {
> +               time64_t time = ktime_get_real_seconds();
> +
> +               /*
> +                * This code is unlikely to still be needed in year 2106,
> +                * but just in case, let's use a few more bits for timestamps
> +                * after y2038 to be sure they keep increasing monotonically
> +                * for the next few hundred years...
> +                */
> +               if (time < 0x80000000)
> +                       atomic64_set(&seq, (ktime_get_real_seconds()) << 32);
> +               else
> +                       atomic64_set(&seq, 0x8000000000000000ull |
> +                                          ktime_get_real_seconds() << 24);
> +       }

Given that these values are never decoded and interpreted as
timestamps, can't we simply switch to the second flavour immediately?

>
>         return atomic64_inc_return(&seq);
>  }
> --
> 2.9.0
>

Re: [PATCH] efi: cper: avoid using get_seconds()

[Arnd Bergmann]

On Mon, Jun 18, 2018 at 5:47 PM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> On 18 June 2018 at 16:17, Arnd Bergmann <arnd@arndb.de> wrote:

>> -               atomic64_set(&seq, ((u64)get_seconds()) << 32);
>> +       if (!atomic64_read(&seq)) {
>> +               time64_t time = ktime_get_real_seconds();
>> +
>> +               /*
>> +                * This code is unlikely to still be needed in year 2106,
>> +                * but just in case, let's use a few more bits for timestamps
>> +                * after y2038 to be sure they keep increasing monotonically
>> +                * for the next few hundred years...
>> +                */
>> +               if (time < 0x80000000)
>> +                       atomic64_set(&seq, (ktime_get_real_seconds()) << 32);
>> +               else
>> +                       atomic64_set(&seq, 0x8000000000000000ull |
>> +                                          ktime_get_real_seconds() << 24);
>> +       }
>
> Given that these values are never decoded and interpreted as
> timestamps, can't we simply switch to the second flavour immediately?

I considered that, but the downside would be that all future filenames would
come before all past file names. I don't know if the order is important at
all, but the current implementation at least looks like it's intended to keep
all file names strictly sorted across boots.

         Arnd

Re: [PATCH] efi: cper: avoid using get_seconds()

[Ard Biesheuvel]

On 18 June 2018 at 17:49, Arnd Bergmann <arnd@arndb.de> wrote:
> On Mon, Jun 18, 2018 at 5:47 PM, Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
>> On 18 June 2018 at 16:17, Arnd Bergmann <arnd@arndb.de> wrote:
>
>>> -               atomic64_set(&seq, ((u64)get_seconds()) << 32);
>>> +       if (!atomic64_read(&seq)) {
>>> +               time64_t time = ktime_get_real_seconds();
>>> +
>>> +               /*
>>> +                * This code is unlikely to still be needed in year 2106,
>>> +                * but just in case, let's use a few more bits for timestamps
>>> +                * after y2038 to be sure they keep increasing monotonically
>>> +                * for the next few hundred years...
>>> +                */
>>> +               if (time < 0x80000000)
>>> +                       atomic64_set(&seq, (ktime_get_real_seconds()) << 32);
>>> +               else
>>> +                       atomic64_set(&seq, 0x8000000000000000ull |
>>> +                                          ktime_get_real_seconds() << 24);
>>> +       }
>>
>> Given that these values are never decoded and interpreted as
>> timestamps, can't we simply switch to the second flavour immediately?
>
> I considered that, but the downside would be that all future filenames would
> come before all past file names.

Won't we have that same problem in 2038?

> I don't know if the order is important at
> all, but the current implementation at least looks like it's intended to keep
> all file names strictly sorted across boots.
>
>          Arnd

Re: [PATCH] efi: cper: avoid using get_seconds()

[Arnd Bergmann]

On Mon, Jun 18, 2018 at 5:50 PM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> On 18 June 2018 at 17:49, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Mon, Jun 18, 2018 at 5:47 PM, Ard Biesheuvel
>> <ard.biesheuvel@linaro.org> wrote:
>>> On 18 June 2018 at 16:17, Arnd Bergmann <arnd@arndb.de> wrote:
>>
>>>> -               atomic64_set(&seq, ((u64)get_seconds()) << 32);
>>>> +       if (!atomic64_read(&seq)) {
>>>> +               time64_t time = ktime_get_real_seconds();
>>>> +
>>>> +               /*
>>>> +                * This code is unlikely to still be needed in year 2106,
>>>> +                * but just in case, let's use a few more bits for timestamps
>>>> +                * after y2038 to be sure they keep increasing monotonically
>>>> +                * for the next few hundred years...
>>>> +                */
>>>> +               if (time < 0x80000000)
>>>> +                       atomic64_set(&seq, (ktime_get_real_seconds()) << 32);
>>>> +               else
>>>> +                       atomic64_set(&seq, 0x8000000000000000ull |
>>>> +                                          ktime_get_real_seconds() << 24);
>>>> +       }
>>>
>>> Given that these values are never decoded and interpreted as
>>> timestamps, can't we simply switch to the second flavour immediately?
>>
>> I considered that, but the downside would be that all future filenames would
>> come before all past file names.
>
> Won't we have that same problem in 2038?

No, it goes from 0x7fffffff00000000 to 0x8000000000000000, followed
by 0x8000000001000000.

       Arnd

Re: [PATCH] efi: cper: avoid using get_seconds()

[Ard Biesheuvel]

On 18 June 2018 at 17:54, Arnd Bergmann <arnd@arndb.de> wrote:
> On Mon, Jun 18, 2018 at 5:50 PM, Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
>> On 18 June 2018 at 17:49, Arnd Bergmann <arnd@arndb.de> wrote:
>>> On Mon, Jun 18, 2018 at 5:47 PM, Ard Biesheuvel
>>> <ard.biesheuvel@linaro.org> wrote:
>>>> On 18 June 2018 at 16:17, Arnd Bergmann <arnd@arndb.de> wrote:
>>>
>>>>> -               atomic64_set(&seq, ((u64)get_seconds()) << 32);
>>>>> +       if (!atomic64_read(&seq)) {
>>>>> +               time64_t time = ktime_get_real_seconds();
>>>>> +
>>>>> +               /*
>>>>> +                * This code is unlikely to still be needed in year 2106,
>>>>> +                * but just in case, let's use a few more bits for timestamps
>>>>> +                * after y2038 to be sure they keep increasing monotonically
>>>>> +                * for the next few hundred years...
>>>>> +                */
>>>>> +               if (time < 0x80000000)
>>>>> +                       atomic64_set(&seq, (ktime_get_real_seconds()) << 32);
>>>>> +               else
>>>>> +                       atomic64_set(&seq, 0x8000000000000000ull |
>>>>> +                                          ktime_get_real_seconds() << 24);
>>>>> +       }
>>>>
>>>> Given that these values are never decoded and interpreted as
>>>> timestamps, can't we simply switch to the second flavour immediately?
>>>
>>> I considered that, but the downside would be that all future filenames would
>>> come before all past file names.
>>
>> Won't we have that same problem in 2038?
>
> No, it goes from 0x7fffffff00000000 to 0x8000000000000000, followed
> by 0x8000000001000000.
>

Ah, right. I'm with you now :-)

I'll queue this in the efi tree.