[PATCH] Linux: Fix UTC offset setting in settimeofday for __TIMESIZE != 64

[PATCH] Linux: Fix UTC offset setting in settimeofday for __TIMESIZE != 64

[Florian Weimer]

The time argument is NULL in this case, and attempt to convert it
leads to a null pointer dereference.

This fixes commit d2e3b697da2433c08702f95c76458c51545c3df1
("y2038: linux: Provide __settimeofday64 implementation").

---
Tested on i686-linux-gnu, but only with the glibc test suite.

 sysdeps/unix/sysv/linux/settimeofday.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/settimeofday.c b/sysdeps/unix/sysv/linux/settimeofday.c
index ea45f1d7cb..40529cdeec 100644
--- a/sysdeps/unix/sysv/linux/settimeofday.c
+++ b/sysdeps/unix/sysv/linux/settimeofday.c
@@ -25,14 +25,14 @@
 int
 __settimeofday64 (const struct __timeval64 *tv, const struct timezone *tz)
 {
+  /* The 64-bit interface does support setting the UTC offset.  */
   if (__glibc_unlikely (tz != 0))
     {
       if (tv != 0)
-	{
-	  __set_errno (EINVAL);
-	  return -1;
-	}
-      return __settimezone (tz);
+	__set_errno (EINVAL);
+      else
+	__set_errno (ENOSYS);
+      return -1;
     }
 
   struct __timespec64 ts = timeval64_to_timespec64 (*tv);
@@ -45,8 +45,18 @@ libc_hidden_def (__settimeofday64)
 int
 __settimeofday (const struct timeval *tv, const struct timezone *tz)
 {
+  /* Backwards compatibility for setting the UTC offset.  */
+  if (__glibc_unlikely (tz != 0))
+    {
+      if (tv != 0)
+	{
+	  __set_errno (EINVAL);
+	  return -1;
+	}
+      return __settimezone (tz);
+    }
+
   struct __timeval64 tv64 = valid_timeval_to_timeval64 (*tv);
-
   return __settimeofday64 (&tv64, tz);
 }
 #endif

Re: [PATCH] Linux: Fix UTC offset setting in settimeofday for __TIMESIZE != 64

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 2600 bytes --]

Hi Florian,

Thank you for the patch.

> The time argument is NULL in this case, and attempt to convert it
> leads to a null pointer dereference.
> 
> This fixes commit d2e3b697da2433c08702f95c76458c51545c3df1
> ("y2038: linux: Provide __settimeofday64 implementation").
> 
> ---
> Tested on i686-linux-gnu, but only with the glibc test suite.
> 
>  sysdeps/unix/sysv/linux/settimeofday.c | 22 ++++++++++++++++------
>  1 file changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/sysdeps/unix/sysv/linux/settimeofday.c
> b/sysdeps/unix/sysv/linux/settimeofday.c index ea45f1d7cb..40529cdeec
> 100644 --- a/sysdeps/unix/sysv/linux/settimeofday.c
> +++ b/sysdeps/unix/sysv/linux/settimeofday.c
> @@ -25,14 +25,14 @@
>  int
>  __settimeofday64 (const struct __timeval64 *tv, const struct
> timezone *tz) {
> +  /* The 64-bit interface does support setting the UTC offset.  */
>    if (__glibc_unlikely (tz != 0))
>      {
>        if (tv != 0)
> -	{
> -	  __set_errno (EINVAL);
> -	  return -1;
> -	}
> -      return __settimezone (tz);

This code was took directly from the old settimeofday implementation.
Please refer to SHA1: d2e3b697da2433c08702f95c76458c51545c3df1 [1]

The reason that this check is in the __settimeofday64() is that for
32 bit systems, after enabling _TIME_BITS==64, we would use
__settimeofday64() as a redirection to settimeofday().

The __settimeofday64 is also used by 64 bit archs (as an alias to
settimeofday).

With your changes applied (below), the relevant check would be only
done for 32 bit systems, and may break legacy setups (as indicated in
[1]).

> +	__set_errno (EINVAL);
> +      else
> +	__set_errno (ENOSYS);
> +      return -1;
>      }
>  
>    struct __timespec64 ts = timeval64_to_timespec64 (*tv);
> @@ -45,8 +45,18 @@ libc_hidden_def (__settimeofday64)
>  int
>  __settimeofday (const struct timeval *tv, const struct timezone *tz)
>  {
> +  /* Backwards compatibility for setting the UTC offset.  */
> +  if (__glibc_unlikely (tz != 0))
> +    {
> +      if (tv != 0)
> +	{
> +	  __set_errno (EINVAL);
> +	  return -1;
> +	}
> +      return __settimezone (tz);
> +    }
> +
>    struct __timeval64 tv64 = valid_timeval_to_timeval64 (*tv);
> -
>    return __settimeofday64 (&tv64, tz);
>  }
>  #endif
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] Linux: Fix UTC offset setting in settimeofday for __TIMESIZE != 64

[Florian Weimer]

* Lukasz Majewski:

> Hi Florian,
>
> Thank you for the patch.
>
>> The time argument is NULL in this case, and attempt to convert it
>> leads to a null pointer dereference.
>> 
>> This fixes commit d2e3b697da2433c08702f95c76458c51545c3df1
>> ("y2038: linux: Provide __settimeofday64 implementation").
>> 
>> ---
>> Tested on i686-linux-gnu, but only with the glibc test suite.
>> 
>>  sysdeps/unix/sysv/linux/settimeofday.c | 22 ++++++++++++++++------
>>  1 file changed, 16 insertions(+), 6 deletions(-)
>> 
>> diff --git a/sysdeps/unix/sysv/linux/settimeofday.c
>> b/sysdeps/unix/sysv/linux/settimeofday.c index ea45f1d7cb..40529cdeec
>> 100644 --- a/sysdeps/unix/sysv/linux/settimeofday.c
>> +++ b/sysdeps/unix/sysv/linux/settimeofday.c
>> @@ -25,14 +25,14 @@
>>  int
>>  __settimeofday64 (const struct __timeval64 *tv, const struct
>> timezone *tz) {
>> +  /* The 64-bit interface does support setting the UTC offset.  */
>>    if (__glibc_unlikely (tz != 0))
>>      {
>>        if (tv != 0)
>> -	{
>> -	  __set_errno (EINVAL);
>> -	  return -1;
>> -	}
>> -      return __settimezone (tz);
>
> This code was took directly from the old settimeofday implementation.
> Please refer to SHA1: d2e3b697da2433c08702f95c76458c51545c3df1 [1]
>
> The reason that this check is in the __settimeofday64() is that for
> 32 bit systems, after enabling _TIME_BITS==64, we would use
> __settimeofday64() as a redirection to settimeofday().
>
> The __settimeofday64 is also used by 64 bit archs (as an alias to
> settimeofday).
>
> With your changes applied (below), the relevant check would be only
> done for 32 bit systems, and may break legacy setups (as indicated in
> [1]).

Right, I'm going to send an updated patch.

Thanks,
Florian