[v2,3/7] y2038: Provide conversion helpers for struct __timespec64

Message ID 20190429104613.16209-4-lukma@denx.de
State Superseded
Headers

Commit Message

Lukasz Majewski April 29, 2019, 10:46 a.m. UTC
  Those functions allow easy conversion between Y2038 safe struct
 __timespec64 and other time related data structures.

An inline function has been added to clear padding of struct
__timespec64. This function shall be used when Y2038 safe system
passes the data to Linux kernel (as it expects upper 32 bits of
tv_nsec being zeroed).

Moreover, those functions are NOT compiled when one runs 64 bit
system (the __TIMESIZE == 64) and are used only for 32 bit
wrappers (like __clock_gettime).

* include/time.h (valid_timeval_to_timespec64): Add.
* include/time.h (valid_timespec_to_timespec64): Likewise.
* include/time.h (valid_timespec64_to_timespec): Likewise.
* include/time.h (valid_timespec64_to_timeval): Likewise.
* include/time.h (IS_VALID_NANOSECONDS): Likewise.
* include/time.h (timespec_to_timespec64): Likewise.
* include/time.h (timespec64_to_timespec): Likewise.
* include/time.h (timespec64_to_timeval): Likewise.
* include/time.h (timespec64_clear_padding): Likewise.

---
Changes for v2:
- Add timespec64_clear_padding function
---
 include/time.h | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 102 insertions(+)
  

Comments

Stepan Golosunov May 2, 2019, 11:56 a.m. UTC | #1
29.04.2019 в 12:46:09 +0200 Lukasz Majewski написал:
> +/* Set to zero the struct __timespec64's tv_pad.  */
> +static inline void
> +timespec64_clear_padding (const struct __timespec64 *tp)
> +{
> +  ((struct __timespec64*)tp)->tv_pad = 0;
> +}

Modification of const values isn't going to work.
  
Lukasz Majewski May 2, 2019, 2:17 p.m. UTC | #2
Hi Stepan,

> 29.04.2019 в 12:46:09 +0200 Lukasz Majewski написал:
> > +/* Set to zero the struct __timespec64's tv_pad.  */
> > +static inline void
> > +timespec64_clear_padding (const struct __timespec64 *tp)
> > +{
> > +  ((struct __timespec64*)tp)->tv_pad = 0;
> > +}  
> 
> Modification of const values isn't going to work.

This was added as a special precaution to avoid passing random data to
kernel syscalls for the Y2038 support use case on 32 bit systems (where
tv_nsec is 32 bit long) .

As we discussed in the other thread - kernel relies on "implementation
depended" behaviour from GCC (which is now to discard higher 32 bits
from tv_nsec [1]).

It is up to glibc community to decide if such precaution is necessary or
shall we only rely on kernel (and gcc) behaviour.


With this particular patch - the problem is with struct timespec's *tp
pointer being const for data being passed to the kernel (to avoid
creating explicit copy of struct timespec and pass it to kernel
syscall).


Note:

[1] - kernel/time/time.c -> get_timespec64: 
ts->tv_nsec = kts.tv_nsec;

(64 bit's kts.tv_nsec to 32 bit tv_nsec truncation).

Discussion:
https://patchwork.ozlabs.org/patch/1085397/
https://lkml.org/lkml/2019/4/26/600


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
  
Joseph Myers May 2, 2019, 3:59 p.m. UTC | #3
On Thu, 2 May 2019, Lukasz Majewski wrote:

> As we discussed in the other thread - kernel relies on "implementation
> depended" behaviour from GCC (which is now to discard higher 32 bits
> from tv_nsec [1]).

The kernel and glibc use a common-usage profile of C, which defines 
conversion of integer types to narrower integer types as modulo.  We do 
not need to be concerned about strange implementations making other 
choices for such conversions; they are not relevant for building the 
kernel.  (This is a separate matter from undefined behavior for signed 
arithmetic overflow.)

In the current draft of the next C++ standard revision, this is required 
for C++, and it's entirely plausible that C will follow at some point.
  
Lukasz Majewski May 2, 2019, 4:19 p.m. UTC | #4
Hi Joseph,

> On Thu, 2 May 2019, Lukasz Majewski wrote:
> 
> > As we discussed in the other thread - kernel relies on
> > "implementation depended" behaviour from GCC (which is now to
> > discard higher 32 bits from tv_nsec [1]).  
> 
> The kernel and glibc use a common-usage profile of C, which defines 
> conversion of integer types to narrower integer types as modulo.  We
> do not need to be concerned about strange implementations making
> other choices for such conversions; they are not relevant for
> building the kernel.  (This is a separate matter from undefined
> behavior for signed arithmetic overflow.)
> 
> In the current draft of the next C++ standard revision, this is
> required for C++, and it's entirely plausible that C will follow at
> some point.
> 

Thank you for a detailed explanation.

