linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] time: create __getnstimeofday for WARNless calls
@ 2012-11-19 18:26 Kees Cook
  2012-12-13 18:17 ` Kees Cook
  0 siblings, 1 reply; 9+ messages in thread
From: Kees Cook @ 2012-11-19 18:26 UTC (permalink / raw)
  To: linux-kernel
  Cc: Anton Vorontsov, Colin Cross, Kees Cook, Tony Luck, John Stultz,
	Thomas Gleixner, Anton Vorontsov

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 <dianders@chromium.org>
Cc: John Stultz <johnstul@us.ibm.com>
Cc: Anton Vorontsov <anton.vorontsov@linaro.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 fs/pstore/ram.c           |   10 +++++++---
 include/linux/time.h      |    1 +
 kernel/time/timekeeping.c |   29 ++++++++++++++++++++++++-----
 3 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 1a4f6da..dacfe78 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -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);
diff --git a/include/linux/time.h b/include/linux/time.h
index 4d358e9..0015aea 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -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,
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index e424970..84820dc 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] time: create __getnstimeofday for WARNless calls
  2012-11-19 18:26 [PATCH] time: create __getnstimeofday for WARNless calls Kees Cook
@ 2012-12-13 18:17 ` Kees Cook
  2012-12-15  1:16   ` John Stultz
  0 siblings, 1 reply; 9+ messages in thread
From: Kees Cook @ 2012-12-13 18:17 UTC (permalink / raw)
  To: linux-kernel
  Cc: Anton Vorontsov, Colin Cross, Kees Cook, Tony Luck, John Stultz,
	Thomas Gleixner, Anton Vorontsov

John, any feedback on this?

Thanks,

-Kees

On Mon, Nov 19, 2012 at 10:26 AM, Kees Cook <keescook@chromium.org> wrote:
> 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 <dianders@chromium.org>
> Cc: John Stultz <johnstul@us.ibm.com>
> Cc: Anton Vorontsov <anton.vorontsov@linaro.org>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  fs/pstore/ram.c           |   10 +++++++---
>  include/linux/time.h      |    1 +
>  kernel/time/timekeeping.c |   29 ++++++++++++++++++++++++-----
>  3 files changed, 32 insertions(+), 8 deletions(-)
>
> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> index 1a4f6da..dacfe78 100644
> --- a/fs/pstore/ram.c
> +++ b/fs/pstore/ram.c
> @@ -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);
> diff --git a/include/linux/time.h b/include/linux/time.h
> index 4d358e9..0015aea 100644
> --- a/include/linux/time.h
> +++ b/include/linux/time.h
> @@ -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,
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index e424970..84820dc 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -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



-- 
Kees Cook
Chrome OS Security

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] time: create __getnstimeofday for WARNless calls
  2012-12-13 18:17 ` Kees Cook
@ 2012-12-15  1:16   ` John Stultz
  2012-12-15  1:17     ` John Stultz
  2012-12-15  7:22     ` Kees Cook
  0 siblings, 2 replies; 9+ messages in thread
From: John Stultz @ 2012-12-15  1:16 UTC (permalink / raw)
  To: Kees Cook
  Cc: linux-kernel, Anton Vorontsov, Colin Cross, Tony Luck,
	Thomas Gleixner, Anton Vorontsov

On 12/13/2012 10:17 AM, Kees Cook wrote:
> 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?

thanks
-john


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] time: create __getnstimeofday for WARNless calls
  2012-12-15  1:16   ` John Stultz
@ 2012-12-15  1:17     ` John Stultz
  2012-12-15  7:22     ` Kees Cook
  1 sibling, 0 replies; 9+ messages in thread
From: John Stultz @ 2012-12-15  1:17 UTC (permalink / raw)
  To: Kees Cook
  Cc: linux-kernel, Anton Vorontsov, Colin Cross, Tony Luck,
	Thomas Gleixner, Anton Vorontsov

On 12/14/2012 05:16 PM, John Stultz wrote:
> On 12/13/2012 10:17 AM, Kees Cook wrote:
>> 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?
Nm, I found it.

thanks
-john


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] time: create __getnstimeofday for WARNless calls
  2012-12-15  1:16   ` John Stultz
  2012-12-15  1:17     ` John Stultz
@ 2012-12-15  7:22     ` Kees Cook
  1 sibling, 0 replies; 9+ messages in thread
From: Kees Cook @ 2012-12-15  7:22 UTC (permalink / raw)
  To: John Stultz
  Cc: linux-kernel, Anton Vorontsov, Colin Cross, Tony Luck,
	Thomas Gleixner, Anton Vorontsov

On Fri, Dec 14, 2012 at 5:16 PM, John Stultz <johnstul@us.ibm.com> wrote:
> On 12/13/2012 10:17 AM, Kees Cook wrote:
>>
>> 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?

Nah, I don't think it's worth it.

Thanks!

-Kees

-- 
Kees Cook
Chrome OS Security

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] time: create __getnstimeofday for WARNless calls
  2012-12-20 11:43 ` Thomas Gleixner
