time: create __getnstimeofday for WARNless calls

The pstore RAM backend can get called during resume, and must be defensive against a suspended time source. Expose getnstimeofday logic that returns an error instead of a WARN. This can be detected and the timestamp can be zeroed out.

Reported-by Doug Anderson <[email protected]>

Cc John Stultz <[email protected]>

Cc Anton Vorontsov <[email protected]>

Signed-off-by Kees Cook <[email protected]>

---
 fs/pstore/ram.c           |   10 +++++++---
 include/linux/time.h      |    1 +
 kernel/time/timekeeping.c |   29 ++++++++++++++++++++++++-----
 3 files changed, 32 insertions(+), 8 deletions(-)

fs/pstore/ram.c +7 -3

index 1a4f6da..dacfe78 100644
@@ -168,12 +168,16 @@ static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type,
 static size_t ramoops_write_kmsg_hdr(struct persistent_ram_zone *prz)
 {
        char *hdr;
-       struct timeval timestamp;
+       struct timespec timestamp;
        size_t len;
 
-       do_gettimeofday(&timestamp);
+       /* Report zeroed timestamp if called before timekeeping has resumed. */
+       if (__getnstimeofday(&timestamp)) {
+               timestamp.tv_sec = 0;
+               timestamp.tv_nsec = 0;
+       }
        hdr = kasprintf(GFP_ATOMIC, RAMOOPS_KERNMSG_HDR "%lu.%lu\n",
-               (long)timestamp.tv_sec, (long)timestamp.tv_usec);
+               (long)timestamp.tv_sec, (long)(timestamp.tv_nsec / 1000));
        WARN_ON_ONCE(!hdr);
        len = hdr ? strlen(hdr) : 0;
        persistent_ram_write(prz, hdr, len);

include/linux/time.h +1 -0

index 4d358e9..0015aea 100644
@@ -158,6 +158,7 @@ extern int do_setitimer(int which, struct itimerval *value,
                        struct itimerval *ovalue);
 extern unsigned int alarm_setitimer(unsigned int seconds);
 extern int do_getitimer(int which, struct itimerval *value);
+extern int __getnstimeofday(struct timespec *tv);
 extern void getnstimeofday(struct timespec *tv);
 extern void getrawmonotonic(struct timespec *ts);
 extern void getnstime_raw_and_real(struct timespec *ts_raw,

kernel/time/timekeeping.c +24 -6

index e424970..84820dc 100644
@@ -220,19 +220,18 @@ static void timekeeping_forward_now(struct timekeeper *tk)
 }
 
 /**
- * getnstimeofday - Returns the time of day in a timespec
+ * __getnstimeofday - Returns the time of day in a timespec.
  * @ts:                pointer to the timespec to be set
  *
- * Returns the time of day in a timespec.
+ * Updates the time of day in the timespec.
+ * Returns 0 on success, or -ve when suspended (timespec will be undefined).
  */
-void getnstimeofday(struct timespec *ts)
+int __getnstimeofday(struct timespec *ts)
 {
        struct timekeeper *tk = &timekeeper;
        unsigned long seq;
        s64 nsecs = 0;
 
-       WARN_ON(timekeeping_suspended);
-
        do {
                seq = read_seqbegin(&tk->lock);
 

@@ -243,6 +242,26 @@ void getnstimeofday(struct timespec *ts)
 
        ts->tv_nsec = 0;
        timespec_add_ns(ts, nsecs);
+
+       /*
+        * Do not bail out early, in case there were callers still using
+        * the value, even in the face of the WARN_ON.
+        */
+       if (unlikely(timekeeping_suspended))
+               return -EAGAIN;
+       return 0;
+}
+EXPORT_SYMBOL(__getnstimeofday);
+
+/**
+ * getnstimeofday - Returns the time of day in a timespec.
+ * @ts:                pointer to the timespec to be set
+ *
+ * Returns the time of day in a timespec (WARN if suspended).
+ */
+void getnstimeofday(struct timespec *ts)
+{
+       WARN_ON(__getnstimeofday(ts));
 }
 EXPORT_SYMBOL(getnstimeofday);
 
--

1.7.9.5

--
Kees Cook
Chrome OS Security

John, any feedback on this?

Quoted 2 lines · Kees Cook

John, any feedback on this?

Looking at my inbox, I actually can't find a copy of this specific patch. Do you mind bouncing it to me, so I have something I can apply?

Quoted 3 lines · Kees Cook

John, any feedback on this?

Sorry, yea, I've been meaning to get back to this.

I'm still on the fence about just making getnstimeofday() safe for when timekeeping is suspended, but at the same time, your issue needs fixing. Also bailing out at the end still seems off to me. Even if someone is using the values despite the WARN_ON, they really are getting junk values, and for all the time that WARN_ON has been there, you're the first to report running into it.

Even so, I think I'm ok with this patch for now, but I suspect we may want to rework it later.

Looking at my inbox, I actually can't find a copy of this specific patch. Do you mind bouncing it to me, so I have something I can apply?

Should this also get marked for -stable?