From patchwork Thu Jan 12 19:51:23 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Linus Torvalds X-Patchwork-Id: 9514103 X-Patchwork-Delegate: herbert@gondor.apana.org.au 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 0ADCE601E5 for ; Thu, 12 Jan 2017 19:51:29 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E991828420 for ; Thu, 12 Jan 2017 19:51:28 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id DAE70286A5; Thu, 12 Jan 2017 19:51:28 +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=-6.3 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,RCVD_IN_SORBS_SPAM,T_DKIM_INVALID,T_TVD_MIME_EPI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 30E0B2866E for ; Thu, 12 Jan 2017 19:51:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750808AbdALTv0 (ORCPT ); Thu, 12 Jan 2017 14:51:26 -0500 Received: from mail-oi0-f67.google.com ([209.85.218.67]:33835 "EHLO mail-oi0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750733AbdALTvZ (ORCPT ); Thu, 12 Jan 2017 14:51:25 -0500 Received: by mail-oi0-f67.google.com with SMTP id 3so4291723oih.1; Thu, 12 Jan 2017 11:51:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=kiBL9w+pJq8eU0xtSEaIpfNtDUAid8OswdNft8aX3/w=; b=PxDaUnqeRVdlIiE53NfHKfANa6NmJwtKRNpVbF1LdqTqnXAr0ZlFPTcMa/zN5PYipO i+u1jqrKllwy54RvZBjU0ahudmyw5foB2ybAGScV7QOwatxm43X1NBrjTNCtO2NZf72Y u7rvGPZL64OIF7/kXJFOmgmTFOaFUd7WMhl0WN0G194zWMDwflo40HoKKp3b50EhnFKS D9j0NAoGXhPZhHpH89Y0qyRkuDrU3KEGvrZNZmdRqIrIzRiURP7AAr4XxLX5nu1TbW+3 wxHkVUvrIFxRIx5znk8F3LGEsoU//XT6GZKBtss3273Nv1csJ0j2oFmlX05wUE6n9Dw5 z3+Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=kiBL9w+pJq8eU0xtSEaIpfNtDUAid8OswdNft8aX3/w=; b=lvKWu+Lh2gSaJpeN/ZvEY0YjLzijlO94vHv1gmPhERHV/+CYz4Atpte26rOW/12QUD VZcJs44hAAW7943lYf+rsEpgEDAAnG7Czn8YeSEBp8Ya2okHgNhg/LPsIrY+tFuPqrRp qZl6JDjiyYB8dqL2n7kuEIUw8ohAA5q/a/PQuTzdbfr//NiWvHEllOI6X2A3DaUBGOsN u0DA63iRZ/IfD/dTVAUxsIfodd/GDRhFszaJb2MweVvz+LNFFd4DPMBQPgfDiRKuJi3v 48WubZo4Giz45LM4B+PEzysvHp88vo4jKrsjlHgOYAD6QbKMvY6UEj5on69YZJ25718T Qo1A== X-Gm-Message-State: AIkVDXL9JeYssY4oVV1CQ9LcIvkqWWq1IqRPUm3GvHxo+yKI5Pv6W30AWl9Hxf7rcFS9h+Cap+v6Ti2L9kbUDg== X-Received: by 10.202.232.76 with SMTP id f73mr1989980oih.60.1484250684860; Thu, 12 Jan 2017 11:51:24 -0800 (PST) MIME-Version: 1.0 Received: by 10.182.63.13 with HTTP; Thu, 12 Jan 2017 11:51:23 -0800 (PST) In-Reply-To: <20170112140215.rh247gwk55fjzmg7@treble> References: <20170110143340.GA3787@gondor.apana.org.au> <20170110143913.GA3822@gondor.apana.org.au> <20170111031124.GA4515@gondor.apana.org.au> <20170111043541.GA4944@gondor.apana.org.au> <20170112140215.rh247gwk55fjzmg7@treble> From: Linus Torvalds Date: Thu, 12 Jan 2017 11:51:23 -0800 X-Google-Sender-Auth: UfZxLm-Ls0faQy7U-op_ZpzRV9Y Message-ID: Subject: Re: x86-64: Maintain 16-byte stack alignment To: Josh Poimboeuf Cc: Andy Lutomirski , Herbert Xu , Linux Kernel Mailing List , Linux Crypto Mailing List , Ingo Molnar , Thomas Gleixner , Andy Lutomirski , Ard Biesheuvel Sender: linux-crypto-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-crypto@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, Jan 12, 2017 at 6:02 AM, Josh Poimboeuf wrote: > > Just to clarify, I think you're asking if, for versions of gcc which > don't support -mpreferred-stack-boundary=3, objtool can analyze all C > functions to ensure their stacks are 16-byte aligned. > > It's certainly possible, but I don't see how that solves the problem. > The stack will still be misaligned by entry code. Or am I missing > something? I think the argument is that we *could* try to align things, if we just had some tool that actually then verified that we aren't missing anything. I'm not entirely happy with checking the generated code, though, because as Ingo says, you have a 50:50 chance of just getting it right by mistake. So I'd much rather have some static tool that checks things at a code level (ie coccinelle or sparse). Almost totally untested "sparse" patch appended. The problem with sparse, obviously, is that few enough people run it, and it gives a lot of other warnings. But maybe Herbert can test whether this would actually have caught his situation, doing something like an allmodconfig build with "C=2" to force a sparse run on everything, and redirecting the warnings to stderr. But this patch does seem to give a warning for the patch that Herbert had, and that caused problems. And in fact it seems to find a few other possible problems (most, but not all, in crypto). This run was with the broken chacha20 patch applied, to verify that I get a warning for that case: arch/x86/crypto/chacha20_glue.c:70:13: warning: symbol 'state' has excessive alignment (16) arch/x86/crypto/aesni-intel_glue.c:724:12: warning: symbol 'iv' has excessive alignment (16) arch/x86/crypto/aesni-intel_glue.c:803:12: warning: symbol 'iv' has excessive alignment (16) crypto/shash.c:82:12: warning: symbol 'ubuf' has excessive alignment (16) crypto/shash.c:118:12: warning: symbol 'ubuf' has excessive alignment (16) drivers/char/hw_random/via-rng.c:89:14: warning: symbol 'buf' has excessive alignment (16) net/bridge/netfilter/ebtables.c:1809:31: warning: symbol 'tinfo' has excessive alignment (64) drivers/crypto/padlock-sha.c:85:14: warning: symbol 'buf' has excessive alignment (16) drivers/crypto/padlock-sha.c:147:14: warning: symbol 'buf' has excessive alignment (16) drivers/crypto/padlock-sha.c:304:12: warning: symbol 'buf' has excessive alignment (16) drivers/crypto/padlock-sha.c:388:12: warning: symbol 'buf' has excessive alignment (16) net/openvswitch/actions.c:797:33: warning: symbol 'ovs_rt' has excessive alignment (64) drivers/net/ethernet/neterion/vxge/vxge-config.c:1006:38: warning: symbol 'vpath' has excessive alignment (64) although I think at least some of these happen to be ok. There are a few places that clearly don't care about exact alignment, and use "__attribute__((aligned))" without any specific alignment value. It's just sparse that thinks that implies 16-byte alignment (it doesn't, really - it's unspecified, and is telling gcc to use "maximum useful alignment", so who knows _what_ gcc will assume). But some of them may well be real issues - if the alignment is about correctness rather than anything else. Anyway, the advantage of this kind of source-level check is that it should really catch things regardless of "luck" wrt alignment. Linus flow.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/flow.c b/flow.c index 7db9548..c876869 100644 --- a/flow.c +++ b/flow.c @@ -601,6 +601,20 @@ static void simplify_one_symbol(struct entrypoint *ep, struct symbol *sym) unsigned long mod; int all, stores, complex; + /* + * Warn about excessive local variable alignment. + * + * This needs to be linked up with some flag to enable + * it, and specify the alignment. The 'max_int_alignment' + * just happens to be what we want for the kernel for x86-64. + */ + mod = sym->ctype.modifiers; + if (!(mod & (MOD_NONLOCAL | MOD_STATIC))) { + unsigned int alignment = sym->ctype.alignment; + if (alignment > max_int_alignment) + warning(sym->pos, "symbol '%s' has excessive alignment (%u)", show_ident(sym->ident), alignment); + } + /* Never used as a symbol? */ pseudo = sym->pseudo; if (!pseudo)