From patchwork Mon Sep 17 09:49:04 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 184357 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id B21742C007D for ; Mon, 17 Sep 2012 19:49:42 +1000 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1348480183; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Message-ID:Date:From:User-Agent:MIME-Version:To:CC:Subject: References:In-Reply-To:Content-Type:Mailing-List:Precedence: List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender: Delivered-To; bh=chx+EYtlpvQCb1sgsZBpxdFy90I=; b=Rlvasju6H4I0RtZ BFMrF9WQx0enr9TtMwA4Rp6tZ078rut6ZnJwsm2tEzbBpY2UDYk16nYx5PydrU/g MA4SI4YATgFwGntBacGabOXEFYRML8LKID5n4rvO1x1BdxTNJp4sAjzOjsTswY95 BO2Yuyubt7DyS5oec8eDfFQwUg3Q= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:Message-ID:Date:From:User-Agent:MIME-Version:To:CC:Subject:References:In-Reply-To:Content-Type:X-IsSubscribed:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=LOwCy0+lR0vsaBixYj51/zAVxI47j/phuu0qvXaeztLeRBc8zGTV4lssvu9Wnx hEV/JsvJQgz6333pOVWZcw7SWGJS6BFPhwGrfZ51uza5FEbjbQqZs+Ifc7YawQzK 5rjQ1FXLg/YWuXNEVzg0DNWdcEv8T658IMsUkVmjIFVe4=; Received: (qmail 21901 invoked by alias); 17 Sep 2012 09:49:30 -0000 Received: (qmail 21754 invoked by uid 22791); 17 Sep 2012 09:49:27 -0000 X-SWARE-Spam-Status: No, hits=-8.0 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_UNTRUST, KHOP_THREADED, RCVD_IN_DNSWL_HI, RCVD_IN_HOSTKARMA_W, RP_MATCHES_RCVD, SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 17 Sep 2012 09:49:06 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q8H9n5tk009120 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 17 Sep 2012 05:49:06 -0400 Received: from fweimer.str.redhat.com (dhcp-5-241.str.redhat.com [10.32.5.241]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id q8H9n4PY009169 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 17 Sep 2012 05:49:05 -0400 Message-ID: <5056F210.7060404@redhat.com> Date: Mon, 17 Sep 2012 11:49:04 +0200 From: Florian Weimer User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120828 Thunderbird/15.0 MIME-Version: 1.0 To: Ian Lance Taylor CC: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] PR other/54411: libiberty: objalloc_alloc integer overflows (CVE-2012-3509) References: <876281zjr0.fsf@mid.deneb.enyo.de> In-Reply-To: X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org On 09/05/2012 07:31 AM, Ian Lance Taylor wrote: > On Wed, Aug 29, 2012 at 10:32 AM, Florian Weimer wrote: >> >> This patches fixes an integer overflow in libiberty, which leads to >> crashes in binutils. The long version of the objalloc_alloc macro >> would have needed another conditional, so I removed that and replaced >> it with a call to the actual implementation. > > I guess I don't see why removing the macro is desirable. In many uses > of objalloc_alloc the conditional can be optimized out anyhow. It's > true that it can't always be, but, so what? The macro is probably > still a win. Fair enough. I've added a wraparound check to the macro. Okay for trunk? 2012-09-17 Florian Weimer PR other/54411 * objalloc.h (objalloc_alloc): Do not use fast path on wraparound. 2012-09-17 Florian Weimer PR other/54411 * objalloc.c (_objalloc_alloc): Add overflow check covering alignment and CHUNK_HEADER_SIZE addition. Index: libiberty/objalloc.c =================================================================== --- libiberty/objalloc.c (revision 191378) +++ libiberty/objalloc.c (working copy) @@ -1,5 +1,5 @@ /* objalloc.c -- routines to allocate memory for objects - Copyright 1997 Free Software Foundation, Inc. + Copyright 1997-2012 Free Software Foundation, Inc. Written by Ian Lance Taylor, Cygnus Solutions. This program is free software; you can redistribute it and/or modify it @@ -112,8 +112,9 @@ /* Allocate space from an objalloc structure. */ PTR -_objalloc_alloc (struct objalloc *o, unsigned long len) +_objalloc_alloc (struct objalloc *o, unsigned long original_len) { + unsigned long len = original_len; /* We avoid confusion from zero sized objects by always allocating at least 1 byte. */ if (len == 0) @@ -121,6 +122,11 @@ len = (len + OBJALLOC_ALIGN - 1) &~ (OBJALLOC_ALIGN - 1); + /* Check for overflow in the alignment operation above and the + malloc argument below. */ + if (len + CHUNK_HEADER_SIZE < original_len) + return NULL; + if (len <= o->current_space) { o->current_ptr += len; Index: include/objalloc.h =================================================================== --- include/objalloc.h (revision 191378) +++ include/objalloc.h (working copy) @@ -1,5 +1,5 @@ /* objalloc.h -- routines to allocate memory for objects - Copyright 1997, 2001 Free Software Foundation, Inc. + Copyright 1997-2012 Free Software Foundation, Inc. Written by Ian Lance Taylor, Cygnus Solutions. This program is free software; you can redistribute it and/or modify it @@ -91,7 +91,7 @@ if (__len == 0) \ __len = 1; \ __len = (__len + OBJALLOC_ALIGN - 1) &~ (OBJALLOC_ALIGN - 1); \ - (__len <= __o->current_space \ + (__len && __len <= __o->current_space \ ? (__o->current_ptr += __len, \ __o->current_space -= __len, \ (void *) (__o->current_ptr - __len)) \