Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757080AbcCCMn2 (ORCPT ); Thu, 3 Mar 2016 07:43:28 -0500 Received: from mail-wm0-f49.google.com ([74.125.82.49]:33675 "EHLO mail-wm0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756852AbcCCMnY (ORCPT ); Thu, 3 Mar 2016 07:43:24 -0500 Date: Thu, 3 Mar 2016 13:43:19 +0100 From: Ingo Molnar To: =?iso-8859-1?Q?M=E5ns_Rullg=E5rd?= Cc: Arnaldo Carvalho de Melo , Peter Zijlstra , Colin King , Ingo Molnar , linux-kernel@vger.kernel.org, Richard Henderson , Jakub Jelinek , Dan Carpenter Subject: Re: Q: why didn't GCC warn about this uninitialized variable? Message-ID: <20160303124319.GA8781@gmail.com> References: <1456923322-29697-1-git-send-email-colin.king@canonical.com> <20160302125901.GF6356@twins.programming.kicks-ass.net> <20160302130350.GO3604@kernel.org> <20160302132127.GG6356@twins.programming.kicks-ass.net> <20160302132323.GP3604@kernel.org> <20160303121944.GB2484@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2834 Lines: 65 * M?ns Rullg?rd wrote: > > So the source of the bug was: > > > > struct sigaction sa; > > > > ... > > > > sigfillset(&sa.sa_mask); > > sa.sa_sigaction = segfault_handler; > > sigaction(SIGSEGV, &sa, NULL); > > > > ... which uninitialized sa.sa_flags field GCC merrily accepted as > > proper C code, despite us turning on essentially _all_ GCC warnings > > for the perf build that exist under the sun: > > > > gcc -Wbad-function-cast -Wdeclaration-after-statement -Wformat-security -Wformat-y2k \ > > -Winit-self -Wmissing-declarations -Wmissing-prototypes -Wnested-externs \ > > -Wno-system-headers -Wold-style-definition -Wpacked -Wredundant-decls \ > > -Wshadow -Wstrict-aliasing=3 -Wstrict-prototypes -Wswitch-default -Wswitch-enum \ > > -Wundef -Wwrite-strings -Wformat \ > > -Werror -O6 -fno-omit-frame-pointer -ggdb3 -funwind-tables -Wall -Wextra -std=gnu99 -fstack-protector-all -D_FORTIFY_SOURCE=2 > > > > This is a _trivial_ uninitialized variable bug, yet GCC never warned > > about it. Why? > > > > People build perf with a wide range of GCC versions, from old ones to > > trunk. I cannot believe it that none of those GCC versions warned > > about this trivial looking bug! > > > > And yes, I know that unitialized structures on the stack are valid C > > code, yet it's one of the most fragile aspects of C and it was the > > source of countless security holes in the past... > > Passing a pointer to an uninitialised object is typically not warned about since > the purpose of the call might be to initialise it in the first place. Now the > second argument of sigaction() is a pointer to const, so the compiler should be > able to see that this isn't the case. > > Maybe it's not warning because some fields in the struct are initialised and the > function, as far as the compiler knows, might only be accessing those. (There's > certainly code out there using that pattern.) If this is the case here, a flag > to warn unless the object is fully initialised would be useful to catch bugs > like this. So it would be absolutely fantastic if one of these solutions existed on GCC: - emit a warning if a structure is passed around uninitialized. A new GCC __attribute__((struct_fully_initialized)) could be used to annotate extern function arguments which fully initialize input arguments. (I'd personally migrate both tools/perf and kernel side code to use it, module by module.) - or memset() to zero all on-stack structures that GCC cannot prove are initialized fully. The first solution takes extra work on the source level, the latter takes extra runtime profiling to find where the extra memset()s matter to performance. Any of these would be fantastic tools for C robustness and security. Thanks, Ingo