@ 2012-12-20 16:02   ` Kees Cook
  0 siblings, 0 replies; 9+ messages in thread
From: Kees Cook @ 2012-12-20 16:02 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: John Stultz, LKML, Anton Vorontsov

On Thu, Dec 20, 2012 at 3:43 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Mon, 17 Dec 2012, John Stultz wrote:
>> From: Kees Cook <keescook@chromium.org>
>>
>> Hey Thomas,
>>    Wanted to see if maybe there was still time for this for 3.8?
>> thanks
>> -john
>>
>> 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.
>
> Shouldn't we zero out the time stamp in the core code ?

It wasn't clear to me if the raw/wrong value should be available to a
caller when they got the EAGAIN. Since pstore is the only known user
of this, I'm fine moving the zeroing into the timekeeping core.

-Kees

>
> Thanks,
>
>         tglx
>
>> Reported-by: Doug Anderson <dianders@chromium.org>
>> Cc: Anton Vorontsov <anton.vorontsov@linaro.org>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> Signed-off-by: John Stultz <john.stultz@linaro.org>
>> ---
>>  fs/pstore/ram.c           |   10 +++++++---
>>  include/linux/time.h      |    1 +
>>  kernel/time/timekeeping.c |   29 ++++++++++++++++++++++++-----
>>  3 files changed, 32 insertions(+), 8 deletions(-)
>>
>> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
>> index 1a4f6da..dacfe78 100644
>> --- a/fs/pstore/ram.c
>> +++ b/fs/pstore/ram.c
>> @@ -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);
>> diff --git a/include/linux/time.h b/include/linux/time.h
>> index 4d358e9..0015aea 100644
>> --- a/include/linux/time.h
>> +++ b/include/linux/time.h
>> @@ -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,
>> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
>> index 4c7de02..dfc7f87 100644
>> --- a/kernel/time/timekeeping.c
>> +++ b/kernel/time/timekeeping.c
>> @@ -214,19 +214,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);
>>
>> @@ -237,6 +236,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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] time: create __getnstimeofday for WARNless calls
  2012-12-17 18:51 John Stultz
  2012-12-17 19:28 ` Anton Vorontsov
@ 2012-12-20 11:43 ` Thomas Gleixner
  2012-12-20 16:02   ` Kees Cook
  1 sibling, 1 reply; 9+ messages in thread
From: Thomas Gleixner @ 2012-12-20 11:43 UTC (permalink / raw)
  To: John Stultz; +Cc: linux-kernel, Kees Cook, Anton Vorontsov

On Mon, 17 Dec 2012, John Stultz wrote:
> From: Kees Cook <keescook@chromium.org>
> 
> Hey Thomas, 
>    Wanted to see if maybe there was still time for this for 3.8?
> thanks
> -john
> 
> 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.

Shouldn't we zero out the time stamp in the core code ?

Thanks,

	tglx

> Reported-by: Doug Anderson <dianders@chromium.org>
> Cc: Anton Vorontsov <anton.vorontsov@linaro.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
>  fs/pstore/ram.c           |   10 +++++++---
>  include/linux/time.h      |    1 +
>  kernel/time/timekeeping.c |   29 ++++++++++++++++++++++++-----
>  3 files changed, 32 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> index 1a4f6da..dacfe78 100644
> --- a/fs/pstore/ram.c
> +++ b/fs/pstore/ram.c
> @@ -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);
> diff --git a/include/linux/time.h b/include/linux/time.h
> index 4d358e9..0015aea 100644
> --- a/include/linux/time.h
> +++ b/include/linux/time.h
> @@ -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,
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index 4c7de02..dfc7f87 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -214,19 +214,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);
>  
> @@ -237,6 +236,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
> 
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] time: create __getnstimeofday for WARNless calls
  2012-12-17 18:51 John Stultz
@ 2012-12-17 19:28 ` Anton Vorontsov
  2012-12-20 11:43 ` Thomas Gleixner
  1 sibling, 0 replies; 9+ messages in thread
From: Anton Vorontsov @ 2012-12-17 19:28 UTC (permalink / raw)
  To: John Stultz; +Cc: linux-kernel, Kees Cook, Thomas Gleixner

On Mon, Dec 17, 2012 at 01:51:54PM -0500, John Stultz wrote:
> From: Kees Cook <keescook@chromium.org>
> 
> Hey Thomas, 
>    Wanted to see if maybe there was still time for this for 3.8?
> thanks
> -john
> 
> 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 <dianders@chromium.org>
> Cc: Anton Vorontsov <anton.vorontsov@linaro.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
>  fs/pstore/ram.c           |   10 +++++++---

For pstore/ram part:

Acked-by: Anton Vorontsov <anton.vorontsov@linaro.org>

Thanks!

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] time: create __getnstimeofday for WARNless calls
@ 2012-12-17 18:51 John Stultz
  2012-12-17 19:28 ` Anton Vorontsov
  2012-12-20 11:43 ` Thomas Gleixner
  0 siblings, 2 replies; 9+ messages in thread
From: John Stultz @ 2012-12-17 18:51 UTC (permalink / raw)
  To: linux-kernel; +Cc: Kees Cook, Anton Vorontsov, Thomas Gleixner, John Stultz

From: Kees Cook <keescook@chromium.org>

Hey Thomas, 
   Wanted to see if maybe there was still time for this for 3.8?
thanks
-john

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 <dianders@chromium.org>
Cc: Anton Vorontsov <anton.vorontsov@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 fs/pstore/ram.c           |   10 +++++++---
 include/linux/time.h      |    1 +
 kernel/time/timekeeping.c |   29 ++++++++++++++++++++++++-----
 3 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 1a4f6da..dacfe78 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -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);
diff --git a/include/linux/time.h b/include/linux/time.h
index 4d358e9..0015aea 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -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,
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 4c7de02..dfc7f87 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -214,19 +214,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);
 
@@ -237,6 +236,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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2012-12-20 16:02 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-19 18:26 [PATCH] time: create __getnstimeofday for WARNless calls Kees Cook
2012-12-13 18:17 ` Kees Cook
2012-12-15  1:16   ` John Stultz
2012-12-15  1:17     ` John Stultz
2012-12-15  7:22     ` Kees Cook
2012-12-17 18:51 John Stultz
2012-12-17 19:28 ` Anton Vorontsov
2012-12-20 11:43 ` Thomas Gleixner
2012-12-20 16:02   ` Kees Cook

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).