[PATCH 0/2] fix vdso coarse clock monotonicity regressions

[PATCH 0/2] fix vdso coarse clock monotonicity regressions

[Nathan Lynch]

Commit 906c55579a63 ("timekeeping: Copy the shadow-timekeeper over the
real timekeeper last") made it so the user can observe the coarse
clocks going backwards on arm and arm64, if they're really looking for
it.

Technically these are fixing regressions versus 4.1, but I won't be
bothered if they don't make 4.2 final at this late stage, since only
the (seldom-used?) coarse clocks are affected.

I'd like to collect review/acks for these now and make sure they at
least make it into 4.3-rc1 (and -stable after that).


Nathan Lynch (2):
  ARM: VDSO: fix coarse clock monotonicity regression
  arm64: VDSO: fix coarse clock monotonicity regression

 arch/arm/kernel/vdso.c   | 7 +++----
 arch/arm64/kernel/vdso.c | 7 +++----
 2 files changed, 6 insertions(+), 8 deletions(-)

-- 
2.1.0

[PATCH 1/2] ARM: VDSO: fix coarse clock monotonicity regression

[Nathan Lynch]

Since 906c55579a63 ("timekeeping: Copy the shadow-timekeeper over the
real timekeeper last") it has become possible on ARM to:

- Obtain a CLOCK_MONOTONIC_COARSE or CLOCK_REALTIME_COARSE timestamp
  via syscall.
- Subsequently obtain a timestamp for the same clock ID via VDSO which
  predates the first timestamp (by one jiffy).

This is because ARM's update_vsyscall is deriving the coarse time
using the __current_kernel_time interface, when it should really be
using the timekeeper object provided to it by the timekeeping core.
It happened to work before only because __current_kernel_time would
access the same timekeeper object which had been passed to
update_vsyscall.  This is no longer the case.

Signed-off-by: Nathan Lynch <nathan_lynch@mentor.com>
---
 arch/arm/kernel/vdso.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/arm/kernel/vdso.c b/arch/arm/kernel/vdso.c
index efe17dd9b921..c8b243c1aef8 100644
--- a/arch/arm/kernel/vdso.c
+++ b/arch/arm/kernel/vdso.c
@@ -296,7 +296,6 @@ static bool tk_is_cntvct(const struct timekeeper *tk)
  */
 void update_vsyscall(struct timekeeper *tk)
 {
-	struct timespec xtime_coarse;
 	struct timespec64 *wtm = &tk->wall_to_monotonic;
 
 	if (!cntvct_ok) {
@@ -308,10 +307,10 @@ void update_vsyscall(struct timekeeper *tk)
 
 	vdso_write_begin(vdso_data);
 
-	xtime_coarse = __current_kernel_time();
 	vdso_data->tk_is_cntvct			= tk_is_cntvct(tk);
-	vdso_data->xtime_coarse_sec		= xtime_coarse.tv_sec;
-	vdso_data->xtime_coarse_nsec		= xtime_coarse.tv_nsec;
+	vdso_data->xtime_coarse_sec		= tk->xtime_sec;
+	vdso_data->xtime_coarse_nsec		= tk->tkr_mono.xtime_nsec >>
+							tk->tkr_mono.shift;
 	vdso_data->wtm_clock_sec		= wtm->tv_sec;
 	vdso_data->wtm_clock_nsec		= wtm->tv_nsec;
 
-- 
2.1.0

[PATCH 1/2] ARM: VDSO: fix coarse clock monotonicity regression

[Will Deacon]

On Sat, Aug 08, 2015 at 03:03:22AM +0100, Nathan Lynch wrote:
> Since 906c55579a63 ("timekeeping: Copy the shadow-timekeeper over the
> real timekeeper last") it has become possible on ARM to:
> 
> - Obtain a CLOCK_MONOTONIC_COARSE or CLOCK_REALTIME_COARSE timestamp
>   via syscall.
> - Subsequently obtain a timestamp for the same clock ID via VDSO which
>   predates the first timestamp (by one jiffy).
> 
> This is because ARM's update_vsyscall is deriving the coarse time
> using the __current_kernel_time interface, when it should really be
> using the timekeeper object provided to it by the timekeeping core.
> It happened to work before only because __current_kernel_time would
> access the same timekeeper object which had been passed to
> update_vsyscall.  This is no longer the case.
> 
> Signed-off-by: Nathan Lynch <nathan_lynch@mentor.com>
> ---
>  arch/arm/kernel/vdso.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/kernel/vdso.c b/arch/arm/kernel/vdso.c
> index efe17dd9b921..c8b243c1aef8 100644
> --- a/arch/arm/kernel/vdso.c
> +++ b/arch/arm/kernel/vdso.c
> @@ -296,7 +296,6 @@ static bool tk_is_cntvct(const struct timekeeper *tk)
>   */
>  void update_vsyscall(struct timekeeper *tk)
>  {
> -	struct timespec xtime_coarse;
>  	struct timespec64 *wtm = &tk->wall_to_monotonic;
>  
>  	if (!cntvct_ok) {
> @@ -308,10 +307,10 @@ void update_vsyscall(struct timekeeper *tk)
>  
>  	vdso_write_begin(vdso_data);
>  
> -	xtime_coarse = __current_kernel_time();
>  	vdso_data->tk_is_cntvct			= tk_is_cntvct(tk);
> -	vdso_data->xtime_coarse_sec		= xtime_coarse.tv_sec;
> -	vdso_data->xtime_coarse_nsec		= xtime_coarse.tv_nsec;
> +	vdso_data->xtime_coarse_sec		= tk->xtime_sec;
> +	vdso_data->xtime_coarse_nsec		= tk->tkr_mono.xtime_nsec >>
> +							tk->tkr_mono.shift;

Do you need a cast to u32 here?

Will

[PATCH 2/2] arm64: VDSO: fix coarse clock monotonicity regression

[Nathan Lynch]

Since 906c55579a63 ("timekeeping: Copy the shadow-timekeeper over the
real timekeeper last") it has become possible on arm64 to:

- Obtain a CLOCK_MONOTONIC_COARSE or CLOCK_REALTIME_COARSE timestamp
  via syscall.
- Subsequently obtain a timestamp for the same clock ID via VDSO which
  predates the first timestamp (by one jiffy).

This is because arm64's update_vsyscall is deriving the coarse time
using the __current_kernel_time interface, when it should really be
using the timekeeper object provided to it by the timekeeping core.
It happened to work before only because __current_kernel_time would
access the same timekeeper object which had been passed to
update_vsyscall.  This is no longer the case.

Signed-off-by: Nathan Lynch <nathan_lynch@mentor.com>
---
 arch/arm64/kernel/vdso.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
index ec37ab3f524f..97bc68f4c689 100644
--- a/arch/arm64/kernel/vdso.c
+++ b/arch/arm64/kernel/vdso.c
@@ -199,16 +199,15 @@ int arch_setup_additional_pages(struct linux_binprm *bprm,
  */
 void update_vsyscall(struct timekeeper *tk)
 {
-	struct timespec xtime_coarse;
 	u32 use_syscall = strcmp(tk->tkr_mono.clock->name, "arch_sys_counter");
 
 	++vdso_data->tb_seq_count;
 	smp_wmb();
 
-	xtime_coarse = __current_kernel_time();
 	vdso_data->use_syscall			= use_syscall;
-	vdso_data->xtime_coarse_sec		= xtime_coarse.tv_sec;
-	vdso_data->xtime_coarse_nsec		= xtime_coarse.tv_nsec;
+	vdso_data->xtime_coarse_sec		= tk->xtime_sec;
+	vdso_data->xtime_coarse_nsec		= tk->tkr_mono.xtime_nsec >>
+							tk->tkr_mono.shift;
 	vdso_data->wtm_clock_sec		= tk->wall_to_monotonic.tv_sec;
 	vdso_data->wtm_clock_nsec		= tk->wall_to_monotonic.tv_nsec;
 
-- 
2.1.0

[PATCH 2/2] arm64: VDSO: fix coarse clock monotonicity regression

[Will Deacon]

Hi Nathan,

On Sat, Aug 08, 2015 at 03:03:23AM +0100, Nathan Lynch wrote:
> Since 906c55579a63 ("timekeeping: Copy the shadow-timekeeper over the
> real timekeeper last") it has become possible on arm64 to:
> 
> - Obtain a CLOCK_MONOTONIC_COARSE or CLOCK_REALTIME_COARSE timestamp
>   via syscall.
> - Subsequently obtain a timestamp for the same clock ID via VDSO which
>   predates the first timestamp (by one jiffy).
> 
> This is because arm64's update_vsyscall is deriving the coarse time
> using the __current_kernel_time interface, when it should really be
> using the timekeeper object provided to it by the timekeeping core.
> It happened to work before only because __current_kernel_time would
> access the same timekeeper object which had been passed to
> update_vsyscall.  This is no longer the case.
> 
> Signed-off-by: Nathan Lynch <nathan_lynch@mentor.com>
> ---
>  arch/arm64/kernel/vdso.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
> index ec37ab3f524f..97bc68f4c689 100644
> --- a/arch/arm64/kernel/vdso.c
> +++ b/arch/arm64/kernel/vdso.c
> @@ -199,16 +199,15 @@ int arch_setup_additional_pages(struct linux_binprm *bprm,
>   */
>  void update_vsyscall(struct timekeeper *tk)
>  {
> -	struct timespec xtime_coarse;
>  	u32 use_syscall = strcmp(tk->tkr_mono.clock->name, "arch_sys_counter");
>  
>  	++vdso_data->tb_seq_count;
>  	smp_wmb();
>  
> -	xtime_coarse = __current_kernel_time();
>  	vdso_data->use_syscall			= use_syscall;
> -	vdso_data->xtime_coarse_sec		= xtime_coarse.tv_sec;
> -	vdso_data->xtime_coarse_nsec		= xtime_coarse.tv_nsec;
> +	vdso_data->xtime_coarse_sec		= tk->xtime_sec;
> +	vdso_data->xtime_coarse_nsec		= tk->tkr_mono.xtime_nsec >>
> +							tk->tkr_mono.shift;
>  	vdso_data->wtm_clock_sec		= tk->wall_to_monotonic.tv_sec;
>  	vdso_data->wtm_clock_nsec		= tk->wall_to_monotonic.tv_nsec;

Looks good,

  Acked-by: Will Deacon <will.deacon@arm.com>

There's probably still time for Catalin to pick this up for 4.2.

Will

[PATCH 2/2] arm64: VDSO: fix coarse clock monotonicity regression

[Catalin Marinas]

On Mon, Aug 10, 2015 at 10:22:53AM +0100, Will Deacon wrote:
> On Sat, Aug 08, 2015 at 03:03:23AM +0100, Nathan Lynch wrote:
> > Since 906c55579a63 ("timekeeping: Copy the shadow-timekeeper over the
> > real timekeeper last") it has become possible on arm64 to:
> > 
> > - Obtain a CLOCK_MONOTONIC_COARSE or CLOCK_REALTIME_COARSE timestamp
> >   via syscall.
> > - Subsequently obtain a timestamp for the same clock ID via VDSO which
> >   predates the first timestamp (by one jiffy).
> > 
> > This is because arm64's update_vsyscall is deriving the coarse time
> > using the __current_kernel_time interface, when it should really be
> > using the timekeeper object provided to it by the timekeeping core.
> > It happened to work before only because __current_kernel_time would
> > access the same timekeeper object which had been passed to
> > update_vsyscall.  This is no longer the case.
> > 
> > Signed-off-by: Nathan Lynch <nathan_lynch@mentor.com>
> > ---
> >  arch/arm64/kernel/vdso.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
> > index ec37ab3f524f..97bc68f4c689 100644
> > --- a/arch/arm64/kernel/vdso.c
> > +++ b/arch/arm64/kernel/vdso.c
> > @@ -199,16 +199,15 @@ int arch_setup_additional_pages(struct linux_binprm *bprm,
> >   */
> >  void update_vsyscall(struct timekeeper *tk)
> >  {
> > -	struct timespec xtime_coarse;
> >  	u32 use_syscall = strcmp(tk->tkr_mono.clock->name, "arch_sys_counter");
> >  
> >  	++vdso_data->tb_seq_count;
> >  	smp_wmb();
> >  
> > -	xtime_coarse = __current_kernel_time();
> >  	vdso_data->use_syscall			= use_syscall;
> > -	vdso_data->xtime_coarse_sec		= xtime_coarse.tv_sec;
> > -	vdso_data->xtime_coarse_nsec		= xtime_coarse.tv_nsec;
> > +	vdso_data->xtime_coarse_sec		= tk->xtime_sec;
> > +	vdso_data->xtime_coarse_nsec		= tk->tkr_mono.xtime_nsec >>
> > +							tk->tkr_mono.shift;
> >  	vdso_data->wtm_clock_sec		= tk->wall_to_monotonic.tv_sec;
> >  	vdso_data->wtm_clock_nsec		= tk->wall_to_monotonic.tv_nsec;
> 
> Looks good,
> 
>   Acked-by: Will Deacon <will.deacon@arm.com>
> 
> There's probably still time for Catalin to pick this up for 4.2.

Applied, I'll send a pull request today/tomorrow. Thanks.

-- 
Catalin

[PATCH 2/2] arm64: VDSO: fix coarse clock monotonicity regression

[John Stultz]

On Fri, Aug 7, 2015 at 7:03 PM, Nathan Lynch <nathan_lynch@mentor.com> wrote:
> Since 906c55579a63 ("timekeeping: Copy the shadow-timekeeper over the
> real timekeeper last") it has become possible on arm64 to:
>
> - Obtain a CLOCK_MONOTONIC_COARSE or CLOCK_REALTIME_COARSE timestamp
>   via syscall.
> - Subsequently obtain a timestamp for the same clock ID via VDSO which
>   predates the first timestamp (by one jiffy).
>
> This is because arm64's update_vsyscall is deriving the coarse time
> using the __current_kernel_time interface, when it should really be
> using the timekeeper object provided to it by the timekeeping core.
> It happened to work before only because __current_kernel_time would
> access the same timekeeper object which had been passed to
> update_vsyscall.  This is no longer the case.
>
> Signed-off-by: Nathan Lynch <nathan_lynch@mentor.com>
> ---
>  arch/arm64/kernel/vdso.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
> index ec37ab3f524f..97bc68f4c689 100644
> --- a/arch/arm64/kernel/vdso.c
> +++ b/arch/arm64/kernel/vdso.c
> @@ -199,16 +199,15 @@ int arch_setup_additional_pages(struct linux_binprm *bprm,
>   */
>  void update_vsyscall(struct timekeeper *tk)
>  {
> -       struct timespec xtime_coarse;
>         u32 use_syscall = strcmp(tk->tkr_mono.clock->name, "arch_sys_counter");
>
>         ++vdso_data->tb_seq_count;
>         smp_wmb();
>
> -       xtime_coarse = __current_kernel_time();
>         vdso_data->use_syscall                  = use_syscall;
> -       vdso_data->xtime_coarse_sec             = xtime_coarse.tv_sec;
> -       vdso_data->xtime_coarse_nsec            = xtime_coarse.tv_nsec;
> +       vdso_data->xtime_coarse_sec             = tk->xtime_sec;
> +       vdso_data->xtime_coarse_nsec            = tk->tkr_mono.xtime_nsec >>
> +                                                       tk->tkr_mono.shift;
>         vdso_data->wtm_clock_sec                = tk->wall_to_monotonic.tv_sec;
>         vdso_data->wtm_clock_nsec               = tk->wall_to_monotonic.tv_nsec;
>

(Sorry for the slow response, I've been out on vacation).

If its worth anything now:
Acked-by: John Stultz <john.stultz@linaro.org>

thanks
-john

[PATCH v2] ARM: VDSO: fix coarse clock monotonicity regression

[Nathan Lynch]

Since 906c55579a63 ("timekeeping: Copy the shadow-timekeeper over the
real timekeeper last") it has become possible on ARM to:

- Obtain a CLOCK_MONOTONIC_COARSE or CLOCK_REALTIME_COARSE timestamp
  via syscall.
- Subsequently obtain a timestamp for the same clock ID via VDSO which
  predates the first timestamp (by one jiffy).

This is because ARM's update_vsyscall is deriving the coarse time
using the __current_kernel_time interface, when it should really be
using the timekeeper object provided to it by the timekeeping core.
It happened to work before only because __current_kernel_time would
access the same timekeeper object which had been passed to
update_vsyscall.  This is no longer the case.

Signed-off-by: Nathan Lynch <nathan_lynch@mentor.com>
---

Changes since v1:
- Add u32 cast to nsec calculation.

 arch/arm/kernel/vdso.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/arm/kernel/vdso.c b/arch/arm/kernel/vdso.c
index efe17dd9b921..54a5aeab988d 100644
--- a/arch/arm/kernel/vdso.c
+++ b/arch/arm/kernel/vdso.c
@@ -296,7 +296,6 @@ static bool tk_is_cntvct(const struct timekeeper *tk)
  */
 void update_vsyscall(struct timekeeper *tk)
 {
-	struct timespec xtime_coarse;
 	struct timespec64 *wtm = &tk->wall_to_monotonic;
 
 	if (!cntvct_ok) {
@@ -308,10 +307,10 @@ void update_vsyscall(struct timekeeper *tk)
 
 	vdso_write_begin(vdso_data);
 
-	xtime_coarse = __current_kernel_time();
 	vdso_data->tk_is_cntvct			= tk_is_cntvct(tk);
-	vdso_data->xtime_coarse_sec		= xtime_coarse.tv_sec;
-	vdso_data->xtime_coarse_nsec		= xtime_coarse.tv_nsec;
+	vdso_data->xtime_coarse_sec		= tk->xtime_sec;
+	vdso_data->xtime_coarse_nsec		= (u32)(tk->tkr_mono.xtime_nsec >>
+							tk->tkr_mono.shift);
 	vdso_data->wtm_clock_sec		= wtm->tv_sec;
 	vdso_data->wtm_clock_nsec		= wtm->tv_nsec;
 
-- 
2.1.0

[PATCH v2] ARM: VDSO: fix coarse clock monotonicity regression

[Will Deacon]

On Mon, Aug 10, 2015 at 04:46:32PM +0100, Nathan Lynch wrote:
> Since 906c55579a63 ("timekeeping: Copy the shadow-timekeeper over the
> real timekeeper last") it has become possible on ARM to:
> 
> - Obtain a CLOCK_MONOTONIC_COARSE or CLOCK_REALTIME_COARSE timestamp
>   via syscall.
> - Subsequently obtain a timestamp for the same clock ID via VDSO which
>   predates the first timestamp (by one jiffy).
> 
> This is because ARM's update_vsyscall is deriving the coarse time
> using the __current_kernel_time interface, when it should really be
> using the timekeeper object provided to it by the timekeeping core.
> It happened to work before only because __current_kernel_time would
> access the same timekeeper object which had been passed to
> update_vsyscall.  This is no longer the case.
> 
> Signed-off-by: Nathan Lynch <nathan_lynch@mentor.com>
> ---
> 
> Changes since v1:
> - Add u32 cast to nsec calculation.

Acked-by: Will Deacon <will.deacon@arm.com>

Probably best sticking it into Russell's patch system with a Cc stable
in case it doesn't make it for 4.2.

Will

>  arch/arm/kernel/vdso.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/kernel/vdso.c b/arch/arm/kernel/vdso.c
> index efe17dd9b921..54a5aeab988d 100644
> --- a/arch/arm/kernel/vdso.c
> +++ b/arch/arm/kernel/vdso.c
> @@ -296,7 +296,6 @@ static bool tk_is_cntvct(const struct timekeeper *tk)
>   */
>  void update_vsyscall(struct timekeeper *tk)
>  {
> -	struct timespec xtime_coarse;
>  	struct timespec64 *wtm = &tk->wall_to_monotonic;
>  
>  	if (!cntvct_ok) {
> @@ -308,10 +307,10 @@ void update_vsyscall(struct timekeeper *tk)
>  
>  	vdso_write_begin(vdso_data);
>  
> -	xtime_coarse = __current_kernel_time();
>  	vdso_data->tk_is_cntvct			= tk_is_cntvct(tk);
> -	vdso_data->xtime_coarse_sec		= xtime_coarse.tv_sec;
> -	vdso_data->xtime_coarse_nsec		= xtime_coarse.tv_nsec;
> +	vdso_data->xtime_coarse_sec		= tk->xtime_sec;
> +	vdso_data->xtime_coarse_nsec		= (u32)(tk->tkr_mono.xtime_nsec >>
> +							tk->tkr_mono.shift);
>  	vdso_data->wtm_clock_sec		= wtm->tv_sec;
>  	vdso_data->wtm_clock_nsec		= wtm->tv_nsec;
>  
> -- 
> 2.1.0
> 

[PATCH v2] ARM: VDSO: fix coarse clock monotonicity regression

[John Stultz]

On Mon, Aug 10, 2015 at 8:46 AM, Nathan Lynch <nathan_lynch@mentor.com> wrote:
> Since 906c55579a63 ("timekeeping: Copy the shadow-timekeeper over the
> real timekeeper last") it has become possible on ARM to:

Apologies I didn't catch that the core change caused a regression for
ARM. Though fixing the ARM vdso logic in this way is a good idea to
avoid future problems.


> - Obtain a CLOCK_MONOTONIC_COARSE or CLOCK_REALTIME_COARSE timestamp
>   via syscall.
> - Subsequently obtain a timestamp for the same clock ID via VDSO which
>   predates the first timestamp (by one jiffy).
>
> This is because ARM's update_vsyscall is deriving the coarse time
> using the __current_kernel_time interface, when it should really be
> using the timekeeper object provided to it by the timekeeping core.
> It happened to work before only because __current_kernel_time would
> access the same timekeeper object which had been passed to
> update_vsyscall.  This is no longer the case.
>
> Signed-off-by: Nathan Lynch <nathan_lynch@mentor.com>
> ---
>
> Changes since v1:
> - Add u32 cast to nsec calculation.
>
>  arch/arm/kernel/vdso.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/kernel/vdso.c b/arch/arm/kernel/vdso.c
> index efe17dd9b921..54a5aeab988d 100644
> --- a/arch/arm/kernel/vdso.c
> +++ b/arch/arm/kernel/vdso.c
> @@ -296,7 +296,6 @@ static bool tk_is_cntvct(const struct timekeeper *tk)
>   */
>  void update_vsyscall(struct timekeeper *tk)
>  {
> -       struct timespec xtime_coarse;
>         struct timespec64 *wtm = &tk->wall_to_monotonic;
>
>         if (!cntvct_ok) {
> @@ -308,10 +307,10 @@ void update_vsyscall(struct timekeeper *tk)
>
>         vdso_write_begin(vdso_data);
>
> -       xtime_coarse = __current_kernel_time();
>         vdso_data->tk_is_cntvct                 = tk_is_cntvct(tk);
> -       vdso_data->xtime_coarse_sec             = xtime_coarse.tv_sec;
> -       vdso_data->xtime_coarse_nsec            = xtime_coarse.tv_nsec;
> +       vdso_data->xtime_coarse_sec             = tk->xtime_sec;
> +       vdso_data->xtime_coarse_nsec            = (u32)(tk->tkr_mono.xtime_nsec >>
> +                                                       tk->tkr_mono.shift);
>         vdso_data->wtm_clock_sec                = wtm->tv_sec;
>         vdso_data->wtm_clock_nsec               = wtm->tv_nsec;

Acked-by: John Stultz <john.stultz@linaro.org>

thanks
-john