From dan@kulesh.obluda.cz Fri Dec 18 16:40:57 2009 Return-Path: Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 1BC2B106566B for ; Fri, 18 Dec 2009 16:40:57 +0000 (UTC) (envelope-from dan@kulesh.obluda.cz) Received: from kulesh.obluda.cz (kgw.obluda.cz [193.179.199.50]) by mx1.freebsd.org (Postfix) with ESMTP id 94A2C8FC0A for ; Fri, 18 Dec 2009 16:40:55 +0000 (UTC) Received: from kulesh.obluda.cz (localhost. [127.0.0.1]) by kulesh.obluda.cz (8.14.3/8.14.3) with ESMTP id nBIGersQ001356 for ; Fri, 18 Dec 2009 17:40:53 +0100 (CET) (envelope-from dan@kulesh.obluda.cz) Received: (from root@localhost) by kulesh.obluda.cz (8.14.3/8.14.3/Submit) id nBIGerCW001355; Fri, 18 Dec 2009 17:40:53 +0100 (CET) (envelope-from dan) Message-Id: <200912181640.nBIGerCW001355@kulesh.obluda.cz> Date: Fri, 18 Dec 2009 17:40:53 +0100 (CET) From: Dan Lukes Reply-To: Dan Lukes To: FreeBSD-gnats-submit@freebsd.org Cc: Subject: double-free in reallocf() X-Send-Pr-Version: 3.113 X-GNATS-Notify: >Number: 141753 >Category: bin >Synopsis: [libc] [patch] double-free in reallocf(3) >Confidential: no >Severity: non-critical >Priority: medium >Responsible: jh >State: closed >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Fri Dec 18 16:50:03 UTC 2009 >Closed-Date: Sat Aug 28 06:47:37 UTC 2010 >Last-Modified: Sat Aug 28 06:47:37 UTC 2010 >Originator: Dan Lukes >Release: FreeBSD 7.2-STABLE i386 >Organization: Obludarium >Environment: System: FreeBSD 7.2-STABLE i386 lib/libc/stdlib/reallocf.c,v 1.4 2002/03/22 21:53:10 ******** SYS V malloc() compatifility (malloc option 'V' in effect) >Description: Imagine the code: ----------------- _malloc_options = "V"; ... ptr=malloc(5); ... nptr=reallocf(ptr,0); ----------------- Now look into libc's reallocf() implementation: void * reallocf(void *ptr, size_t size) { void *nptr; nptr = realloc(ptr, size); if (!nptr && ptr) free(ptr); return (nptr); } The realloc() is called with non-NULL ptr. Zero-size realloc never fail, so ptr is freed by realloc. nptr is NULL because of size=0 and option V Unfortunatelly, it mean the free(ptr) is called again causing double-free of ptr. It never fail (allocation of >How-To-Repeat: See code in description. >Fix: The free must not be called when size=0 and opt_sysv == true because the pointer is already freed. Unfortunatelly the opt_sysv variable is not avaiable here, it is static variable within malloc.c It sounds to me that better solution is to move reallocf() implementation from reallocf.c to malloc.c (opt_sysv is avaiable here) but there may be other solution. >Release-Note: >Audit-Trail: From: Dan Lukes To: bug-followup@FreeBSD.org Cc: Subject: Re: bin/141753: double-free in reallocf() Date: Fri, 18 Dec 2009 18:12:00 +0100 I missed simple and obvious fix! Replace > if (!nptr && ptr) > free(ptr); with > if (!nptr && ptr && size > 0) > free(ptr); Because - When size>0 the previous behavior is maintained. When size==0 and V option not set, then nptr!=NULL (realloc(,0) never fail) so the ptr is not free - correct behavior. When size==0 and V option IS set then we know the ptr is freed by realloc and it's not freed again here - correct behavior. Dan Responsible-Changed-From-To: freebsd-bugs->jh Responsible-Changed-By: jh Responsible-Changed-When: Sun Feb 28 14:36:44 UTC 2010 Responsible-Changed-Why: Take. http://www.freebsd.org/cgi/query-pr.cgi?pr=141753 From: Jaakko Heinonen To: Dan Lukes , bug-followup@FreeBSD.org Cc: Subject: Re: bin/141753: double-free in reallocf() Date: Tue, 2 Mar 2010 16:06:58 +0200 On 2009-12-18, Dan Lukes wrote: > > if (!nptr && ptr) > > free(ptr); > > with > > > if (!nptr && ptr && size > 0) > > free(ptr); > > Because - > > When size>0 the previous behavior is maintained. > > When size==0 and V option not set, then nptr!=NULL (realloc(,0) never > fail) so the ptr is not free - correct behavior. This is not actually true. When the V flag is not set realloc(ptr, 0) can fail if ptr == NULL. (It's equivalent to malloc(1).) It is not a problem however because there's no need to call free() on NULL pointer. > When size==0 and V option IS set then we know the ptr is freed by > realloc and it's not freed again here - correct behavior. -- Jaakko From: dfilter@FreeBSD.ORG (dfilter service) To: bug-followup@FreeBSD.org Cc: Subject: Re: bin/141753: commit references a PR Date: Wed, 3 Mar 2010 15:43:36 +0000 (UTC) Author: jh Date: Wed Mar 3 15:43:26 2010 New Revision: 204636 URL: http://svn.freebsd.org/changeset/base/204636 Log: In reallocf(3), free the memory only when size != 0. Otherwise, when the System V compatibility option (malloc "V" flag) is in effect a zero sized reallocf() could cause a double free. PR: bin/141753 Submitted by: Dan Lukes Modified: head/lib/libc/stdlib/reallocf.c Modified: head/lib/libc/stdlib/reallocf.c ============================================================================== --- head/lib/libc/stdlib/reallocf.c Wed Mar 3 15:05:58 2010 (r204635) +++ head/lib/libc/stdlib/reallocf.c Wed Mar 3 15:43:26 2010 (r204636) @@ -35,7 +35,14 @@ reallocf(void *ptr, size_t size) void *nptr; nptr = realloc(ptr, size); - if (!nptr && ptr) + + /* + * When the System V compatibility option (malloc "V" flag) is + * in effect, realloc(ptr, 0) frees the memory and returns NULL. + * So, to avoid double free, call free() only when size != 0. + * realloc(ptr, 0) can't fail when ptr != NULL. + */ + if (!nptr && ptr && size != 0) free(ptr); return (nptr); } _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org" State-Changed-From-To: open->patched State-Changed-By: jh State-Changed-When: Wed Mar 3 16:07:17 UTC 2010 State-Changed-Why: Patched in head (r204636). Thanks! http://www.freebsd.org/cgi/query-pr.cgi?pr=141753 From: dfilter@FreeBSD.ORG (dfilter service) To: bug-followup@FreeBSD.org Cc: Subject: Re: bin/141753: commit references a PR Date: Mon, 2 Aug 2010 09:13:22 +0000 (UTC) Author: jh Date: Mon Aug 2 09:13:09 2010 New Revision: 210745 URL: http://svn.freebsd.org/changeset/base/210745 Log: MFC r204636: In reallocf(3), free the memory only when size != 0. Otherwise, when the System V compatibility option (malloc "V" flag) is in effect a zero sized reallocf() could cause a double free. PR: bin/141753 Modified: stable/8/lib/libc/stdlib/reallocf.c Directory Properties: stable/8/lib/libc/ (props changed) stable/8/lib/libc/stdtime/ (props changed) stable/8/lib/libc/sys/ (props changed) Modified: stable/8/lib/libc/stdlib/reallocf.c ============================================================================== --- stable/8/lib/libc/stdlib/reallocf.c Mon Aug 2 09:03:31 2010 (r210744) +++ stable/8/lib/libc/stdlib/reallocf.c Mon Aug 2 09:13:09 2010 (r210745) @@ -35,7 +35,14 @@ reallocf(void *ptr, size_t size) void *nptr; nptr = realloc(ptr, size); - if (!nptr && ptr) + + /* + * When the System V compatibility option (malloc "V" flag) is + * in effect, realloc(ptr, 0) frees the memory and returns NULL. + * So, to avoid double free, call free() only when size != 0. + * realloc(ptr, 0) can't fail when ptr != NULL. + */ + if (!nptr && ptr && size != 0) free(ptr); return (nptr); } _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org" From: dfilter@FreeBSD.ORG (dfilter service) To: bug-followup@FreeBSD.org Cc: Subject: Re: bin/141753: commit references a PR Date: Sat, 28 Aug 2010 06:33:20 +0000 (UTC) Author: jh Date: Sat Aug 28 06:33:01 2010 New Revision: 211919 URL: http://svn.freebsd.org/changeset/base/211919 Log: MFC r204636: In reallocf(3), free the memory only when size != 0. Otherwise, when the System V compatibility option (malloc "V" flag) is in effect a zero sized reallocf() could cause a double free. PR: bin/141753 Modified: stable/7/lib/libc/stdlib/reallocf.c Directory Properties: stable/7/lib/libc/ (props changed) stable/7/lib/libc/stdtime/ (props changed) Modified: stable/7/lib/libc/stdlib/reallocf.c ============================================================================== --- stable/7/lib/libc/stdlib/reallocf.c Sat Aug 28 06:31:15 2010 (r211918) +++ stable/7/lib/libc/stdlib/reallocf.c Sat Aug 28 06:33:01 2010 (r211919) @@ -35,7 +35,14 @@ reallocf(void *ptr, size_t size) void *nptr; nptr = realloc(ptr, size); - if (!nptr && ptr) + + /* + * When the System V compatibility option (malloc "V" flag) is + * in effect, realloc(ptr, 0) frees the memory and returns NULL. + * So, to avoid double free, call free() only when size != 0. + * realloc(ptr, 0) can't fail when ptr != NULL. + */ + if (!nptr && ptr && size != 0) free(ptr); return (nptr); } _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org" State-Changed-From-To: patched->closed State-Changed-By: jh State-Changed-When: Sat Aug 28 06:47:35 UTC 2010 State-Changed-Why: Fixed in head, stable/8 and stable/7. Thanks! http://www.freebsd.org/cgi/query-pr.cgi?pr=141753 >Unformatted: