From patchwork Tue Apr 28 08:22:06 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Wielaard X-Patchwork-Id: 6473 Received: (qmail 66200 invoked by alias); 28 Apr 2015 08:22:14 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 65373 invoked by uid 89); 28 Apr 2015 08:22:14 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.8 required=5.0 tests=AWL, BAYES_00, KAM_LAZY_DOMAIN_SECURITY, SPF_HELO_PASS, T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Message-ID: <1430209326.1938.141.camel@bordewijk.wildebeest.org> Subject: Re: [PATCH] elf.h SHF_EXCLUDE signed int 31 bit shift triggers undefined behaviour. From: Mark Wielaard To: Florian Weimer Cc: Szabolcs Nagy , "libc-alpha@sourceware.org" , Josh Stone Date: Tue, 28 Apr 2015 10:22:06 +0200 In-Reply-To: <55376772.9000207@redhat.com> References: <1427193579-26102-1-git-send-email-mjw@redhat.com> <55117118.1080706@arm.com> <20150324211541.GA2318@blokker.redhat.com> <1429608058.1938.53.camel@bordewijk.wildebeest.org> <5537587D.5020707@redhat.com> <55376772.9000207@redhat.com> Mime-Version: 1.0 On Wed, 2015-04-22 at 11:18 +0200, Florian Weimer wrote: > On 04/22/2015 10:14 AM, Florian Weimer wrote: > > On 04/21/2015 11:20 AM, Mark Wielaard wrote: > >> -#define SHF_EXCLUDE (1 << 31) /* Section is excluded unless > >> +#define SHF_EXCLUDE (1U << 31) /* Section is excluded unless > > > > I think the safer change is to use -0x80000000 as the value of the > > constant, without making it unsigned. Otherwise my previous objections > > apply. > > I thought some more about this, and have changed my opinion completely. > Making the constant unsigned is less risky than making it negative > because of potential sign extension issues. It's the lesser of two evils. > > The proposed patch is okay with me. Thanks. I didn't see other objections. So if it is good to go in could someone please push it for me? (I don't have glibc git push access.) ChangeLog * elf/elf.h (SHF_EXCLUDE): Use unsigned 1 for shift. From 86771e8963653c306e53c07e1640914081afb30a Mon Sep 17 00:00:00 2001 From: Mark Wielaard Date: Tue, 24 Mar 2015 11:32:36 +0100 Subject: [PATCH] elf.h SHF_EXCLUDE signed int 31 bit shift triggers undefined behaviour. Any use of SHF_EXCLUDE in code that tries to check it against sh_flags will trigger undefined behaviour because it is defined as a 31 bit shift against an signed integer. Fix by explicitly using an unsigned int. --- ChangeLog | 4 ++++ elf/elf.h | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/elf/elf.h b/elf/elf.h index 71492a2..39bafc2 100644 --- a/elf/elf.h +++ b/elf/elf.h @@ -371,7 +371,7 @@ typedef struct #define SHF_MASKPROC 0xf0000000 /* Processor-specific */ #define SHF_ORDERED (1 << 30) /* Special ordering requirement (Solaris). */ -#define SHF_EXCLUDE (1 << 31) /* Section is excluded unless +#define SHF_EXCLUDE (1U << 31) /* Section is excluded unless referenced or allocated (Solaris).*/ /* Section group handling. */ -- 1.8.3.1