From patchwork Sat Mar 31 01:52:36 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Kees Cook X-Patchwork-Id: 10318489 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id BC3A160380 for ; Sat, 31 Mar 2018 01:52:56 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 98AC129337 for ; Sat, 31 Mar 2018 01:52:56 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 8CE982A4B1; Sat, 31 Mar 2018 01:52:56 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU autolearn=ham version=3.3.1 Received: from mother.openwall.net (mother.openwall.net [195.42.179.200]) by mail.wl.linuxfoundation.org (Postfix) with SMTP id 107DD29337 for ; Sat, 31 Mar 2018 01:52:54 +0000 (UTC) Received: (qmail 22274 invoked by uid 550); 31 Mar 2018 01:52:52 -0000 Mailing-List: contact kernel-hardening-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Delivered-To: mailing list kernel-hardening@lists.openwall.com Received: (qmail 22237 invoked from network); 31 Mar 2018 01:52:51 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=date:from:to:cc:subject:message-id:mime-version:content-disposition :content-transfer-encoding; bh=pU4bnVT94aB2863bO/FEp94lh2qIMfiybQq0IjDRgr4=; b=aIJfVgkhc8f23ynhlQRf1rHEdp8q0fWjOx4izC1DqsLTzRONPTaH+3ERh+sACXfXhR uVkdiQWYbP5rQPApaGfngril3CMXQXY/N8RW5rUIjCsQDufS16hPSQucDMDI4OcnVrqA ekDRpgK3jQqeLyKrT0yLN65OOzDNjOqI5jbvY= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:mime-version :content-disposition:content-transfer-encoding; bh=pU4bnVT94aB2863bO/FEp94lh2qIMfiybQq0IjDRgr4=; b=k+Hmy364Wcd7G8XxPEj0wreWzlct0e9HqVOahEB1S9k5AAiU7B7HBaMgm4F1kern6l NJxltuQoNOhX0VRCSMz+A4fgzZrzqQya0Qp/Q988xnRrTYjQ/6BizK3hFKwSJ1bTOUgL QxheKQWMFnHa8ZwV4PfjePRznJDSPcDHAj3X1JYEolzYx3FzqdUEz876GV1Cor8TNFSK I7ScVscjqaZTNalc+oOxpD7atQf8SOj/3BBaORIlbszPSCX/LL7iG575dH5VlMtLcfQP PeXlyVcCg15O3HjsQKzjN07+J1PHyv5dpjHBK1eeiJ3mTFH5fEK17I9totSrrfx75YB9 c15w== X-Gm-Message-State: AElRT7G97pNN1p904CxxOF7yabdDKcXFnGqB9BbeL0EQrnZzBo5cWI1b 5TnitaFNwlo33VtHCi6Z5+K6Fg== X-Google-Smtp-Source: AIpwx4/TiVzHxV8NnqKRk5xYpadk9fc/32ZKyt06C6g3ssL5jRKxysXBa7Et/vL3vPLPLbe6p7UDDQ== X-Received: by 10.99.112.92 with SMTP id a28mr770352pgn.17.1522461159044; Fri, 30 Mar 2018 18:52:39 -0700 (PDT) Date: Fri, 30 Mar 2018 18:52:36 -0700 From: Kees Cook To: Andrew Morton Cc: Linus Torvalds , Martin Uecker , Josh Poimboeuf , Rasmus Villemoes , Randy Dunlap , Miguel Ojeda , Ingo Molnar , David Laight , Ian Abbott , linux-kernel@vger.kernel.org, kernel-hardening@lists.openwall.com Subject: [PATCH v7] kernel.h: Retain constant expression output for max()/min() Message-ID: <20180331015236.GA3852@beast> MIME-Version: 1.0 Content-Disposition: inline X-Virus-Scanned: ClamAV using ClamSMTP In the effort to remove all VLAs from the kernel[1], it is desirable to build with -Wvla. However, this warning is overly pessimistic, in that it is only happy with stack array sizes that are declared as constant expressions, and not constant values. One case of this is the evaluation of the max() macro which, due to its construction, ends up converting constant expression arguments into a constant value result. All attempts to rewrite this macro with __builtin_constant_p() failed with older compilers (e.g. gcc 4.4)[2]. However, Martin Uecker, constructed[3] a mind-shattering solution that works everywhere. Cthulhu fhtagn! This patch updates the min()/max() macros to evaluate to a constant expression when called on constant expression arguments. This removes several false-positive stack VLA warnings from an x86 allmodconfig build when -Wvla is added: $ diff -u before.txt after.txt | grep ^- -drivers/input/touchscreen/cyttsp4_core.c:871:2: warning: ISO C90 forbids variable length array ‘ids’ [-Wvla] -fs/btrfs/tree-checker.c:344:4: warning: ISO C90 forbids variable length array ‘namebuf’ [-Wvla] -lib/vsprintf.c:747:2: warning: ISO C90 forbids variable length array ‘sym’ [-Wvla] -net/ipv4/proc.c:403:2: warning: ISO C90 forbids variable length array ‘buff’ [-Wvla] -net/ipv6/proc.c:198:2: warning: ISO C90 forbids variable length array ‘buff’ [-Wvla] -net/ipv6/proc.c:218:2: warning: ISO C90 forbids variable length array ‘buff64’ [-Wvla] This also updates the one case where different enums were being compared and explicitly casts them to int (which matches the old side-effect of the single-evaluation code). [1] https://lkml.org/lkml/2018/3/7/621 [2] https://lkml.org/lkml/2018/3/10/170 [3] https://lkml.org/lkml/2018/3/20/845 Co-Developed-by: Linus Torvalds Co-Developed-by: Martin Uecker Signed-off-by: Kees Cook Acked-by: Ingo Molnar Acked-by: Miguel Ojeda --- v7: - __is_constant() renamed to __is_constexpr() (Miguel Ojeda) - adjust memory offset from 1 to 8 (David Laight) - min_t()/max_t() "t" renamed back to "type" (0-day bot) - add Acks --- drivers/char/tpm/tpm_tis_core.h | 8 ++--- include/linux/kernel.h | 71 ++++++++++++++++++++++++----------------- 2 files changed, 45 insertions(+), 34 deletions(-) diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h index d5c6a2e952b3..f6e1dbe212a7 100644 --- a/drivers/char/tpm/tpm_tis_core.h +++ b/drivers/char/tpm/tpm_tis_core.h @@ -62,10 +62,10 @@ enum tis_defaults { /* Some timeout values are needed before it is known whether the chip is * TPM 1.0 or TPM 2.0. */ -#define TIS_TIMEOUT_A_MAX max(TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_A) -#define TIS_TIMEOUT_B_MAX max(TIS_LONG_TIMEOUT, TPM2_TIMEOUT_B) -#define TIS_TIMEOUT_C_MAX max(TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_C) -#define TIS_TIMEOUT_D_MAX max(TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_D) +#define TIS_TIMEOUT_A_MAX max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_A) +#define TIS_TIMEOUT_B_MAX max_t(int, TIS_LONG_TIMEOUT, TPM2_TIMEOUT_B) +#define TIS_TIMEOUT_C_MAX max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_C) +#define TIS_TIMEOUT_D_MAX max_t(int, TIS_SHORT_TIMEOUT, TPM2_TIMEOUT_D) #define TPM_ACCESS(l) (0x0000 | ((l) << 12)) #define TPM_INT_ENABLE(l) (0x0008 | ((l) << 12)) diff --git a/include/linux/kernel.h b/include/linux/kernel.h index 3fd291503576..87acb8b58ae9 100644 --- a/include/linux/kernel.h +++ b/include/linux/kernel.h @@ -783,41 +783,58 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { } #endif /* CONFIG_TRACING */ /* - * min()/max()/clamp() macros that also do - * strict type-checking.. See the - * "unnecessary" pointer comparison. + * min()/max()/clamp() macros must accomplish three things: + * + * - avoid multiple evaluations of the arguments (so side-effects like + * "x++" happen only once) when non-constant. + * - perform strict type-checking (to generate warnings instead of + * nasty runtime surprises). See the "unnecessary" pointer comparison + * in __typecheck(). + * - retain result as a constant expressions when called with only + * constant expressions (to avoid tripping VLA warnings in stack + * allocation usage). + */ +#define __typecheck(x, y) \ + (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1))) + +/* + * This returns a constant expression while determining if an argument is + * a constant expression, most importantly without evaluating the argument. + * Glory to Martin Uecker */ -#define __min(t1, t2, min1, min2, x, y) ({ \ - t1 min1 = (x); \ - t2 min2 = (y); \ - (void) (&min1 == &min2); \ - min1 < min2 ? min1 : min2; }) +#define __is_constexpr(x) \ + (sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8))) + +#define __no_side_effects(x, y) \ + (__is_constexpr(x) && __is_constexpr(y)) + +#define __safe_cmp(x, y) \ + (__typecheck(x, y) && __no_side_effects(x, y)) + +#define __cmp(x, y, op) ((x) op (y) ? (x) : (y)) + +#define __cmp_once(x, y, op) ({ \ + typeof(x) __x = (x); \ + typeof(y) __y = (y); \ + __cmp(__x, __y, op); }) + +#define __careful_cmp(x, y, op) \ + __builtin_choose_expr(__safe_cmp(x, y), \ + __cmp(x, y, op), __cmp_once(x, y, op)) /** * min - return minimum of two values of the same or compatible types * @x: first value * @y: second value */ -#define min(x, y) \ - __min(typeof(x), typeof(y), \ - __UNIQUE_ID(min1_), __UNIQUE_ID(min2_), \ - x, y) - -#define __max(t1, t2, max1, max2, x, y) ({ \ - t1 max1 = (x); \ - t2 max2 = (y); \ - (void) (&max1 == &max2); \ - max1 > max2 ? max1 : max2; }) +#define min(x, y) __careful_cmp(x, y, <) /** * max - return maximum of two values of the same or compatible types * @x: first value * @y: second value */ -#define max(x, y) \ - __max(typeof(x), typeof(y), \ - __UNIQUE_ID(max1_), __UNIQUE_ID(max2_), \ - x, y) +#define max(x, y) __careful_cmp(x, y, >) /** * min3 - return minimum of three values @@ -869,10 +886,7 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { } * @x: first value * @y: second value */ -#define min_t(type, x, y) \ - __min(type, type, \ - __UNIQUE_ID(min1_), __UNIQUE_ID(min2_), \ - x, y) +#define min_t(type, x, y) __careful_cmp((type)(x), (type)(y), <) /** * max_t - return maximum of two values, using the specified type @@ -880,10 +894,7 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { } * @x: first value * @y: second value */ -#define max_t(type, x, y) \ - __max(type, type, \ - __UNIQUE_ID(min1_), __UNIQUE_ID(min2_), \ - x, y) +#define max_t(type, x, y) __careful_cmp((type)(x), (type)(y), >) /** * clamp_t - return a value clamped to a given range using a given type