From patchwork Sun May 18 21:43:05 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?R=C3=BCdiger_Sonderfeld?= X-Patchwork-Id: 1005 Return-Path: X-Original-To: siddhesh@wilcox.dreamhost.com Delivered-To: siddhesh@wilcox.dreamhost.com Received: from homiemail-mx23.g.dreamhost.com (mx2.sub5.homie.mail.dreamhost.com [208.113.200.128]) by wilcox.dreamhost.com (Postfix) with ESMTP id 03C67360098 for ; Sun, 18 May 2014 14:43:23 -0700 (PDT) Received: by homiemail-mx23.g.dreamhost.com (Postfix, from userid 14307373) id 9507263D61ADE; Sun, 18 May 2014 14:43:23 -0700 (PDT) X-Original-To: glibc@patchwork.siddhesh.in Delivered-To: x14307373@homiemail-mx23.g.dreamhost.com Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by homiemail-mx23.g.dreamhost.com (Postfix) with ESMTPS id 7671863D61AB4 for ; Sun, 18 May 2014 14:43:23 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding:content-type; q=dns; s=default; b=oLAKxHLkKNClXiEY3yyI1bj2wRqPJityMvw/HQf67Nr EY1n+B5zG2iPzgK/p4HmkYdvk8fAuomwW/E/fyoXxCbIBmirS5HQ5Bl1v998d/05 2lmWzteu0a1KYP/+rv7G0rWH9epr7RfFtz0Oz99/VgHb4CvBigc3xXqhGT1fIJ3w = DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding:content-type; s=default; bh=YVUB/LBIUQJdScrRsOiyj+C4hE0=; b=NJWOR5G0yg8AVlgSr kecW/hH0XYELqv1UGk/SAv2ZbP9lc8CquqF/vB+eLPbumhKI0MCo46LelohWB3Hq 7Wwm+jlFFW6IIZU7EcHw0F4rZuwrCzw/QHH/As0xa4Mrma3ZbZnDbO9qvpoOm8G9 Ar2ZWOc13zreQ0SJaVgnvuLXoo= Received: (qmail 25705 invoked by alias); 18 May 2014 21:43:19 -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 25681 invoked by uid 89); 18 May 2014 21:43:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS autolearn=ham version=3.3.2 X-HELO: ptmx.org From: =?ISO-8859-1?Q?R=FCdiger?= Sonderfeld To: Paul Eggert Cc: libc-alpha@sourceware.org Subject: [RFC][PATCH v2] Add reallocarray function. Date: Sun, 18 May 2014 23:43:05 +0200 Message-ID: <2080621.6fAB4UMNoY@descartes> User-Agent: KMail/4.13 (Linux/3.13.0-24-generic; KDE/4.13.0; x86_64; ; ) In-Reply-To: <5379208F.8030000@cs.ucla.edu> References: <5379208F.8030000@cs.ucla.edu> MIME-Version: 1.0 X-DH-Original-To: glibc@patchwork.siddhesh.in Hello Paul, > Instead of cut and pasting this code from calloc, please refactor so > that the code is present only once, with the goal of optimizing it when > the GCC folks get their act together and have a function like the > __builtin_umul_overflow function that Clang has had since January. This > will let calloc and reallocarray do the unsigned multiplication and > inspect the hardware's overflow bit directly, which is nicer than the > above hackery. thanks for your feedback. I've changed the patch to add a `check_mul_overflow' function. Regards, Rüdiger ---- 8< ------------------------------------------------------ >8 ---- The reallocarray function is an extension from OpenBSD. It is an integer-overflow-safe replacement for realloc(p, X*Y) and malloc(X*Y) (realloc(NULL, X*Y)). It can therefore help in preventing certain security issues in code. See http://www.openbsd.org/cgi-bin/man.cgi?query=reallocarray&sektion=3&manpath=OpenBSD+Current --- ChangeLog | 11 ++++ malloc/Makefile | 2 +- malloc/Versions | 4 ++ malloc/malloc.c | 45 ++++++++++--- malloc/malloc.h | 8 +++ malloc/tst-reallocarray.c | 160 ++++++++++++++++++++++++++++++++++++++++++++++ stdlib/stdlib.h | 7 ++ 7 files changed, 226 insertions(+), 11 deletions(-) create mode 100644 malloc/tst-reallocarray.c diff --git a/ChangeLog b/ChangeLog index c606b0d..1e142e1 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,14 @@ +2014-05-18 Rüdiger Sonderfeld + + * malloc/Versions: Add reallocarray and __libc_rallocarray. + * malloc/Makefile (tests): Add tst-reallocarray.c. + * malloc/tst-reallocarray.c: New test file. + * malloc/malloc.h (reallocarray): New declaration. + * stdlib/stdlib.h (reallocarray): Likewise. + * malloc/malloc.c (check_mul_overflow): New inline function. + (__libc_reallocarray): New function. + (__libc_calloc): Use `check_mul_overflow'. + 2014-05-17 Jose E. Marchesi [BZ #16958] diff --git a/malloc/Makefile b/malloc/Makefile index 7a716f9..1b415ae 100644 --- a/malloc/Makefile +++ b/malloc/Makefile @@ -26,7 +26,7 @@ dist-headers := malloc.h headers := $(dist-headers) obstack.h mcheck.h tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \ tst-mallocstate tst-mcheck tst-mallocfork tst-trim1 \ - tst-malloc-usable tst-realloc tst-posix_memalign \ + tst-malloc-usable tst-realloc tst-reallocarray tst-posix_memalign \ tst-pvalloc tst-memalign tst-mallopt test-srcs = tst-mtrace diff --git a/malloc/Versions b/malloc/Versions index 7ca9bdf..64fade5 100644 --- a/malloc/Versions +++ b/malloc/Versions @@ -61,6 +61,10 @@ libc { GLIBC_2.16 { aligned_alloc; } + GLIBC_2.20 { + __libc_reallocarray; + reallocarray; + } GLIBC_PRIVATE { # Internal startup hook for libpthread. __libc_malloc_pthread_startup; diff --git a/malloc/malloc.c b/malloc/malloc.c index 1120d4d..822e400 100644 --- a/malloc/malloc.c +++ b/malloc/malloc.c @@ -2943,6 +2943,36 @@ void *weak_variable (*__memalign_hook) } libc_hidden_def (__libc_free) +static inline bool +check_mul_overflow(INTERNAL_SIZE_T l, INTERNAL_SIZE_T r, + INTERNAL_SIZE_T *result) +{ + /* size_t is unsigned so the behavior on overflow is defined. */ + *result = l * r; +#define HALF_INTERNAL_SIZE_T \ + (((INTERNAL_SIZE_T) 1) << (8 * sizeof (INTERNAL_SIZE_T) / 2)) + if (__glibc_unlikely ((l | r) >= HALF_INTERNAL_SIZE_T)) + { + if (r != 0 && *result / r != l) + return true; + } + return false; +#undef HALF_INTERNAL_SIZE_T +} + +void * +__libc_reallocarray(void *optr, size_t nmemb, size_t elem_size) +{ + INTERNAL_SIZE_T bytes; + if (check_mul_overflow(nmemb, elem_size, &bytes)) + { + __set_errno (ENOMEM); + return 0; + } + else + return __libc_realloc (optr, bytes); +} + void * __libc_realloc (void *oldmem, size_t bytes) { @@ -3153,17 +3183,10 @@ void *weak_variable (*__memalign_hook) unsigned long nclears; INTERNAL_SIZE_T *d; - /* size_t is unsigned so the behavior on overflow is defined. */ - bytes = n * elem_size; -#define HALF_INTERNAL_SIZE_T \ - (((INTERNAL_SIZE_T) 1) << (8 * sizeof (INTERNAL_SIZE_T) / 2)) - if (__builtin_expect ((n | elem_size) >= HALF_INTERNAL_SIZE_T, 0)) + if (check_mul_overflow(n, elem_size, &bytes)) { - if (elem_size != 0 && bytes / elem_size != n) - { - __set_errno (ENOMEM); - return 0; - } + __set_errno (ENOMEM); + return 0; } void *(*hook) (size_t, const void *) = @@ -5171,6 +5194,8 @@ struct mallinfo strong_alias (__libc_malloc, __malloc) strong_alias (__libc_malloc, malloc) strong_alias (__libc_memalign, __memalign) weak_alias (__libc_memalign, memalign) +strong_alias (__libc_reallocarray, __reallocarray) + strong_alias (__libc_reallocarray, reallocarray) strong_alias (__libc_realloc, __realloc) strong_alias (__libc_realloc, realloc) strong_alias (__libc_valloc, __valloc) weak_alias (__libc_valloc, valloc) strong_alias (__libc_pvalloc, __pvalloc) weak_alias (__libc_pvalloc, pvalloc) diff --git a/malloc/malloc.h b/malloc/malloc.h index 30bb91a..db7e5a9 100644 --- a/malloc/malloc.h +++ b/malloc/malloc.h @@ -49,6 +49,14 @@ extern void *calloc (size_t __nmemb, size_t __size) extern void *realloc (void *__ptr, size_t __size) __THROW __attribute_warn_unused_result__; +/* Re-allocate the previously allocated block in PTR, making the new + block large enough for NMEMB elements of SIZE bytes each. */ +/* __attribute_malloc__ is not used, because if realloc returns + the same pointer that was passed to it, aliasing needs to be allowed + between objects pointed by the old and new pointers. */ +extern void *reallocarray (void *__ptr, size_t __nmemb, size_t __size) + __THROW __attribute_warn_unused_result__; + /* Free a block allocated by `malloc', `realloc' or `calloc'. */ extern void free (void *__ptr) __THROW; diff --git a/malloc/tst-reallocarray.c b/malloc/tst-reallocarray.c new file mode 100644 index 0000000..b85825e --- /dev/null +++ b/malloc/tst-reallocarray.c @@ -0,0 +1,160 @@ +/* Copyright (C) 2014 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include +#include +#include +#include +#include + +static int errors = 0; + +static void +merror (const char *msg) +{ + ++errors; + printf ("Error: %s.\n", msg); +} + +static int +do_test (void) + +{ + void *ptr = NULL; + void *ptr2 = NULL; + unsigned char *c; + size_t i; + int ok; + const size_t max = ~(size_t)0; + size_t a, b; + + /* Test overflow detection. */ + errno = 0; + ptr = reallocarray (NULL, max, 2); + if (ptr) + { + merror ("Overflow for size_t MAX * 2 not detected"); + free(ptr); + } + else if (errno != ENOMEM) + merror ("errno is not set correctly"); + + errno = 0; + ptr = reallocarray (NULL, 2, max); + if (ptr) + { + merror ("Overflow for 2 * size_t MAX not detected"); + free(ptr); + } + else if (errno != ENOMEM) + merror ("errno is not set correctly"); + + a = 65537; + b = max/65537 + 1; + errno = 0; + ptr = reallocarray (NULL, a, b); + if (ptr) + { + merror ("Overflow for (size_t MAX/65537 + 1) * 65537 not detected"); + free(ptr); + } + else if (errno != ENOMEM) + merror ("errno is not set correctly"); + + errno = 0; + ptr = reallocarray (NULL, b, a); + if (ptr) + { + merror ("Overflow for 65537 * (size_t MAX/65537 + 1) not detected"); + free(ptr); + } + else if (errno != ENOMEM) + merror ("errno is not set correctly"); + + /* Test realloc-like behavior. */ + /* Allocate memory like malloc. */ + ptr = reallocarray(NULL, 10, 2); + if (!ptr) + merror ("realloc(NULL, 10, 2) failed"); + + memset (ptr, 0xAF, 10*2); + + /* Enlarge buffer. */ + ptr2 = reallocarray(ptr, 20, 2); + if (!ptr2) + merror ("realloc(ptr, 20, 2) failed (enlarge)"); + else + ptr = ptr2; + + c = ptr; + ok = 1; + for (i = 0; i < 10*2; ++i) + { + if (c[i] != 0xAF) + ok = 0; + } + if (!ok) + merror ("Enlarging changed buffer content (10*2)"); + + /* Decrease buffer size. */ + ptr2 = reallocarray(ptr, 5, 3); + if (!ptr2) + merror ("realloc(ptr, 5, 3) failed (decrease)"); + else + ptr = ptr2; + + c = ptr; + ok = 1; + for (i = 0; i < 5*3; ++i) + { + if (c[i] != 0xAF) + ok = 0; + } + if (!ok) + merror ("Reducing changed buffer content (5*3)"); + + /* Overflow should leave buffer untouched. */ + errno = 0; + ptr2 = reallocarray(ptr, 2, ~(size_t)0); + if (ptr2) + merror ("realloc(ptr, 2, size_t MAX) failed to detect overflow"); + if (errno != ENOMEM) + merror ("errno not set correctly"); + + c = ptr; + ok = 1; + for (i = 0; i < 5*3; ++i) + { + if (c[i] != 0xAF) + ok = 0; + } + if (!ok) + merror ("Overflow changed buffer content (5*3)"); + + /* Free buffer (glibc). */ + errno = 0; + ptr2 = reallocarray (ptr, 0, 0); + if (ptr2) + merror ("reallocarray (ptr, 0, 0) returned non-NULL"); + + free (ptr2); + + return errors != 0; +} + +#define TEST_FUNCTION do_test () +#include "../test-skeleton.c" diff --git a/stdlib/stdlib.h b/stdlib/stdlib.h index 00329a2..b75c28f 100644 --- a/stdlib/stdlib.h +++ b/stdlib/stdlib.h @@ -479,6 +479,13 @@ extern void *calloc (size_t __nmemb, size_t __size) between objects pointed by the old and new pointers. */ extern void *realloc (void *__ptr, size_t __size) __THROW __attribute_warn_unused_result__; +/* Re-allocate the previously allocated block in PTR, making the new + block large enough for NMEMB elements of SIZE bytes each. */ +/* __attribute_malloc__ is not used, because if realloc returns + the same pointer that was passed to it, aliasing needs to be allowed + between objects pointed by the old and new pointers. */ +extern void *reallocarray (void *__ptr, size_t __nmemb, size_t __size) + __THROW __attribute_warn_unused_result__; /* Free a block allocated by `malloc', `realloc' or `calloc'. */ extern void free (void *__ptr) __THROW; __END_NAMESPACE_STD