From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3941 invoked by alias); 12 Nov 2018 12:42:27 -0000 Mailing-List: contact libc-stable-help@sourceware.org; run by ezmlm Precedence: bulk List-Post: List-Help: List-Subscribe: List-Archive: Sender: libc-stable-owner@sourceware.org Received: (qmail 3536 invoked by uid 89); 12 Nov 2018 12:42:26 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Checked: by ClamAV 0.100.2 on sourceware.org X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.9 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 spammy=footer, integrity, measures X-Spam-Status: No, score=-24.9 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on sourceware.org X-Spam-Level: X-HELO: mail-qk1-f181.google.com Received: from mail-qk1-f181.google.com (HELO mail-qk1-f181.google.com) (209.85.222.181) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 12 Nov 2018 12:42:25 +0000 Received: by mail-qk1-f181.google.com with SMTP id 131so12954937qkd.4 for ; Mon, 12 Nov 2018 04:42:25 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:subject:openpgp:organization:to:message-id :date:user-agent:mime-version:content-language; bh=nueAkHcbayJlSB8bLT57ihlKQafu2hYoodaTd1qIBGU=; b=qOxqBuAT6qzAWDfQlUuSdkIRDrwfElC8Y6HxSlcFJnPomJOdfjlOE7TjFlx4CiTTVd T13AIMWag5CM5NV6dUngr1aVQSKlhUj6Ps5OEqCUMlSFlDY09j8KBTdKskGyVZC2vRnK Vi0t+pfnNJeIA0zOdosEL8UzG51j2RM3MDOETwRg2BJHnLzX9TSaLnQaqM0LQkjqyVDQ 8TbNDpZZBa17eSvIr9rtHl5I7KbA+w14ObO2j4qV5FnmHnByhvnbMNIH+V4BQp1NfCqc g6rXtEFbkjJgaITQGaBP5i7QjWZ4YCeyQ9C3oxruUchTVUiimMmxpK7T+7ukshg6VzVs jMJw== X-Gm-Message-State: AGRZ1gIJFebH7i++uHEzUvxtEF9ASj4vUJCzSsVj/sqjqjuVco9NPznd XUOKMXKxft40OZlZmHDaJGMFsYsf1ZEyIA== X-Google-Smtp-Source: AJdET5dUDS25C2PAKgzhjQviSLCL5z7wBgLFeACAJmkZvK2RjnXKxVjzrXyaNeHv/JiX/PJip3PlQg== X-Received: by 2002:aed:22c6:: with SMTP id q6mr712992qtc.145.1542026543425; Mon, 12 Nov 2018 04:42:23 -0800 (PST) Received: from [10.150.73.190] (97.sub-174-227-17.myvzw.com. [174.227.17.97]) by smtp.gmail.com with ESMTPSA id o2sm7706136qtp.48.2018.11.12.04.42.22 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 12 Nov 2018 04:42:22 -0800 (PST) From: Carlos O'Donell Subject: [2.28 COMMITTED] malloc: Additional checks for unsorted bin integrity I. Openpgp: preference=signencrypt Organization: Red Hat To: "GNU C Library (Stable)" Message-ID: <3369ad5f-566b-6e83-288a-30398eb83d4f@redhat.com> Date: Mon, 01 Jan 2018 00:00:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="------------E3E3575B6966F9A9E1B7EA4E" Content-Language: en-US X-SW-Source: 2018-11/txt/msg00019.txt.bz2 This is a multi-part message in MIME format. --------------E3E3575B6966F9A9E1B7EA4E Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Content-length: 548 Tested on x86_64. build-many-glibcs run in progress. -- Ensure the following properties of chunks encountered during binning: - victim chunk has reasonable size - next chunk has reasonable size - next->prev_size == victim->size - valid double linked list - PREV_INUSE of next chunk is unset * malloc/malloc.c (_int_malloc): Additional binning code checks. (cherry picked from commit b90ddd08f6dd688e651df9ee89ca3a69ff88cd0c) --- malloc/malloc.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) -- Cheers, Carlos. --------------E3E3575B6966F9A9E1B7EA4E Content-Type: text/x-patch; name="0004-malloc-Additional-checks-for-unsorted-bin-integrity-.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0004-malloc-Additional-checks-for-unsorted-bin-integrity-.pa"; filename*1="tch" Content-length: 5294 >From 53a7e59405cbbbd24c1cf64b0298a9e6212a82e2 Mon Sep 17 00:00:00 2001 From: Istvan Kurucsai Date: Tue, 16 Jan 2018 14:54:32 +0100 Subject: [PATCH 4/8] malloc: Additional checks for unsorted bin integrity I. On Thu, Jan 11, 2018 at 3:50 PM, Florian Weimer wrote: > On 11/07/2017 04:27 PM, Istvan Kurucsai wrote: >> >> + next = chunk_at_offset (victim, size); > > > For new code, we prefer declarations with initializers. Noted. >> + if (__glibc_unlikely (chunksize_nomask (victim) <= 2 * SIZE_SZ) >> + || __glibc_unlikely (chunksize_nomask (victim) > >> av->system_mem)) >> + malloc_printerr("malloc(): invalid size (unsorted)"); >> + if (__glibc_unlikely (chunksize_nomask (next) < 2 * SIZE_SZ) >> + || __glibc_unlikely (chunksize_nomask (next) > >> av->system_mem)) >> + malloc_printerr("malloc(): invalid next size (unsorted)"); >> + if (__glibc_unlikely ((prev_size (next) & ~(SIZE_BITS)) != >> size)) >> + malloc_printerr("malloc(): mismatching next->prev_size >> (unsorted)"); > > > I think this check is redundant because prev_size (next) and chunksize > (victim) are loaded from the same memory location. I'm fairly certain that it compares mchunk_size of victim against mchunk_prev_size of the next chunk, i.e. the size of victim in its header and footer. >> + if (__glibc_unlikely (bck->fd != victim) >> + || __glibc_unlikely (victim->fd != unsorted_chunks (av))) >> + malloc_printerr("malloc(): unsorted double linked list >> corrupted"); >> + if (__glibc_unlikely (prev_inuse(next))) >> + malloc_printerr("malloc(): invalid next->prev_inuse >> (unsorted)"); > > > There's a missing space after malloc_printerr. Noted. > Why do you keep using chunksize_nomask? We never investigated why the > original code uses it. It may have been an accident. You are right, I don't think it makes a difference in these checks. So the size local can be reused for the checks against victim. For next, leaving it as such avoids the masking operation. > Again, for non-main arenas, the checks against av->system_mem could be made > tighter (against the heap size). Maybe you could put the condition into a > separate inline function? We could also do a chunk boundary check similar to what I proposed in the thread for the first patch in the series to be even more strict. I'll gladly try to implement either but believe that refining these checks would bring less benefits than in the case of the top chunk. Intra-arena or intra-heap overlaps would still be doable here with unsorted chunks and I don't see any way to counter that besides more generic measures like randomizing allocations and your metadata encoding patches. I've attached a revised version with the above comments incorporated but without the refined checks. Thanks, Istvan >From a12d5d40fd7aed5fa10fc444dcb819947b72b315 Mon Sep 17 00:00:00 2001 From: Istvan Kurucsai Date: Tue, 16 Jan 2018 14:48:16 +0100 Subject: [PATCH v2 1/1] malloc: Additional checks for unsorted bin integrity I. Ensure the following properties of chunks encountered during binning: - victim chunk has reasonable size - next chunk has reasonable size - next->prev_size == victim->size - valid double linked list - PREV_INUSE of next chunk is unset * malloc/malloc.c (_int_malloc): Additional binning code checks. (cherry picked from commit b90ddd08f6dd688e651df9ee89ca3a69ff88cd0c) --- malloc/malloc.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/malloc/malloc.c b/malloc/malloc.c index 7c8bf8413c..47795601c8 100644 --- a/malloc/malloc.c +++ b/malloc/malloc.c @@ -3716,11 +3716,22 @@ _int_malloc (mstate av, size_t bytes) while ((victim = unsorted_chunks (av)->bk) != unsorted_chunks (av)) { bck = victim->bk; - if (__builtin_expect (chunksize_nomask (victim) <= 2 * SIZE_SZ, 0) - || __builtin_expect (chunksize_nomask (victim) - > av->system_mem, 0)) - malloc_printerr ("malloc(): memory corruption"); size = chunksize (victim); + mchunkptr next = chunk_at_offset (victim, size); + + if (__glibc_unlikely (size <= 2 * SIZE_SZ) + || __glibc_unlikely (size > av->system_mem)) + malloc_printerr ("malloc(): invalid size (unsorted)"); + if (__glibc_unlikely (chunksize_nomask (next) < 2 * SIZE_SZ) + || __glibc_unlikely (chunksize_nomask (next) > av->system_mem)) + malloc_printerr ("malloc(): invalid next size (unsorted)"); + if (__glibc_unlikely ((prev_size (next) & ~(SIZE_BITS)) != size)) + malloc_printerr ("malloc(): mismatching next->prev_size (unsorted)"); + if (__glibc_unlikely (bck->fd != victim) + || __glibc_unlikely (victim->fd != unsorted_chunks (av))) + malloc_printerr ("malloc(): unsorted double linked list corrupted"); + if (__glibc_unlikely (prev_inuse(next))) + malloc_printerr ("malloc(): invalid next->prev_inuse (unsorted)"); /* If a small request, try to use last remainder if it is the -- 2.17.2 --------------E3E3575B6966F9A9E1B7EA4E--