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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170612210610.GA23730@outlook.office365.com>
Date:   Mon, 12 Jun 2017 14:06:11 -0700
From:   Andrei Vagin <avagin@...tuozzo.com>
To:     tip-bot for Thomas Gleixner <tipbot@...or.com>
Cc:     linux-tip-commits@...r.kernel.org, hpa@...or.com,
        linux-kernel@...r.kernel.org, gorcunov@...nvz.org,
        john.stultz@...aro.org, mingo@...nel.org, tglx@...utronix.de,
        peterz@...radead.org
Subject: Re: [tip:timers/core] posix-timers: Zero out oldval itimerspec

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@...utronix.de>
> AuthorDate: Mon, 12 Jun 2017 19:44:09 +0200
> Committer:  Thomas Gleixner <tglx@...utronix.de>
> 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@...tuozzo.com>
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: John Stultz <john.stultz@...aro.org>
> Cc: Cyrill Gorcunov <gorcunov@...nvz.org>
> Link: http://lkml.kernel.org/r/20170609201156.GB21491@outlook.office365.com
> ---
>  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)