Re: [tip:timers/core] posix-timers: Zero out oldval itimerspec

[Andrei Vagin]

On Mon, Jun 12, 2017 at 12:13:15PM -0700, tip-bot for Thomas Gleixner wrote:
> Commit-ID:  5c7a3a3d20a4e175304c0e23809e3d70be8fed8a
> Gitweb:     http://git.kernel.org/tip/5c7a3a3d20a4e175304c0e23809e3d70be8fed8a
> Author:     Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> AuthorDate: Mon, 12 Jun 2017 19:44:09 +0200
> Committer:  Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> CommitDate: Mon, 12 Jun 2017 21:07:40 +0200
> 
> posix-timers: Zero out oldval itimerspec
> 
> The recent posix timer rework moved the clearing of the itimerspec to the
> real syscall implementation, but forgot that the kclock->timer_get() is
> used by timer_settime() as well. That results in an uninitialized variable
> and bogus values returned to user space.
> 
> Add the missing memset to timer_settime().
> 
> Fixes: eabdec043853 ("posix-timers: Zero settings value in common code")
> Reported-by: Andrei Vagin <avagin@xxxxxxxxxxxxx>
> Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: John Stultz <john.stultz@xxxxxxxxxx>
> Cc: Cyrill Gorcunov <gorcunov@xxxxxxxxxx>
> Link: http://lkml.kernel.org/r/20170609201156.GB21491@xxxxxxxxxxxxxxxxxxxxx
> ---
>  kernel/time/posix-timers.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c
> index b53a0b5..88517dc 100644
> --- a/kernel/time/posix-timers.c
> +++ b/kernel/time/posix-timers.c
> @@ -828,6 +828,8 @@ SYSCALL_DEFINE4(timer_settime, timer_t, timer_id, int, flags,
>  	if (!timespec64_valid(&new_spec64.it_interval) ||
>  	    !timespec64_valid(&new_spec64.it_value))
>  		return -EINVAL;
> +	if (rtn)
> +		memset(rtn, 0, sizeof(*rtn));

Maybe we need to call memset after "retry:"?

common_timer_get() is called at the begining of common_timer_set(), then
common_timer_set() can return TIMER_RETRY. common_timer_get() will be
called again and some fields of rtn which have been touched first time
will not be touched.

At the end, rtn will contain data from two executions of
common_timer_get().

Thanks,
Andrei

>  retry:
>  	timr = lock_timer(timer_id, &flag);
>  	if (!timr)