From patchwork Fri Jun 28 07:42:30 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Po-Hsu Lin X-Patchwork-Id: 1124029 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=canonical.com Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 45Zpdh6V1dz9s9h; Fri, 28 Jun 2019 17:42:47 +1000 (AEST) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1hglWt-0000wy-TU; Fri, 28 Jun 2019 07:42:43 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtps (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:128) (Exim 4.86_2) (envelope-from ) id 1hglWq-0000wI-Eb for kernel-team@lists.ubuntu.com; Fri, 28 Jun 2019 07:42:40 +0000 Received: from mail-pf1-f199.google.com ([209.85.210.199]) by youngberry.canonical.com with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.76) (envelope-from ) id 1hglWp-00054a-VU for kernel-team@lists.ubuntu.com; Fri, 28 Jun 2019 07:42:40 +0000 Received: by mail-pf1-f199.google.com with SMTP id i26so3328182pfo.22 for ; Fri, 28 Jun 2019 00:42:39 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references; bh=+uOMEhsl1Vk8y1rW4XpqV2HQQJjIsVBUPtFPYt3nCqg=; b=gC0q4Vhg0+dYS3EYLGB0O/I9mBlqswyzu7/2oSI2GeNRyj4rNriDgL4R+KtyOTrsS+ N1Wmn7gxudu00ozq9Ny/k3vhwUXFCTC+so+zE4eSLQZ5OGFN6E4GH9Iri1K3rklXVKXj rDH3X1VhP2roxFBXEmMrLRfDpTuxGgW/qBVGyXLf+76Z5qKSyyembm+PVkeK5VV/F8fW 9qZaUXVKgNsxh3dPsjtWwljAltsNmHBCXNC13rJoGi1/4ZvArt7YQqR/Rc87XAG//j7v xPSOLEBq3N9uYcEPnSgOYIXT9EbH4Li0huuNHhEJkOxZK3Xpxo4Ca8qB2Jy6xOfvJnVZ OjGg== X-Gm-Message-State: APjAAAXiX3F0aj6+ibCx5g2Q77SpnWFN3wqK8/0rWx9cqN7z9bY3nEch Ae1U8+P50lwkjqHSLvPZnOXcCWqU4FhZhOPkpxB28NZTay867ZLTSsHOUwkV/cmuhIw7aT5cbpm McChTitG2pUUAmITste2GCCnCQVRV6jZ+As4SUPCj X-Received: by 2002:a17:90a:22ef:: with SMTP id s102mr11719280pjc.2.1561707758457; Fri, 28 Jun 2019 00:42:38 -0700 (PDT) X-Google-Smtp-Source: APXvYqzb5AEDRiIyH7KFBjYfHlRCQ45oGzl0DPsq3q6Ohn78DQ9+mOzBl8wq9Cj7FKp1JenscghYqw== X-Received: by 2002:a17:90a:22ef:: with SMTP id s102mr11719251pjc.2.1561707758153; Fri, 28 Jun 2019 00:42:38 -0700 (PDT) Received: from Leggiero.taipei.internal (61-220-137-37.HINET-IP.hinet.net. [61.220.137.37]) by smtp.gmail.com with ESMTPSA id h6sm1384698pfn.79.2019.06.28.00.42.37 for (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Fri, 28 Jun 2019 00:42:37 -0700 (PDT) From: Po-Hsu Lin To: kernel-team@lists.ubuntu.com Subject: [D][Unstable][SRU][PATCH 1/1] sysctl: handle overflow in proc_get_long Date: Fri, 28 Jun 2019 15:42:30 +0800 Message-Id: <20190628074230.838-4-po-hsu.lin@canonical.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20190628074230.838-1-po-hsu.lin@canonical.com> References: <20190628074230.838-1-po-hsu.lin@canonical.com> X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Christian Brauner BugLink: https://bugs.launchpad.net/bugs/1833935 proc_get_long() is a funny function. It uses simple_strtoul() and for a good reason. proc_get_long() wants to always succeed the parse and return the maybe incorrect value and the trailing characters to check against a pre-defined list of acceptable trailing values. However, simple_strtoul() explicitly ignores overflows which can cause funny things like the following to happen: echo 18446744073709551616 > /proc/sys/fs/file-max cat /proc/sys/fs/file-max 0 (Which will cause your system to silently die behind your back.) On the other hand kstrtoul() does do overflow detection but does not return the trailing characters, and also fails the parse when anything other than '\n' is a trailing character whereas proc_get_long() wants to be more lenient. Now, before adding another kstrtoul() function let's simply add a static parse strtoul_lenient() which: - fails on overflow with -ERANGE - returns the trailing characters to the caller The reason why we should fail on ERANGE is that we already do a partial fail on overflow right now. Namely, when the TMPBUFLEN is exceeded. So we already reject values such as 184467440737095516160 (21 chars) but accept values such as 18446744073709551616 (20 chars) but both are overflows. So we should just always reject 64bit overflows and not special-case this based on the number of chars. Link: http://lkml.kernel.org/r/20190107222700.15954-2-christian@brauner.io Signed-off-by: Christian Brauner Acked-by: Kees Cook Cc: "Eric W. Biederman" Cc: Luis Chamberlain Cc: Joe Lawrence Cc: Waiman Long Cc: Dominik Brodowski Cc: Al Viro Cc: Alexey Dobriyan Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds (cherry picked from commit 7f2923c4f73f21cfd714d12a2d48de8c21f11cfe) Signed-off-by: Po-Hsu Lin --- kernel/sysctl.c | 40 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 7e75254..7c1a10e 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -67,6 +67,8 @@ #include #include +#include "../lib/kstrtox.h" + #include #include @@ -2108,6 +2110,41 @@ static void proc_skip_char(char **buf, size_t *size, const char v) } } +/** + * strtoul_lenient - parse an ASCII formatted integer from a buffer and only + * fail on overflow + * + * @cp: kernel buffer containing the string to parse + * @endp: pointer to store the trailing characters + * @base: the base to use + * @res: where the parsed integer will be stored + * + * In case of success 0 is returned and @res will contain the parsed integer, + * @endp will hold any trailing characters. + * This function will fail the parse on overflow. If there wasn't an overflow + * the function will defer the decision what characters count as invalid to the + * caller. + */ +static int strtoul_lenient(const char *cp, char **endp, unsigned int base, + unsigned long *res) +{ + unsigned long long result; + unsigned int rv; + + cp = _parse_integer_fixup_radix(cp, &base); + rv = _parse_integer(cp, base, &result); + if ((rv & KSTRTOX_OVERFLOW) || (result != (unsigned long)result)) + return -ERANGE; + + cp += rv; + + if (endp) + *endp = (char *)cp; + + *res = (unsigned long)result; + return 0; +} + #define TMPBUFLEN 22 /** * proc_get_long - reads an ASCII formatted integer from a user buffer @@ -2151,7 +2188,8 @@ static int proc_get_long(char **buf, size_t *size, if (!isdigit(*p)) return -EINVAL; - *val = simple_strtoul(p, &p, 0); + if (strtoul_lenient(p, &p, 0, val)) + return -EINVAL; len = p - tmp;