From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Google-Smtp-Source: AG47ELuhCvr1T2nK2uGieFZoGyiJxk9wiJH77lCI8ZhnlC8IhUKNKtAYGQmu5GqdB7fjslmHUtpb ARC-Seal: i=1; a=rsa-sha256; t=1520552020; cv=none; d=google.com; s=arc-20160816; b=ADk6hMPzVAwG3yPnmwLaP+lb5KbugiPe0JOnOiCZCiTF/zw0ppIIsvS+8qDpPXpBLk M/IXxqChb2oj7VfzP37IB7RaFMzqZsGAOCkvaDyJz3vSZzCuyQFqTHnvPk6YrQQKUxas dT7NUDkqpZ3YbNe1MUxD+KTVyWv+7T5HbMlFq1ozBtM/5fKkC7wlsXZKE+V4qyMcDgrI u9ii+ad9cQia2cHzGO3Gy/UPPcSnbIEfEd5CONIeJ8GS8iOcnHVxHf8B1P3QRYQfOI88 1YFEvtNkAT8Lu6RbDsw/P7qsrEaNM6DDyJYSizYVevoviXDaXIyFaOp5jKEQhruOoyUx dQ9Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=cc:to:subject:message-id:date:from:references:in-reply-to:sender :mime-version:dkim-signature:dkim-signature:delivered-to:list-id :list-subscribe:list-unsubscribe:list-help:list-post:precedence :mailing-list:arc-authentication-results; bh=Ju5CUDT32V7H9c41piopHtAvr5/l0bdWUI/ehziRGz0=; b=WE6acAnIwQwWf2m1giEGJAALVvMIg+V1xLJmApwJ7BoEKgXWKUWz+wPJONlA/HhKTN 4xRqz+yj56UOHyKK+KSzAsmjkyiCYWQak5hcTiDrcZvVnUkRYSATa5EJogMPBiz6rLhZ 3KZboxTQS2IAyEm/xLYbhfn5ZhEmvIMnwSOYAecsvWN9qaZAhkIPPWK1piS+uZQdyaNF mGk2A2ValMBaN4XjWTJbP6Nx+K1Rms6x1kaomIHJJpF1pOt8al9GK1V72vCvAEOOV/S+ Jim4vmdnAGVYoX7kErFJ/SBvFTkgB3J/WFxCuHHrr6irg/gnW/esEkaertpe0vVto6Co 1U3w== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20161025 header.b=uoeFUx/b; dkim=pass header.i=@chromium.org header.s=google header.b=Q9uYKqWV; spf=pass (google.com: domain of kernel-hardening-return-12281-gregkh=linuxfoundation.org@lists.openwall.com designates 195.42.179.200 as permitted sender) smtp.mailfrom=kernel-hardening-return-12281-gregkh=linuxfoundation.org@lists.openwall.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20161025 header.b=uoeFUx/b; dkim=pass header.i=@chromium.org header.s=google header.b=Q9uYKqWV; spf=pass (google.com: domain of kernel-hardening-return-12281-gregkh=linuxfoundation.org@lists.openwall.com designates 195.42.179.200 as permitted sender) smtp.mailfrom=kernel-hardening-return-12281-gregkh=linuxfoundation.org@lists.openwall.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Mailing-List: contact kernel-hardening-help@lists.openwall.com; run by ezmlm List-Post: List-Help: List-Unsubscribe: List-Subscribe: MIME-Version: 1.0 Sender: keescook@google.com In-Reply-To: References: <1520479847-39174-1-git-send-email-keescook@chromium.org> <20180308150236.5tysfbm3xdouii5n@treble> <78428dec-270e-5d74-0160-83feb5c0ff02@rasmusvillemoes.dk> From: Kees Cook Date: Thu, 8 Mar 2018 15:33:21 -0800 X-Google-Sender-Auth: 4C3RXdaFKeUsZm4CRJhEk3J5Ka4 Message-ID: Subject: Re: [PATCH 0/3] Remove accidental VLA usage To: Rasmus Villemoes Cc: Josh Poimboeuf , Andrew Morton , LKML , Jonathan Corbet , "Gustavo A. R. Silva" , Steven Rostedt , Chris Mason , Josef Bacik , David Sterba , "David S. Miller" , Alexey Kuznetsov , Hideaki YOSHIFUJI , Ingo Molnar , Peter Zijlstra , Thomas Gleixner , Masahiro Yamada , Borislav Petkov , Randy Dunlap , Ian Abbott , "Tobin C. Harding" , Sergey Senozhatsky , Petr Mladek , Andy Shevchenko , Pantelis Antoniou , Linux Btrfs , Network Development , Kernel Hardening Content-Type: text/plain; charset="UTF-8" X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: =?utf-8?q?1594338745857869070?= X-GMAIL-MSGID: =?utf-8?q?1594414355629409081?= X-Mailing-List: linux-kernel@vger.kernel.org List-ID: On Thu, Mar 8, 2018 at 2:12 PM, Rasmus Villemoes wrote: > On 8 March 2018 at 21:39, Kees Cook wrote: >> However, this works for me: >> >> #define __new_max(t1, t2, max1, max2, x, y) \ >> __builtin_choose_expr(__builtin_constant_p(x) && \ >> __builtin_constant_p(y) && \ >> __builtin_types_compatible_p(t1, t2), \ >> (t1)(x) > (t2)(y) ? (t1)(x) : (t2)(y), \ >> __max(t1, t2, max1, max2, x, y)) >> >> #define new_max(x, y) \ >> __new_max(typeof(x), typeof(y), \ >> __UNIQUE_ID(max1_), __UNIQUE_ID(max2_), \ >> x, y) > > Yes, that would seem to do the trick. > > Thinking out loud: do we really want or need the > __builtin_types_compatible condition when x and y are compile-time > constants? I think it would be nice to be able to use max(16, > sizeof(bla)) without having to cast either the literal or the sizeof. > Just omitting the type compatibility check might be dangerous, but > perhaps it could be relaxed to a check that both values are > representable in their common promoted type. Something like > > (type_signed(t1) == type_signed(t2)) || ((t1)x >= 0 && (t2)y >= 0) > > should be safe (if the types have same signedness, or the value of > signed type is positive), though it doesn't allow a few corner cases > (e.g. short vs. unsigned char is always ok due to promotion to int, > and also if the signed type is strictly wider than the unsigned type). I agree, it would be nice. However, I think it'd be better to continue to depend on max_t() for these kinds of cases though. For example: char foo[max_t(size_t, 6, sizeof(something))]; Works with the proposed patch. Also, I think this mismatch would already be triggering warnings, so we shouldn't have any currently. -Kees -- Kees Cook Pixel Security