From patchwork Fri Dec 21 18:38:12 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 154418 Delivered-To: patch@linaro.org Received: by 2002:a2e:299d:0:0:0:0:0 with SMTP id p29-v6csp1248416ljp; Fri, 21 Dec 2018 10:38:41 -0800 (PST) X-Google-Smtp-Source: AFSGD/U/T/rfSlpjAE9TOxf4jmfvogGXZNr5r68KiE+khFqNoDLtLc2fSpLmC9VE95EdbR2zs1kq X-Received: by 2002:a62:7e93:: with SMTP id z141mr3569211pfc.239.1545417521086; Fri, 21 Dec 2018 10:38:41 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1545417521; cv=none; d=google.com; s=arc-20160816; b=jXRttaL4ajAnWvc0HKMKqBWwx9AnQHvQmqhrhKZDfDaIfAfyqSaACSzoejZRiu61nO Q2Djj282ybajjAdAJPcB33QmUzHSuFuvR4Rk9btjplS53dvLXhIP/3ti29Bb1nIMht7h GMZL7I8QrZB7irNVYMfZiogTQzcEumdjpsoRViXlNNzrCjFr4lsA6Dkrgp7f3+zYiToV tVm9QqAbBZGmJ9dYQC9fs/7CRutoC67ZCuI7Rp9CNQTfHX++LvtZ7IcAlTfG3X8ecNhU E9Vkl9RTbojS9YxYJhxAEcfw/zi3Ajpt+2gcOd4DvAklsiQT1LFjAyoDeNfm2T1bjpQe juaw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=message-id:date:subject:to:from:dkim-signature:delivered-to:sender :list-help:list-post:list-archive:list-subscribe:list-unsubscribe :list-id:precedence:mailing-list:dkim-signature:domainkey-signature; bh=dLNZV6/QWR157oOMzPeShfwU2o/vVR9pZbkUX91TLQk=; b=KD83lSQr2Yd8xR+PvKW4brW76LPCwV4+fr82iY899ZP9JTjJ58lrPqfzGiM229huVx 7z2gjiRBXR7K5NHI1aA0Fm+ewjJ+UdEhqbeKwqPO9w4DnCrasii7s86EA8VmZyU5gnFL ZBpRghMhcYO/s6gyCC/FRNifEQwtWU6jNf8/Ar2x+GDN81UuiGhOu4lk1SlOZUAWFqCp olBWfCsdrcGrhmjUPoDJwMkb5NMFAP1X0RbIY+3asExXgjks8dnDeI2+CfoT0nrpLNRK Oxu0KhjEl+xjclGOvaCgr3D9yONU2ZNqV3zcYPbjhxzD1gIb9KWizi5PbiWBdtkbO18O 9OFA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@sourceware.org header.s=default header.b=VYztuylc; dkim=pass header.i=@linaro.org header.s=google header.b="gH/qCv88"; spf=pass (google.com: domain of libc-alpha-return-98718-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom="libc-alpha-return-98718-patch=linaro.org@sourceware.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id p3si3468167plk.424.2018.12.21.10.38.40 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 21 Dec 2018 10:38:41 -0800 (PST) Received-SPF: pass (google.com: domain of libc-alpha-return-98718-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@sourceware.org header.s=default header.b=VYztuylc; dkim=pass header.i=@linaro.org header.s=google header.b="gH/qCv88"; spf=pass (google.com: domain of libc-alpha-return-98718-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom="libc-alpha-return-98718-patch=linaro.org@sourceware.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org 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:subject:date:message-id; q=dns; s= default; b=ah6k8TRXJVFdBhEeOdtGiE8k8wAsmtsdGucL/MKmY8vGxdJFf2XRL 44DeBQrhuiVtxPjPUvvQE0JK7Wk9Munc/JGENvr3u/DhLmm92/s27G8PTrQO5Er3 FWku83UlyRILr9vrh3wFGKFDlFElNbovMu8D+zg74HsNsow/DkYtHU= 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:subject:date:message-id; s=default; bh=27/vRfWPFUUyYB20kc0P4RAHWR8=; b=VYztuylcR4tvLn2Rims09TOfB6H8 iV04f53+uea0iLtU95M+aAbjkt5nZiQCa+oz42zPz0+pVnGn2x1EH7M0S/zZm+L4 Yqe5vROwgrbfFSmKKhOuKRHcfoBblPqtHMW6zV/qw3Twut4fwI0IsUPmtuSMoYIi qd6nJ9VA9qLvw50= Received: (qmail 10685 invoked by alias); 21 Dec 2018 18:38:31 -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 10671 invoked by uid 89); 21 Dec 2018 18:38:31 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=fill, shutdown, 387, 1024 X-HELO: mail-qt1-f196.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:subject:date:message-id; bh=dLNZV6/QWR157oOMzPeShfwU2o/vVR9pZbkUX91TLQk=; b=gH/qCv8859vH42U70S8dITXt3VxJo8o9KAZddEz7XDK5jFX1+9HTgI4UBACSsPF9YE MvmL28lu04QrunCaVZ8rxIN46sUaEGJ6FBMj8kE1nWlJkKuksCP2xzvTs/lCIRglGjDb caUxx1JwHteBmgWlnB3Jf+yXw90v4fAMxKBgc= Return-Path: From: Adhemerval Zanella To: libc-alpha@sourceware.org Subject: [PATCH 1/2] Replace check_mul_overflow_size_t with INT_MULTIPLY_WRAPV Date: Fri, 21 Dec 2018 16:38:12 -0200 Message-Id: <20181221183813.16245-1-adhemerval.zanella@linaro.org> Checked on x86_64-linux-gnu and i686-linux-gnu. * malloc/alloc_buffer_alloc_array.c (__libc_alloc_buffer_alloc_array): Use INT_MULTIPLY_WRAPV in place of check_mul_overflow_size_t. * malloc/dynarray_emplace_enlarge.c (__libc_dynarray_emplace_enlarge): Likewise. * malloc/dynarray_resize.c (__libc_dynarray_resize): Likewise. * malloc/reallocarray.c (__libc_reallocarray): Likewise. * malloc/malloc-internal.h (check_mul_overflow_size_t): Remove function. * support/blob_repeat.c (check_mul_overflow_size_t, (minimum_stride_size, support_blob_repeat_allocate): Likewise. --- ChangeLog | 13 +++++++++++++ malloc/alloc_buffer_alloc_array.c | 4 ++-- malloc/dynarray_emplace_enlarge.c | 4 ++-- malloc/dynarray_resize.c | 4 ++-- malloc/malloc-internal.h | 20 -------------------- malloc/reallocarray.c | 7 +++---- support/blob_repeat.c | 27 ++++----------------------- 7 files changed, 26 insertions(+), 53 deletions(-) -- 2.17.1 diff --git a/malloc/alloc_buffer_alloc_array.c b/malloc/alloc_buffer_alloc_array.c index 1dd098a8fc..35de7115e2 100644 --- a/malloc/alloc_buffer_alloc_array.c +++ b/malloc/alloc_buffer_alloc_array.c @@ -17,7 +17,7 @@ . */ #include -#include +#include #include void * @@ -28,7 +28,7 @@ __libc_alloc_buffer_alloc_array (struct alloc_buffer *buf, size_t element_size, /* The caller asserts that align is a power of two. */ size_t aligned = ALIGN_UP (current, align); size_t size; - bool overflow = check_mul_overflow_size_t (element_size, count, &size); + bool overflow = INT_MULTIPLY_WRAPV (element_size, count, &size); size_t new_current = aligned + size; if (!overflow /* Multiplication did not overflow. */ && aligned >= current /* No overflow in align step. */ diff --git a/malloc/dynarray_emplace_enlarge.c b/malloc/dynarray_emplace_enlarge.c index 0408271e27..7538cbe4c5 100644 --- a/malloc/dynarray_emplace_enlarge.c +++ b/malloc/dynarray_emplace_enlarge.c @@ -18,7 +18,7 @@ #include #include -#include +#include #include #include @@ -52,7 +52,7 @@ __libc_dynarray_emplace_enlarge (struct dynarray_header *list, } size_t new_size; - if (check_mul_overflow_size_t (new_allocated, element_size, &new_size)) + if (INT_MULTIPLY_WRAPV (new_allocated, element_size, &new_size)) return false; void *new_array; if (list->array == scratch) diff --git a/malloc/dynarray_resize.c b/malloc/dynarray_resize.c index 0bfca1ba4b..4d766605ff 100644 --- a/malloc/dynarray_resize.c +++ b/malloc/dynarray_resize.c @@ -18,7 +18,7 @@ #include #include -#include +#include #include #include @@ -38,7 +38,7 @@ __libc_dynarray_resize (struct dynarray_header *list, size_t size, over-allocation here. */ size_t new_size_bytes; - if (check_mul_overflow_size_t (size, element_size, &new_size_bytes)) + if (INT_MULTIPLY_WRAPV (size, element_size, &new_size_bytes)) { /* Overflow. */ __set_errno (ENOMEM); diff --git a/malloc/malloc-internal.h b/malloc/malloc-internal.h index 9cee0fb2d7..70d5b38504 100644 --- a/malloc/malloc-internal.h +++ b/malloc/malloc-internal.h @@ -74,24 +74,4 @@ void __malloc_fork_unlock_child (void) attribute_hidden; /* Called as part of the thread shutdown sequence. */ void __malloc_arena_thread_freeres (void) attribute_hidden; -/* Set *RESULT to LEFT * RIGHT. Return true if the multiplication - overflowed. */ -static inline bool -check_mul_overflow_size_t (size_t left, size_t right, size_t *result) -{ -#if __GNUC__ >= 5 - return __builtin_mul_overflow (left, right, result); -#else - /* size_t is unsigned so the behavior on overflow is defined. */ - *result = left * right; - size_t half_size_t = ((size_t) 1) << (8 * sizeof (size_t) / 2); - if (__glibc_unlikely ((left | right) >= half_size_t)) - { - if (__glibc_unlikely (right != 0 && *result / right != left)) - return true; - } - return false; -#endif -} - #endif /* _MALLOC_INTERNAL_H */ diff --git a/malloc/reallocarray.c b/malloc/reallocarray.c index 319eccd21f..f3a83abc91 100644 --- a/malloc/reallocarray.c +++ b/malloc/reallocarray.c @@ -18,19 +18,18 @@ #include #include -#include +#include void * __libc_reallocarray (void *optr, size_t nmemb, size_t elem_size) { size_t bytes; - if (check_mul_overflow_size_t (nmemb, elem_size, &bytes)) + if (INT_MULTIPLY_WRAPV (nmemb, elem_size, &bytes)) { __set_errno (ENOMEM); return 0; } - else - return realloc (optr, bytes); + return realloc (optr, bytes); } libc_hidden_def (__libc_reallocarray) diff --git a/support/blob_repeat.c b/support/blob_repeat.c index 718846d81d..e4260872fd 100644 --- a/support/blob_repeat.c +++ b/support/blob_repeat.c @@ -29,31 +29,12 @@ #include #include #include +#include /* Small allocations should use malloc directly instead of the mmap optimization because mappings carry a lot of overhead. */ static const size_t maximum_small_size = 4 * 1024 * 1024; -/* Set *RESULT to LEFT * RIGHT. Return true if the multiplication - overflowed. See . */ -static inline bool -check_mul_overflow_size_t (size_t left, size_t right, size_t *result) -{ -#if __GNUC__ >= 5 - return __builtin_mul_overflow (left, right, result); -#else - /* size_t is unsigned so the behavior on overflow is defined. */ - *result = left * right; - size_t half_size_t = ((size_t) 1) << (8 * sizeof (size_t) / 2); - if (__glibc_unlikely ((left | right) >= half_size_t)) - { - if (__glibc_unlikely (right != 0 && *result / right != left)) - return true; - } - return false; -#endif -} - /* Internal helper for fill. */ static void fill0 (char *target, const char *element, size_t element_size, @@ -138,8 +119,8 @@ minimum_stride_size (size_t page_size, size_t element_size) common multiple, it appears only once. Therefore, shift one factor. */ size_t multiple; - if (check_mul_overflow_size_t (page_size >> common_zeros, element_size, - &multiple)) + if (INT_MULTIPLY_WRAPV (page_size >> common_zeros, element_size, + &multiple)) return 0; return multiple; } @@ -275,7 +256,7 @@ support_blob_repeat_allocate (const void *element, size_t element_size, size_t count) { size_t total_size; - if (check_mul_overflow_size_t (element_size, count, &total_size)) + if (INT_MULTIPLY_WRAPV (element_size, count, &total_size)) { errno = EOVERFLOW; return (struct support_blob_repeat) { 0 }; From patchwork Fri Dec 21 18:38:13 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 154419 Delivered-To: patch@linaro.org Received: by 2002:a2e:299d:0:0:0:0:0 with SMTP id p29-v6csp1248531ljp; Fri, 21 Dec 2018 10:38:51 -0800 (PST) X-Google-Smtp-Source: ALg8bN6cKh9aFGBbAFNqSWG1oMCkUxNj8xysmPTJm/62FJmY/nE1l17J1mRwvwOD99CYsRCbdBCJ X-Received: by 2002:a63:1e56:: with SMTP id p22mr3482823pgm.126.1545417530960; Fri, 21 Dec 2018 10:38:50 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1545417530; cv=none; d=google.com; s=arc-20160816; b=rrN+AlcyjbbyiCmQqPGp2nhVLTS9XHoxNTXTByerpKBwJzEuVkyV1vkHgo8Qp6YpNn 8rU1s1HXGAYpKOSaQg612ykoYJbeqmAZHdgXro9zF68Ky7p3QGlAs3TGT6zfAM1lbHPR s5IkeROl+71KKcYgBeDJB9NhBKlxoTZJqPCDhGUmRpG7OeZA49d5BsmjbfKlWVo3eHtD CfgbClC52AsBt2lPkwQXOKo5tysN6er0hWvL0iZnCDeq5WNwKwl69mSPR6Mhc7fsMGp8 tvSJ3uQUkrH76jEJZETwQ78ji90oYl050hvX5AQm16Cf+SwrbMv5ke8+fFXGTSjyxkdH Gglw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=references:in-reply-to:message-id:date:subject:to:from :dkim-signature:delivered-to:sender:list-help:list-post:list-archive :list-subscribe:list-unsubscribe:list-id:precedence:mailing-list :dkim-signature:domainkey-signature; bh=An7JeEB8LPKsCr/yPoosAfud2PObP7ybzdQC/w200U8=; b=CIBOe+yweHZtk9lF81RarfP324pXNveqNVS7gdtRKpfutBYnX2g5K4i+4Jy7Wmbhtg k+PGWIIhDddl3Sk189fZYEHU1dz+mJ0V7m4C3eJe3UJ/yTY82xfXj+Vx9XfLlL6Whrkl W5TYHqR/87QkUGOrwvew5AhBWvtzVHR61XuQheOqscGjPU5qpGfNNQ++OKrHO5zudWea 51WaXtnC10VVkB6Zd5ResdbTXZr4hxIHXgpBq6NcVG2Os+EJaDJAPhfCeZtBcBK3oN/Z +WrbNPF4LQmZteU632QgYvBUAXhd4RRrS84Di2DGG6X6yu8Y8ityw5BsGue6tgVak55X S+fA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@sourceware.org header.s=default header.b="P/biiCRo"; dkim=pass header.i=@linaro.org header.s=google header.b=cM1sGnjr; spf=pass (google.com: domain of libc-alpha-return-98719-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom="libc-alpha-return-98719-patch=linaro.org@sourceware.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id g34si3581695pld.15.2018.12.21.10.38.50 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 21 Dec 2018 10:38:50 -0800 (PST) Received-SPF: pass (google.com: domain of libc-alpha-return-98719-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@sourceware.org header.s=default header.b="P/biiCRo"; dkim=pass header.i=@linaro.org header.s=google header.b=cM1sGnjr; spf=pass (google.com: domain of libc-alpha-return-98719-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom="libc-alpha-return-98719-patch=linaro.org@sourceware.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org 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:subject:date:message-id:in-reply-to :references; q=dns; s=default; b=QvwpEXA9K2FmWUoIjPeOs39kGJp6lFP T9VmDATiN+FyWPF167Cv4tuk0yjFD+f/4l7W/AOYNSUFo+ypKliAj6/BIocAXoYE 0VRV0GEEl7Ufr4Bds7FhSCNS54NEA8bp3cHJjSmNomAjtDZ5NzP36NISy4v6iqHg tRf18Cjd7+Yk= 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:subject:date:message-id:in-reply-to :references; s=default; bh=lyQ+59HFyk9cXckfc9sfjah6i5Y=; b=P/bii CRopJ+0oGJVN8Q7K/azXZDgiG5am0sP4eA0BSSqDE94lItynHzjPeq2TfEAOlwgs IMpIiEgRWiOdGtfoDBdLOTwjYX+4qsGvtP6/OQB8Wv58e7EZVe6jEYItcPut9HdU fZq8KU9B0j6hhplRuqJNmB6HzuGuyun9koPVok= Received: (qmail 11452 invoked by alias); 21 Dec 2018 18:38:38 -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 11244 invoked by uid 89); 21 Dec 2018 18:38:36 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=smallest, 2023, pieces, 20, 6 X-HELO: mail-qk1-f195.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:subject:date:message-id:in-reply-to:references; bh=An7JeEB8LPKsCr/yPoosAfud2PObP7ybzdQC/w200U8=; b=cM1sGnjrV7aqjXMjyEt5TmO0+pba5mjLQNYA9d2FPVLt5NSSuZE0n94Ur7GFqTuJFh JQ/EP+Owlla3qQjb07InrUsidG4Q8L8huJKLDIKUEiUO2ltWZws9Vi/uJORfmShRzSna ftn1FlPMPYOHZlrV3Oa+rj/fpMeBIl+vvFVO8= Return-Path: From: Adhemerval Zanella To: libc-alpha@sourceware.org Subject: [PATCH 2/2] malloc: make malloc fail with requests larger than PTRDIFF_MAX Date: Fri, 21 Dec 2018 16:38:13 -0200 Message-Id: <20181221183813.16245-2-adhemerval.zanella@linaro.org> In-Reply-To: <20181221183813.16245-1-adhemerval.zanella@linaro.org> References: <20181221183813.16245-1-adhemerval.zanella@linaro.org> As discussed previously on libc-alpha [1], this patch follows up the idea and add both the __attribute_alloc_size__ on malloc functions (malloc, calloc, realloc, reallocarray, valloc, pvalloc, memaling, and posix_memalign) and limit maximum requested allocation size to up PTRDIFF_MAX (minus internal padding and alignment when required). This aligns glibc with gcc expected size defined by default warning -Walloc-size-larger-than value which warns for allocation larger than PTRDIFF_MAX. It also aligns with gcc expectation regarding libc and expected size, such as described in PR#67999 [2] and previously discussed ISO C11 issues [3] on libc-alpha. >From the RFC thread [4] and previous discussion, it seems that consensus now is only to limit such requested size for malloc functions, not system allocation one (mmap, sbrk, etc.). This patch follows this idea. The implementation removes checked_request2size and move the requested size before hook calls (this avoid code duplication in the hook and make allocations returned by hooks safe even if they do not check for requested size). Checked on x86_64-linux-gnu and i686-linux-gnu. [BZ #23741] * malloc/hooks.c (malloc_check, realloc_check, memalign_check): Remove overflow check. * malloc/malloc.c (__libc_malloc, __libc_realloc, _mid_memalign, __libc_pvalloc, __libc_calloc, _int_memalign): Limit maximum allocation size to PTRDIFF_MAX. (_int_malloc): Remove overflow check. (padize): New define. (request2size): Define based on padsize. * malloc/malloc.h (malloc, calloc, realloc, reallocarray, memalign, valloc, pvalloc): Add __attribute_alloc_size__. * stdlib/stdlib.h (malloc, realloc, reallocarray, valloc, posix_memalign): Likewise. * malloc/tst-malloc-too-large.c (do_test): Add check for allocation larger than PTRDIFF_MAX. * malloc/tst-memalign.c (do_test): Disable -Walloc-size-larger-than= around tests of malloc with negative sizes. * malloc/tst-posix_memalign.c (do_test): Likewise. * malloc/tst-pvalloc.c (do_test): Likewise. * malloc/tst-valloc.c (do_test): Likewise. * malloc/tst-reallocarray.c (do_test): Replace call to reallocarray with resulting size allocation larger than PTRDIFF_MAX with reallocarray_nowarn. (reallocarray_nowarn): New function. [1] https://sourceware.org/ml/libc-alpha/2018-11/msg00223.html [2] https://gcc.gnu.org/bugzilla//show_bug.cgi?id=67999 [3] https://sourceware.org/ml/libc-alpha/2011-12/msg00066.html [4] https://sourceware.org/ml/libc-alpha/2018-11/msg00224.html --- ChangeLog | 25 +++++++ malloc/hooks.c | 22 +------ malloc/malloc.c | 118 +++++++++++++--------------------- malloc/malloc.h | 17 +++-- malloc/tst-malloc-too-large.c | 10 +++ malloc/tst-memalign.c | 10 +++ malloc/tst-posix_memalign.c | 10 +++ malloc/tst-pvalloc.c | 10 +++ malloc/tst-reallocarray.c | 27 ++++++-- malloc/tst-valloc.c | 10 +++ stdlib/stdlib.h | 15 +++-- 11 files changed, 160 insertions(+), 114 deletions(-) -- 2.17.1 diff --git a/malloc/hooks.c b/malloc/hooks.c index ae7305b036..b77c908e28 100644 --- a/malloc/hooks.c +++ b/malloc/hooks.c @@ -227,12 +227,6 @@ malloc_check (size_t sz, const void *caller) { void *victim; - if (sz + 1 == 0) - { - __set_errno (ENOMEM); - return NULL; - } - __libc_lock_lock (main_arena.mutex); top_check (); victim = _int_malloc (&main_arena, sz + 1); @@ -269,11 +263,6 @@ realloc_check (void *oldmem, size_t bytes, const void *caller) void *newmem = 0; unsigned char *magic_p; - if (bytes + 1 == 0) - { - __set_errno (ENOMEM); - return NULL; - } if (oldmem == 0) return malloc_check (bytes, NULL); @@ -289,7 +278,7 @@ realloc_check (void *oldmem, size_t bytes, const void *caller) malloc_printerr ("realloc(): invalid pointer"); const INTERNAL_SIZE_T oldsize = chunksize (oldp); - checked_request2size (bytes + 1, nb); + nb = request2size (bytes + 1); __libc_lock_lock (main_arena.mutex); if (chunk_is_mmapped (oldp)) @@ -320,8 +309,6 @@ realloc_check (void *oldmem, size_t bytes, const void *caller) else { top_check (); - INTERNAL_SIZE_T nb; - checked_request2size (bytes + 1, nb); newmem = _int_realloc (&main_arena, oldp, oldsize, nb); } @@ -362,13 +349,6 @@ memalign_check (size_t alignment, size_t bytes, const void *caller) return 0; } - /* Check for overflow. */ - if (bytes > SIZE_MAX - alignment - MINSIZE) - { - __set_errno (ENOMEM); - return 0; - } - /* Make sure alignment is power of 2. */ if (!powerof2 (alignment)) { diff --git a/malloc/malloc.c b/malloc/malloc.c index c33709e966..bb80809983 100644 --- a/malloc/malloc.c +++ b/malloc/malloc.c @@ -247,6 +247,9 @@ /* For SINGLE_THREAD_P. */ #include +/* For INT_MULTIPLY_WRAPV. */ +#include + /* Debugging: @@ -1187,39 +1190,15 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ ((uintptr_t)(MALLOC_ALIGNMENT == 2 * SIZE_SZ ? (p) : chunk2mem (p)) \ & MALLOC_ALIGN_MASK) +/* pad request bytes into a usable size -- internal version */ -/* - Check if a request is so large that it would wrap around zero when - padded and aligned. To simplify some other code, the bound is made - low enough so that adding MINSIZE will also not wrap around zero. - */ - -#define REQUEST_OUT_OF_RANGE(req) \ - ((unsigned long) (req) >= \ - (unsigned long) (INTERNAL_SIZE_T) (-2 * MINSIZE)) +#define padsize (SIZE_SZ + MALLOC_ALIGN_MASK) -/* pad request bytes into a usable size -- internal version */ +#define request2size(req) \ + (((req) + padsize < MINSIZE) ? \ + MINSIZE : \ + ((req) + padsize) & ~MALLOC_ALIGN_MASK) -#define request2size(req) \ - (((req) + SIZE_SZ + MALLOC_ALIGN_MASK < MINSIZE) ? \ - MINSIZE : \ - ((req) + SIZE_SZ + MALLOC_ALIGN_MASK) & ~MALLOC_ALIGN_MASK) - -/* Same, except also perform an argument and result check. First, we check - that the padding done by request2size didn't result in an integer - overflow. Then we check (using REQUEST_OUT_OF_RANGE) that the resulting - size isn't so large that a later alignment would lead to another integer - overflow. */ -#define checked_request2size(req, sz) \ -({ \ - (sz) = request2size (req); \ - if (((sz) < (req)) \ - || REQUEST_OUT_OF_RANGE (sz)) \ - { \ - __set_errno (ENOMEM); \ - return 0; \ - } \ -}) /* --------------- Physical chunk operations --------------- @@ -3109,14 +3088,20 @@ __libc_malloc (size_t bytes) mstate ar_ptr; void *victim; + if (__glibc_unlikely (bytes > PTRDIFF_MAX - padsize)) + { + __set_errno (ENOMEM); + return NULL; + } + void *(*hook) (size_t, const void *) = atomic_forced_read (__malloc_hook); if (__builtin_expect (hook != NULL, 0)) return (*hook)(bytes, RETURN_ADDRESS (0)); + #if USE_TCACHE /* int_free also calls request2size, be careful to not pad twice. */ - size_t tbytes; - checked_request2size (bytes, tbytes); + size_t tbytes = request2size (bytes); size_t tc_idx = csize2tidx (tbytes); MAYBE_INIT_TCACHE (); @@ -3213,6 +3198,12 @@ __libc_realloc (void *oldmem, size_t bytes) void *newp; /* chunk to return */ + if (__glibc_unlikely (bytes > PTRDIFF_MAX - padsize)) + { + __set_errno (ENOMEM); + return 0; + } + void *(*hook) (void *, size_t, const void *) = atomic_forced_read (__realloc_hook); if (__builtin_expect (hook != NULL, 0)) @@ -3253,7 +3244,7 @@ __libc_realloc (void *oldmem, size_t bytes) && !DUMPED_MAIN_ARENA_CHUNK (oldp)) malloc_printerr ("realloc(): invalid pointer"); - checked_request2size (bytes, nb); + nb = request2size (bytes); if (chunk_is_mmapped (oldp)) { @@ -3342,6 +3333,12 @@ _mid_memalign (size_t alignment, size_t bytes, void *address) mstate ar_ptr; void *p; + if (__glibc_unlikely (bytes > PTRDIFF_MAX - padsize)) + { + __set_errno (ENOMEM); + return 0; + } + void *(*hook) (size_t, size_t, const void *) = atomic_forced_read (__memalign_hook); if (__builtin_expect (hook != NULL, 0)) @@ -3363,14 +3360,6 @@ _mid_memalign (size_t alignment, size_t bytes, void *address) return 0; } - /* Check for overflow. */ - if (bytes > SIZE_MAX - alignment - MINSIZE) - { - __set_errno (ENOMEM); - return 0; - } - - /* Make sure alignment is power of 2. */ if (!powerof2 (alignment)) { @@ -3432,7 +3421,7 @@ __libc_pvalloc (size_t bytes) size_t rounded_bytes = ALIGN_UP (bytes, pagesize); /* Check for overflow. */ - if (bytes > SIZE_MAX - 2 * pagesize - MINSIZE) + if (__glibc_unlikely (bytes > PTRDIFF_MAX - 2 * pagesize - MINSIZE)) { __set_errno (ENOMEM); return 0; @@ -3452,24 +3441,18 @@ __libc_calloc (size_t n, size_t elem_size) 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 (INT_MULTIPLY_WRAPV (n, elem_size, &bytes) + || __glibc_unlikely (bytes > PTRDIFF_MAX - padsize)) { - if (elem_size != 0 && bytes / elem_size != n) - { - __set_errno (ENOMEM); - return 0; - } + __set_errno (ENOMEM); + return NULL; } + sz = bytes; void *(*hook) (size_t, const void *) = atomic_forced_read (__malloc_hook); if (__builtin_expect (hook != NULL, 0)) { - sz = bytes; mem = (*hook)(sz, RETURN_ADDRESS (0)); if (mem == 0) return 0; @@ -3477,8 +3460,6 @@ __libc_calloc (size_t n, size_t elem_size) return memset (mem, 0, sz); } - sz = bytes; - MAYBE_INIT_TCACHE (); if (SINGLE_THREAD_P) @@ -3621,16 +3602,7 @@ _int_malloc (mstate av, size_t bytes) size_t tcache_unsorted_count; /* count of unsorted chunks processed */ #endif - /* - Convert request size to internal form by adding SIZE_SZ bytes - overhead plus possibly more to obtain necessary alignment and/or - to obtain a size of at least MINSIZE, the smallest allocatable - size. Also, checked_request2size traps (returning 0) request sizes - that are so large that they wrap around zero when padded and - aligned. - */ - - checked_request2size (bytes, nb); + nb = request2size (bytes); /* There are no usable arenas. Fall back to sysmalloc to get a chunk from mmap. */ @@ -4741,22 +4713,18 @@ _int_memalign (mstate av, size_t alignment, size_t bytes) unsigned long remainder_size; /* its size */ INTERNAL_SIZE_T size; - - - checked_request2size (bytes, nb); + if (__glibc_unlikely (bytes > PTRDIFF_MAX - padsize)) + { + __set_errno (ENOMEM); + return 0; + } /* Strategy: find a spot within that chunk that meets the alignment request, and then possibly free the leading and trailing space. */ - - /* Check for overflow. */ - if (nb > SIZE_MAX - alignment - MINSIZE) - { - __set_errno (ENOMEM); - return 0; - } + nb = request2size (bytes); /* Call malloc with worst case padding to hit alignment. */ diff --git a/malloc/malloc.h b/malloc/malloc.h index 3e7c447be1..d3c4841f11 100644 --- a/malloc/malloc.h +++ b/malloc/malloc.h @@ -35,11 +35,12 @@ __BEGIN_DECLS /* Allocate SIZE bytes of memory. */ -extern void *malloc (size_t __size) __THROW __attribute_malloc__ __wur; +extern void *malloc (size_t __size) __THROW __attribute_malloc__ + __attribute_alloc_size__ ((1)) __wur; /* Allocate NMEMB elements of SIZE bytes each, all initialized to 0. */ extern void *calloc (size_t __nmemb, size_t __size) -__THROW __attribute_malloc__ __wur; +__THROW __attribute_malloc__ __attribute_alloc_size__ ((1, 2)) __wur; /* Re-allocate the previously allocated block in __ptr, making the new block SIZE bytes long. */ @@ -47,7 +48,7 @@ __THROW __attribute_malloc__ __wur; the same pointer that was passed to it, aliasing needs to be allowed between objects pointed by the old and new pointers. */ extern void *realloc (void *__ptr, size_t __size) -__THROW __attribute_warn_unused_result__; +__THROW __attribute_warn_unused_result__ __attribute_alloc_size__ ((2)); /* Re-allocate the previously allocated block in PTR, making the new block large enough for NMEMB elements of SIZE bytes each. */ @@ -55,21 +56,23 @@ __THROW __attribute_warn_unused_result__; 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__; +__THROW __attribute_warn_unused_result__ __attribute_alloc_size__ ((2, 3)); /* Free a block allocated by `malloc', `realloc' or `calloc'. */ extern void free (void *__ptr) __THROW; /* Allocate SIZE bytes allocated to ALIGNMENT bytes. */ extern void *memalign (size_t __alignment, size_t __size) -__THROW __attribute_malloc__ __wur; +__THROW __attribute_malloc__ __attribute_alloc_size__ ((2)) __wur; /* Allocate SIZE bytes on a page boundary. */ -extern void *valloc (size_t __size) __THROW __attribute_malloc__ __wur; +extern void *valloc (size_t __size) __THROW __attribute_malloc__ + __attribute_alloc_size__ ((1)) __wur; /* Equivalent to valloc(minimum-page-that-holds(n)), that is, round up __size to nearest pagesize. */ -extern void *pvalloc (size_t __size) __THROW __attribute_malloc__ __wur; +extern void *pvalloc (size_t __size) __THROW __attribute_malloc__ + __attribute_alloc_size__ ((1)) __wur; /* Underlying allocation function; successive calls should return contiguous pieces of memory. */ diff --git a/malloc/tst-malloc-too-large.c b/malloc/tst-malloc-too-large.c index 10fb136528..7e4721b78c 100644 --- a/malloc/tst-malloc-too-large.c +++ b/malloc/tst-malloc-too-large.c @@ -226,6 +226,16 @@ do_test (void) test_large_aligned_allocations (SIZE_MAX - i); } + /* Allocation larger than PTRDIFF_MAX does play well with C standard, + since pointer substraction within the object might overflow ptrdiff_t + resulting in undefined behaviour. To prevent it malloc function fail + for such allocations. */ + for (size_t i = 0; i <= FOURTEEN_ON_BITS; i++) + { + test_large_allocations (PTRDIFF_MAX + i); + test_large_aligned_allocations (PTRDIFF_MAX + i); + } + #if __WORDSIZE >= 64 /* On 64-bit targets, we need to test a much wider range of too-large sizes, so we test at intervals of (1 << 50) that allocation sizes diff --git a/malloc/tst-memalign.c b/malloc/tst-memalign.c index 904bf9491d..3cba6dc7d3 100644 --- a/malloc/tst-memalign.c +++ b/malloc/tst-memalign.c @@ -21,6 +21,7 @@ #include #include #include +#include static int errors = 0; @@ -41,9 +42,18 @@ do_test (void) errno = 0; + DIAG_PUSH_NEEDS_COMMENT; +#if __GNUC_PREREQ (7, 0) + /* GCC 7 warns about too-large allocations; here we want to test + that they fail. */ + DIAG_IGNORE_NEEDS_COMMENT (7, "-Walloc-size-larger-than="); +#endif /* An attempt to allocate a huge value should return NULL and set errno to ENOMEM. */ p = memalign (sizeof (void *), -1); +#if __GNUC_PREREQ (7, 0) + DIAG_POP_NEEDS_COMMENT; +#endif save = errno; diff --git a/malloc/tst-posix_memalign.c b/malloc/tst-posix_memalign.c index 81b4c05ef4..5c4188a7ac 100644 --- a/malloc/tst-posix_memalign.c +++ b/malloc/tst-posix_memalign.c @@ -21,6 +21,7 @@ #include #include #include +#include static int errors = 0; @@ -41,9 +42,18 @@ do_test (void) p = NULL; + DIAG_PUSH_NEEDS_COMMENT; +#if __GNUC_PREREQ (7, 0) + /* GCC 7 warns about too-large allocations; here we want to test + that they fail. */ + DIAG_IGNORE_NEEDS_COMMENT (7, "-Walloc-size-larger-than="); +#endif /* An attempt to allocate a huge value should return ENOMEM and p should remain NULL. */ ret = posix_memalign (&p, sizeof (void *), -1); +#if __GNUC_PREREQ (7, 0) + DIAG_POP_NEEDS_COMMENT; +#endif if (ret != ENOMEM) merror ("posix_memalign (&p, sizeof (void *), -1) succeeded."); diff --git a/malloc/tst-pvalloc.c b/malloc/tst-pvalloc.c index aa391447ee..cd3b46d68b 100644 --- a/malloc/tst-pvalloc.c +++ b/malloc/tst-pvalloc.c @@ -21,6 +21,7 @@ #include #include #include +#include static int errors = 0; @@ -41,9 +42,18 @@ do_test (void) errno = 0; + DIAG_PUSH_NEEDS_COMMENT; +#if __GNUC_PREREQ (7, 0) + /* GCC 7 warns about too-large allocations; here we want to test + that they fail. */ + DIAG_IGNORE_NEEDS_COMMENT (7, "-Walloc-size-larger-than="); +#endif /* An attempt to allocate a huge value should return NULL and set errno to ENOMEM. */ p = pvalloc (-1); +#if __GNUC_PREREQ (7, 0) + DIAG_POP_NEEDS_COMMENT; +#endif save = errno; diff --git a/malloc/tst-reallocarray.c b/malloc/tst-reallocarray.c index b0d80ebc58..784f77e7b0 100644 --- a/malloc/tst-reallocarray.c +++ b/malloc/tst-reallocarray.c @@ -20,6 +20,23 @@ #include #include #include +#include + +static void * +reallocarray_nowarn (void *ptr, size_t nmemb, size_t size) +{ +#if __GNUC_PREREQ (7, 0) + /* GCC 7 warns about too-large allocations; here we want to test + that they fail. */ + DIAG_IGNORE_NEEDS_COMMENT (7, "-Walloc-size-larger-than="); +#endif + void *ret = reallocarray (ptr, nmemb, size); +#if __GNUC_PREREQ (7, 0) + DIAG_POP_NEEDS_COMMENT; +#endif + return ret; +} + static int do_test (void) @@ -34,24 +51,24 @@ do_test (void) /* Test overflow detection. */ errno = 0; - ptr = reallocarray (NULL, max, 2); + ptr = reallocarray_nowarn (NULL, max, 2); TEST_VERIFY (!ptr); TEST_VERIFY (errno == ENOMEM); errno = 0; - ptr = reallocarray (NULL, 2, max); + ptr = reallocarray_nowarn (NULL, 2, max); TEST_VERIFY (!ptr); TEST_VERIFY (errno == ENOMEM); a = 65537; b = max/65537 + 1; errno = 0; - ptr = reallocarray (NULL, a, b); + ptr = reallocarray_nowarn (NULL, a, b); TEST_VERIFY (!ptr); TEST_VERIFY (errno == ENOMEM); errno = 0; - ptr = reallocarray (NULL, b, a); + ptr = reallocarray_nowarn (NULL, b, a); TEST_VERIFY (!ptr); TEST_VERIFY (errno == ENOMEM); @@ -97,7 +114,7 @@ do_test (void) /* Overflow should leave buffer untouched. */ errno = 0; - ptr2 = reallocarray (ptr, 2, ~(size_t)0); + ptr2 = reallocarray_nowarn (ptr, 2, ~(size_t)0); TEST_VERIFY (!ptr2); TEST_VERIFY (errno == ENOMEM); diff --git a/malloc/tst-valloc.c b/malloc/tst-valloc.c index ba57d9559a..b37edb6c74 100644 --- a/malloc/tst-valloc.c +++ b/malloc/tst-valloc.c @@ -21,6 +21,7 @@ #include #include #include +#include static int errors = 0; @@ -41,9 +42,18 @@ do_test (void) errno = 0; + DIAG_PUSH_NEEDS_COMMENT; +#if __GNUC_PREREQ (7, 0) + /* GCC 7 warns about too-large allocations; here we want to test + that they fail. */ + DIAG_IGNORE_NEEDS_COMMENT (7, "-Walloc-size-larger-than="); +#endif /* An attempt to allocate a huge value should return NULL and set errno to ENOMEM. */ p = valloc (-1); +#if __GNUC_PREREQ (7, 0) + DIAG_POP_NEEDS_COMMENT; +#endif save = errno; diff --git a/stdlib/stdlib.h b/stdlib/stdlib.h index 870e02d904..303c90ff11 100644 --- a/stdlib/stdlib.h +++ b/stdlib/stdlib.h @@ -536,10 +536,11 @@ extern int lcong48_r (unsigned short int __param[7], #endif /* Use misc or X/Open. */ /* Allocate SIZE bytes of memory. */ -extern void *malloc (size_t __size) __THROW __attribute_malloc__ __wur; +extern void *malloc (size_t __size) __THROW __attribute_malloc__ + __attribute_alloc_size__ ((1)) __wur; /* Allocate NMEMB elements of SIZE bytes each, all initialized to 0. */ extern void *calloc (size_t __nmemb, size_t __size) - __THROW __attribute_malloc__ __wur; + __THROW __attribute_malloc__ __attribute_alloc_size__ ((1, 2)) __wur; /* Re-allocate the previously allocated block in PTR, making the new block SIZE bytes long. */ @@ -547,7 +548,7 @@ extern void *calloc (size_t __nmemb, size_t __size) the same pointer that was passed to it, aliasing needs to be allowed between objects pointed by the old and new pointers. */ extern void *realloc (void *__ptr, size_t __size) - __THROW __attribute_warn_unused_result__; + __THROW __attribute_warn_unused_result__ __attribute_alloc_size__ ((2)); #ifdef __USE_MISC /* Re-allocate the previously allocated block in PTR, making the new @@ -556,7 +557,8 @@ extern void *realloc (void *__ptr, size_t __size) 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__; + __THROW __attribute_warn_unused_result__ + __attribute_alloc_size__ ((2, 3)); #endif /* Free a block allocated by `malloc', `realloc' or `calloc'. */ @@ -569,13 +571,14 @@ extern void free (void *__ptr) __THROW; #if (defined __USE_XOPEN_EXTENDED && !defined __USE_XOPEN2K) \ || defined __USE_MISC /* Allocate SIZE bytes on a page boundary. The storage cannot be freed. */ -extern void *valloc (size_t __size) __THROW __attribute_malloc__ __wur; +extern void *valloc (size_t __size) __THROW __attribute_malloc__ + __attribute_alloc_size__ ((1)) __wur; #endif #ifdef __USE_XOPEN2K /* Allocate memory of SIZE bytes with an alignment of ALIGNMENT. */ extern int posix_memalign (void **__memptr, size_t __alignment, size_t __size) - __THROW __nonnull ((1)) __wur; + __THROW __nonnull ((1)) __attribute_alloc_size__ ((3)) __wur; #endif #ifdef __USE_ISOC11