From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753043AbbLFAP6 (ORCPT ); Sat, 5 Dec 2015 19:15:58 -0500 Received: from userp1040.oracle.com ([156.151.31.81]:41222 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752223AbbLFAP5 (ORCPT ); Sat, 5 Dec 2015 19:15:57 -0500 Subject: Re: [PATCH] time: verify time values in adjtimex ADJ_SETOFFSET to avoid overflow To: Thomas Gleixner References: <1449198571-21133-1-git-send-email-sasha.levin@oracle.com> Cc: john.stultz@linaro.org, linux-kernel@vger.kernel.org From: Sasha Levin Message-ID: <56637E2D.1040603@oracle.com> Date: Sat, 5 Dec 2015 19:15:41 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Source-IP: aserv0021.oracle.com [141.146.126.233] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 12/05/2015 12:10 PM, Thomas Gleixner wrote: > On Thu, 3 Dec 2015, Sasha Levin wrote: > >> Make sure the tv_usec makes sense. We might multiply them later which can >> cause an overflow and undefined behavior. >> >> Signed-off-by: Sasha Levin >> --- >> kernel/time/timekeeping.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c >> index d563c19..aa3c1c2 100644 >> --- a/kernel/time/timekeeping.c >> +++ b/kernel/time/timekeeping.c >> @@ -1987,6 +1987,10 @@ int do_adjtimex(struct timex *txc) >> >> if (txc->modes & ADJ_SETOFFSET) { >> struct timespec delta; >> + >> + if (txc->time.tv_usec >= USEC_PER_SEC || txc->time.tv_usec <= -USEC_PER_SEC) >> + return -EINVAL; > > That's not a canonical timeval. timeval_valid() is what you want to > check it. Or has adjtimex some magic exception here? Nope, it looks like timeval_valid() is indeed what I've needed to use. Is there a reason ntp_validate_timex() doesn't do timeval_valid() too for at least the ADJ_SETOFFSET case? If not, I'll add it in. Thanks, Sasha