Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752690AbdFLWBY (ORCPT ); Mon, 12 Jun 2017 18:01:24 -0400 Received: from Galois.linutronix.de ([146.0.238.70]:34887 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752386AbdFLWBW (ORCPT ); Mon, 12 Jun 2017 18:01:22 -0400 Date: Tue, 13 Jun 2017 00:01:17 +0200 (CEST) From: Thomas Gleixner To: Andrei Vagin cc: tip-bot for Thomas Gleixner , linux-tip-commits@vger.kernel.org, hpa@zytor.com, linux-kernel@vger.kernel.org, gorcunov@openvz.org, john.stultz@linaro.org, mingo@kernel.org, peterz@infradead.org Subject: Re: [tip:timers/core] posix-timers: Zero out oldval itimerspec In-Reply-To: <20170612210610.GA23730@outlook.office365.com> Message-ID: References: <20170609201156.GB21491@outlook.office365.com> <20170612210610.GA23730@outlook.office365.com> User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1533 Lines: 49 On Mon, 12 Jun 2017, Andrei Vagin wrote: > > 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:"? That would be counter productive. > 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(). No. See the full code sequence: retry: timr = lock_timer(timer_id, &flag); if (!timr) return -EINVAL; kc = clockid_to_kclock(timr->it_clock); if (WARN_ON_ONCE(!kc || !kc->timer_set)) error = -EINVAL; else error = kc->timer_set(timr, flags, &new_spec64, rtn); unlock_timer(timr, flag); if (error == TIMER_RETRY) { rtn = NULL; // We already got the old time... goto retry; } If you clear it after retry, you'll get all zeros in the retry case. Not what you really want. Thanks, tglx