As Arnd in the other mail [1] confirmed that the intention of the
kernel is to ignore padding - zero'ing padding can be safely removed
from this glibc patch series.


Note:

[1] - https://lkml.org/lkml/2019/4/22/824

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
  

Patch

diff --git a/include/time.h b/include/time.h
index 7540ac0f8b..9827d2f045 100644
--- a/include/time.h
+++ b/include/time.h
@@ -3,6 +3,7 @@ 
 
 #ifndef _ISOMAC
 # include <bits/types/locale_t.h>
+# include <stdbool.h>
 # include <endian.h>
 
 extern __typeof (strftime_l) __strftime_l;
@@ -177,5 +178,106 @@  extern double __difftime (time_t time1, time_t time0);
    actual clock ID.  */
 #define CLOCK_IDFIELD_SIZE	3
 
+/* Check whether T fits in time_t.  */
+static inline bool
+in_time_t_range (__time64_t t)
+{
+  time_t s = t;
+  return s == t;
+}
+
+# if __TIMESIZE != 64
+/* Set to zero the struct __timespec64's tv_pad.  */
+static inline void
+timespec64_clear_padding (const struct __timespec64 *tp)
+{
+  ((struct __timespec64*)tp)->tv_pad = 0;
+}
+/* Convert a known valid struct timeval into a struct __timespec64.  */
+static inline void
+valid_timeval_to_timespec64 (const struct timeval *tv32,
+			     struct __timespec64 *ts64)
+{
+  ts64->tv_sec = tv32->tv_sec;
+  ts64->tv_nsec = tv32->tv_usec * 1000;
+}
+
+/* Convert a known valid struct timespec into a struct __timespec64.  */
+static inline void
+valid_timespec_to_timespec64 (const struct timespec *ts32,
+			      struct __timespec64 *ts64)
+{
+  ts64->tv_sec = ts32->tv_sec;
+  ts64->tv_nsec = ts32->tv_nsec;
+  /* We only need to zero ts64->tv_pad if we pass it to the kernel.  */
+}
+
+/* Convert a known valid struct __timespec64 into a struct timespec.  */
+static inline void
+valid_timespec64_to_timespec (const struct __timespec64 *ts64,
+			      struct timespec *ts32)
+{
+  ts32->tv_sec = (time_t) ts64->tv_sec;
+  ts32->tv_nsec = ts64->tv_nsec;
+}
+
+/* Convert a known valid struct __timespec64 into a struct timeval.  */
+static inline void
+valid_timespec64_to_timeval (const struct __timespec64 *ts64,
+			     struct timeval *tv32)
+{
+  tv32->tv_sec = (time_t) ts64->tv_sec;
+  tv32->tv_usec = ts64->tv_nsec / 1000;
+}
+
+/* Check if a value lies with the valid nanoseconds range.  */
+#define IS_VALID_NANOSECONDS(ns) ((ns) >= 0 && (ns) <= 999999999)
+
+/* Check and convert a struct timespec into a struct __timespec64.  */
+static inline bool
+timespec_to_timespec64 (const struct timespec *ts32,
+                        struct __timespec64 *ts64)
+{
+  /* Check that ts32 holds a valid count of nanoseconds.  */
+  if (! IS_VALID_NANOSECONDS (ts32->tv_nsec))
+    return false;
+  /* All ts32 fields can fit in ts64, so copy them.  */
+  valid_timespec_to_timespec64 (ts32, ts64);
+  /* We only need to zero ts64->tv_pad if we pass it to the kernel.  */
+  return true;
+}
+
+/* Check and convert a struct __timespec64 into a struct timespec.  */
+static inline bool
+timespec64_to_timespec (const struct __timespec64 *ts64,
+                        struct timespec *ts32)
+{
+  /* Check that tv_nsec holds a valid count of nanoseconds.  */
+  if (! IS_VALID_NANOSECONDS (ts64->tv_nsec))
+    return false;
+  /* Check that tv_sec can fit in a __time_t.  */
+  if (! in_time_t_range (ts64->tv_sec))
+    return false;
+  /* All ts64 fields can fit in ts32, so copy them.  */
+  valid_timespec64_to_timespec (ts64, ts32);
+  return true;
+}
+
+/* Check and convert a struct __timespec64 into a struct timeval.  */
+static inline bool
+timespec64_to_timeval (const struct __timespec64 *ts64,
+                       struct timeval *tv32)
+{
+  /* Check that tv_nsec holds a valid count of nanoseconds.  */
+  if (! IS_VALID_NANOSECONDS (ts64->tv_nsec))
+    return false;
+  /* Check that tv_sec can fit in a __time_t.  */
+  if (! in_time_t_range (ts64->tv_sec))
+    return false;
+  /* All ts64 fields can fit in tv32, so copy them.  */
+  valid_timespec64_to_timeval (ts64, tv32);
+  return true;
+}
+# endif
 #endif
 #endif