From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 91393 invoked by alias); 24 Jul 2018 14:50:27 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 91382 invoked by uid 89); 24 Jul 2018 14:50:27 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_PASS autolearn=ham version=3.3.2 spammy=bases X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 24 Jul 2018 14:50:26 +0000 Received: from relay1.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id D23E8ADBB; Tue, 24 Jul 2018 14:50:23 +0000 (UTC) Date: Tue, 24 Jul 2018 14:50:00 -0000 From: Richard Biener To: Bernd Edlinger cc: GCC Patches , Jeff Law , Jakub Jelinek Subject: Re: [PATCH] Make strlen range computations more conservative In-Reply-To: Message-ID: References: User-Agent: Alpine 2.20 (LSU 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-SW-Source: 2018-07/txt/msg01390.txt.bz2 On Tue, 24 Jul 2018, Bernd Edlinger wrote: > Hi! > > This patch makes strlen range computations more conservative. > > Firstly if there is a visible type cast from type A to B before passing > then value to strlen, don't expect the type layout of B to restrict the > possible return value range of strlen. > > Furthermore use the outermost enclosing array instead of the > innermost one, because too aggressive optimization will likely > convert harmless errors into security-relevant errors, because > as the existing test cases demonstrate, this optimization is actively > attacking string length checks in user code, while and not giving > any warnings. > > > > Bootstrapped and reg-tested on x86_64-pc-linux-gnu. > Is it OK for trunk? I'd like us to be explicit in what we support, not what we do not support, thus + /* Avoid arrays of pointers. */ + if (TREE_CODE (TREE_TYPE (arg)) == POINTER_TYPE) + return false; should become /* We handle arrays of integer types. */ if (TREE_CODE (TRE_TYPE (arg)) != INTEGER_TYPE) return false; + tree base = arg; + while (TREE_CODE (base) == ARRAY_REF + || TREE_CODE (base) == ARRAY_RANGE_REF + || TREE_CODE (base) == COMPONENT_REF) + base = TREE_OPERAND (base, 0); + + /* If this looks like a type cast don't assume anything. */ + if ((TREE_CODE (base) == MEM_REF + && (! integer_zerop (TREE_OPERAND (base, 1)) + || TREE_TYPE (TREE_TYPE (TREE_OPERAND (base, 0))) + != TREE_TYPE (base))) + || TREE_CODE (base) == VIEW_CONVERT_EXPR) return false; likewise - you miss to handle BIT_FIELD_REF. So, instead if (!(DECL_P (base) || TREE_CODE (base) == STRING_CST || (TREE_CODE (base) == MEM_REF && ... you should look at comparing TYPE_MAIN_VARIANT in your type check, aligned/unaligned or const/non-const accesses shouldn't be considered a "type cast". Maybe even use types_compatible_p. Not sure why you enforce zero-offset MEMs? Do we in the end only handle &decl bases of MEMs? Given you strip arbitrary COMPONENT_REFs the offset in a MEM isn't so much different? It looks like the component-ref stripping plus type-check part could be factored out into sth like get_base_address? I don't have a good name or suggested semantics for it though. Richard. > > Thanks > Bernd. -- Richard Biener SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)