From patchwork Thu Dec 10 19:51:35 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Stultz X-Patchwork-Id: 58240 Delivered-To: patches@linaro.org Received: by 10.112.147.194 with SMTP id tm2csp709018lbb; Thu, 10 Dec 2015 11:51:45 -0800 (PST) X-Received: by 10.66.219.228 with SMTP id pr4mr18593655pac.99.1449777104226; Thu, 10 Dec 2015 11:51:44 -0800 (PST) Return-Path: Received: from mail-pf0-x231.google.com (mail-pf0-x231.google.com. [2607:f8b0:400e:c00::231]) by mx.google.com with ESMTPS id 29si22068242pfq.225.2015.12.10.11.51.44 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 10 Dec 2015 11:51:44 -0800 (PST) Received-SPF: pass (google.com: domain of john.stultz@linaro.org designates 2607:f8b0:400e:c00::231 as permitted sender) client-ip=2607:f8b0:400e:c00::231; Authentication-Results: mx.google.com; spf=pass (google.com: domain of john.stultz@linaro.org designates 2607:f8b0:400e:c00::231 as permitted sender) smtp.mailfrom=john.stultz@linaro.org; dkim=pass header.i=@linaro-org.20150623.gappssmtp.com Received: by pfbg73 with SMTP id g73so54047054pfb.1 for ; Thu, 10 Dec 2015 11:51:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro-org.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=0RQixA6QBacJClSTiWO2fzDFR01ZCyNITqhVJdgj+C8=; b=XfLdIXX3GELuojBWhnZet8ju9ObISu/Olv7d17J88FKskAaXlt0e196FZIr/YWVAS7 8fwjJixBxvXbPjfQU0g5GurUgAznVd9iiC4Aot3zyv5D9GKRYza8ToKtqe4iaK0rDiDu bpscCkbqOZB5RfgcTTWOF8oBJK3ak0qzqAnqxaMsV5gwM+FMn6JQ8uLkXjau9LvcCGP0 f0GVVc/V4zWWHrMr/WQ08qjSiCmDZt8iv1VYyfc9flL7mDDmMUBCBJrDivAkD5qyIXWQ AhwxOK+Vm+b6wRR7JoL1hsvm3oZjiJvR+lxn/nEYCQPHi1Njhm+6QNZ+qeM3jXUMFI9o 3yBw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=0RQixA6QBacJClSTiWO2fzDFR01ZCyNITqhVJdgj+C8=; b=O8gN0Jkocrhm5/aE3tb0gVg5OYPEHgQQmm73mTr6sGGMrXT3XJU7W2vcPUAyRWN8e8 Kyaxzv1uvqrnxPR6ABuTX9Mki8Nvy8Kf29PEekjYxRmM5nEn7HivrKBt5Lt8VMyKuHwK WydHXZdIrAzmQA/GHZgepKNUAdoRaKd+eDfPfizcivFQB3pBWRveDa2K4t37ZY1g7LCb qQDOpHdYDgFWAGHy6ZPo/SDeNwmfkW4pa/nnRNvZXmBoEtjVKuBo+YtUYgM1ntsCQPHr It7yzAERggjkPWymJZjBRJB5K/8R3/VgN2dDcN1bH3PAMPwPEuiQRYwlk+ULup4/4dn3 AppQ== X-Gm-Message-State: ALoCoQnq3rb+0B+2YsI6Va9mXQKgTTREe5/fe6OO+qBb+hvWYPUaC3Byn+XnDcuRN0z/u3eZrK84DvAdwyEGLf3jg0c+TdIorg== X-Received: by 10.98.69.157 with SMTP id n29mr9550510pfi.36.1449777103857; Thu, 10 Dec 2015 11:51:43 -0800 (PST) Return-Path: Received: from localhost.localdomain (c-76-115-103-22.hsd1.or.comcast.net. [76.115.103.22]) by smtp.gmail.com with ESMTPSA id ck9sm20552615pad.28.2015.12.10.11.51.43 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 10 Dec 2015 11:51:43 -0800 (PST) From: John Stultz To: lkml Cc: John Stultz , Sasha Levin , Richard Cochran , Thomas Gleixner Subject: [PATCH v3] time: Verify time values in adjtimex ADJ_SETOFFSET to avoid overflow Date: Thu, 10 Dec 2015 11:51:35 -0800 Message-Id: <1449777095-21981-2-git-send-email-john.stultz@linaro.org> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1449777095-21981-1-git-send-email-john.stultz@linaro.org> References: <1449777095-21981-1-git-send-email-john.stultz@linaro.org> For adjtimex()'s ADJ_SETOFFSET, make sure the tv_usec value is sane. We might multiply them later which can cause an overflow and undefined behavior. This patch introduces new helper functions to simplify the checking code and adds comments to clarify Orginally this patch was by Sasha Levin, but I've basically rewritten it, so he should get credit for finding the issue and I should get the blame for any mistakes made since. Also, credit to Richard Cochran for the phrasing used in the comment for what is considered valid here. Cc: Sasha Levin Cc: Richard Cochran Cc: Thomas Gleixner Reported-by: Sasha Levin Signed-off-by: John Stultz --- v3: * Reworked to use helper functions, per tglx's request. * Re-attributed authorship, since its been totally redone. include/linux/time.h | 26 ++++++++++++++++++++++++++ kernel/time/ntp.c | 10 ++++++++-- kernel/time/timekeeping.c | 2 +- 3 files changed, 35 insertions(+), 3 deletions(-) -- 1.9.1 diff --git a/include/linux/time.h b/include/linux/time.h index beebe3a..297f09f 100644 --- a/include/linux/time.h +++ b/include/linux/time.h @@ -125,6 +125,32 @@ static inline bool timeval_valid(const struct timeval *tv) extern struct timespec timespec_trunc(struct timespec t, unsigned gran); +/* + * Validates if a timespec/timeval used to inject a time offset is valid. + * Offsets can be postive or negative. The value of the timeval/timespec + * is the sum of its fields, but *NOTE*: the field tv_usec/tv_nsec must + * always be non-negative. + */ +static inline bool timeval_inject_offset_valid(const struct timeval *tv) +{ + /* We don't check the tv_sec as it can be positive or negative */ + + /* Can't have more microseconds then a second */ + if (tv->tv_usec < 0 || tv->tv_usec >= USEC_PER_SEC) + return false; + return true; +} + +static inline bool timespec_inject_offset_valid(const struct timespec *ts) +{ + /* We don't check the tv_sec as it can be positive or negative */ + + /* Can't have more nanoseconds then a second */ + if (ts->tv_nsec < 0 || ts->tv_nsec >= NSEC_PER_SEC) + return false; + return true; +} + #define CURRENT_TIME (current_kernel_time()) #define CURRENT_TIME_SEC ((struct timespec) { get_seconds(), 0 }) diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c index 125fc03..4073c95 100644 --- a/kernel/time/ntp.c +++ b/kernel/time/ntp.c @@ -676,8 +676,14 @@ int ntp_validate_timex(struct timex *txc) return -EINVAL; } - if ((txc->modes & ADJ_SETOFFSET) && (!capable(CAP_SYS_TIME))) - return -EPERM; + if (txc->modes & ADJ_SETOFFSET) { + /* In order to inject time, you gotta be super-user! */ + if (!capable(CAP_SYS_TIME)) + return -EPERM; + + if (!timeval_inject_offset_valid(&txc->time)) + return -EINVAL; + } /* * Check for potential multiplication overflows that can diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index 99188ee..d9249da 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -958,7 +958,7 @@ int timekeeping_inject_offset(struct timespec *ts) struct timespec64 ts64, tmp; int ret = 0; - if ((unsigned long)ts->tv_nsec >= NSEC_PER_SEC) + if (!timespec_inject_offset_valid(ts)) return -EINVAL; ts64 = timespec_to_timespec64(*ts);