From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf1-x431.google.com (mail-pf1-x431.google.com [IPv6:2607:f8b0:4864:20::431]) by sourceware.org (Postfix) with ESMTPS id 43C94385840A for ; Fri, 29 Jul 2022 05:44:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 43C94385840A Received: by mail-pf1-x431.google.com with SMTP id o12so3745915pfp.5 for ; Thu, 28 Jul 2022 22:44:36 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=zhot/WG9aJcWx/YffBfRpXnsCyFg4PPviua3JbRyasM=; b=i+XUEt4M1m+sb9HYEKrCJ9tFUbirT6mT8ZwOvLJW7nrlspjBI3FJp20rM/LYVFMu7n bTRhlxmYwR03g+4AYHJc+uZBSTxUe6ef0qfH2ij6xMPOVlLqVUYLWexfoSr5K0JhOX5R lca/0F1wND23mlfmhjfwGU/WKzn1PP7U/X6o75oEf5yrxB0PPL7rXaOp32Xs0XIiP1WL Fn8Hit/3960QT7kYJPIuiJv+l4Js8DWCFdD0FYfMswuuFNS6piljC69FAY9KkmoWcVzA c7uBsLey8zO4LeTHznMnyh9Lju3gPXxtesGwvZOfn1+kbJwI1R8CN8R+Q6jKfH0bQ46X y5uw== X-Gm-Message-State: AJIora9eHAMSlR/atKrBzZGkz7l5K0BL/U2WT1PAHflsrWF6npdusx/X 7IvZmIlHG+eaVKwwZHzQOceBFw== X-Google-Smtp-Source: AGRyM1s2biWHbYCu5G6BwwRGWlnRrn9XpvLayvxiDISxWN1zV2qXosZUSl1s+vn+I4hrIJ5oHLyQKw== X-Received: by 2002:a63:e4f:0:b0:41a:9472:eca0 with SMTP id 15-20020a630e4f000000b0041a9472eca0mr1665599pgo.623.1659073473938; Thu, 28 Jul 2022 22:44:33 -0700 (PDT) Received: from www.outflux.net (smtp.outflux.net. [198.145.64.163]) by smtp.gmail.com with ESMTPSA id z20-20020a170903409400b0016a33177d3csm2407119plc.160.2022.07.28.22.44.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 28 Jul 2022 22:44:33 -0700 (PDT) Date: Thu, 28 Jul 2022 22:44:32 -0700 From: Kees Cook To: Richard Biener Cc: Qing Zhao , gcc-patches Paul A Clarke via , jakub Jelinek , martin Sebor , "joseph@codesourcery.com" Subject: Re: [GCC13][Patch][V2][1/2]Add a new option -fstrict-flex-array[=n] and attribute strict_flex_array(n) and use it in PR101836 Message-ID: <202207282202.88A5A9E8@keescook> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Spam-Status: No, score=-3.0 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, JMQ_SPF_NEUTRAL, KAM_NUMSUBJECT, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 Jul 2022 05:44:38 -0000 On Thu, Jul 28, 2022 at 07:26:57AM +0000, Richard Biener wrote: > On Tue, 19 Jul 2022, Qing Zhao wrote: > > [...] > > +@cindex @code{strict_flex_array} variable attribute > > +@item strict_flex_array (@var{level}) > > +The @code{strict_flex_array} attribute should be attached to the trailing > > +array field of a structure. It specifies the level of strictness of > > +treating the trailing array field of a structure as a flexible array > > +member. @var{level} must be an integer betwen 0 to 3. > > + > > +@var{level}=0 is the least strict level, all trailing arrays of structures > > +are treated as flexible array members. @var{level}=3 is the strictest level, > > +only when the trailing array is declared as a flexible array member per C99 > > +standard onwards ([]), it is treated as a flexible array member. > > How is level 3 (thus -fstrict-flex-array) interpreted when you specify > -std=c89? How for -std=gnu89? To me, it makes sense that either c99 is required (most sane to me) or it would disable flexible arrays entirely (seems an unlikely combo to be useful). > > > + > > +There are two more levels in between 0 and 3, which are provided to support > > +older codes that use GCC zero-length array extension ([0]) or one-size array > > +as flexible array member ([1]): > > +When @var{level} is 1, the trailing array is treated as a flexible array member > > +when it is declared as either "[]", "[0]", or "[1]"; > > +When @var{level} is 2, the trailing array is treated as a flexible array member > > +when it is declared as either "[]", or "[0]". > > Given the above does adding level 2 make sense given that [0] is a GNU > extension? Level 1 removes the general "all trailing arrays are flex arrays" logic, but allows the 2 common "historical" fake flex array styles ("[1]" and "[0]"). Level 2 additionally removes the "[1]" style. Level 3 additionally removes the "[0]" style. I don't understand how "[0]" being a GNU extension matters here for level 2 -- it's dropping "[1]". And for level 3, the point is to defang the GNU extension of "[0]" to no longer mean "flexible array", and instead only mean "zero sized member" (as if it were something like "struct { } no_size;"). Note that for the Linux kernel, we only care about level 3, but could make do with level 2. We need to purge all the "fake" flexible array usage so we can start building a sane set of behaviors around array bounds that are reliably introspectable. As a related bit of feature creep, it would be great to expose something like __builtin_has_flex_array_p() so FORTIFY could do a better job filtering __builtin_object_size() information. Given: struct inside { int foo; int bar; unsigned long items[]; }; struct outside { int a; int b; struct inside inner; }; The follow properties are seen within, for example: void stuff(struct outside *outer, struct inside *inner) { ... } __builtin_object_size(&outer->inner, 1) == 8 __builtin_object_size(inner, 1) == -1 (see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101832) So things like FORTIFY misfire on &outer->inner, as it's _not_ actually 8 bytes -- it has a potential trailing flex array. If it could be introspected better, FORTIFY could check for the flex array. For example, instead of using the inconsistent __bos(ptr, 1) for finding member sizes, it could do something like: #define __member_size(ptr) \ (__builtin_has_flex_array_p(ptr) ? -1 : \ __builtin_object_size(ptr, 1)) -- Kees Cook