From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-17.4 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7B7FCC43457 for ; Fri, 9 Oct 2020 12:12:50 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id C0343222B8 for ; Fri, 9 Oct 2020 12:12:49 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="UTyO88Bj" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C0343222B8 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 2AACA6B0068; Fri, 9 Oct 2020 08:12:49 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 25D496B006C; Fri, 9 Oct 2020 08:12:49 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 199186B006E; Fri, 9 Oct 2020 08:12:49 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0160.hostedemail.com [216.40.44.160]) by kanga.kvack.org (Postfix) with ESMTP id E22506B0068 for ; Fri, 9 Oct 2020 08:12:48 -0400 (EDT) Received: from smtpin13.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 87CA9181AE867 for ; Fri, 9 Oct 2020 12:12:48 +0000 (UTC) X-FDA: 77352275616.13.women33_4a04882271e0 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin13.hostedemail.com (Postfix) with ESMTP id B630518140B69 for ; Fri, 9 Oct 2020 12:12:46 +0000 (UTC) X-HE-Tag: women33_4a04882271e0 X-Filterd-Recvd-Size: 7353 Received: from mail-ot1-f66.google.com (mail-ot1-f66.google.com [209.85.210.66]) by imf17.hostedemail.com (Postfix) with ESMTP for ; Fri, 9 Oct 2020 12:12:46 +0000 (UTC) Received: by mail-ot1-f66.google.com with SMTP id n61so8747043ota.10 for ; Fri, 09 Oct 2020 05:12:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=vglF5TgelpcXWuhwbhmz3Tg260d8h9z/0FEQQjRk5vw=; b=UTyO88Bjpy8t9kBwxo/swiOkD1L6J1kBQ9GDBVYxyL/REnT78PO20a2P9CCqTLJ5du YKamn7VgJK8zBd7ObqFgEG1n2orYT+a2OyWzufbja7ebUkVpRyKu2PgojxPgIrELMr8e kludZFOOnFPXSa48JL0H9+A1WozKeALMfXbVdlnrXqjbPvLPVGIACtcJE/MuwhM5x52D EmvvBzQ2/FTePghmV73M5V02JB0rcu2SBD7GNbMMtZWEz3oFZehmP7IccR9vmfb1h649 pFlQnW/kChNLHDc+9lKdy3Huao8U2qvnq+VmYbYauc3FIFqo0VqKNvCWHRpJA4bUFpi5 aAzg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=vglF5TgelpcXWuhwbhmz3Tg260d8h9z/0FEQQjRk5vw=; b=ixbKtsfPh82iIj+8S/Xp8EIRP8e7DltyGmGIh/jl5AQ05+pQkHVd/JP7GpiuYW1SCF R95dRSafXNDtQqKPNGpt+tDop7ak1rfIqn28AE/Vf3Q6DJ6sbsFjrVQQjL87HUKwl4Vy AxlGsy4XU0Af3aB7uxiHjnfDfdkKM/tWP/XRZmgqzKJpekKLn5IDJFd2F/a1Gus6ZxF1 zuIiz1NPdedS3cvGhCX0XWkINna6fTJ8cNyqW2stu2wEP971jjmUUj9PcUGhKUQKCKEJ 6hGqSgaNgbiQORdD3RXMNEiB3ivRZ8O1DOetXaBoFYCL9r44Dgu65YIi8lo7xGVCMCP6 /djA== X-Gm-Message-State: AOAM532Q0ov1HYoA0CZ4lJYUgschnLodIvmM5lqu39ZpEXmxFCvhh6wz zzV/TI5DZkIDpeMylBue5aeVe5pz1HDp1vJde961Dg== X-Google-Smtp-Source: ABdhPJynIFpoV4/xv/4/DqIHyB4TgoRXIVbijDMjFudDDNIymFhrX/mL2+SMq02IXJZWSbKGCJiwYbUdjQawdAIzNtI= X-Received: by 2002:a9d:34d:: with SMTP id 71mr7876128otv.251.1602245565330; Fri, 09 Oct 2020 05:12:45 -0700 (PDT) MIME-Version: 1.0 References: <20201008233443.3335464-1-keescook@chromium.org> In-Reply-To: <20201008233443.3335464-1-keescook@chromium.org> From: Marco Elver Date: Fri, 9 Oct 2020 14:12:34 +0200 Message-ID: Subject: Re: [PATCH] slub: Actually fix freelist pointer vs redzoning To: Kees Cook Cc: Andrew Morton , stable@vger.kernel.org, Pekka Enberg , Waiman Long , Christoph Lameter , David Rientjes , Joonsoo Kim , LKML , Linux Memory Management List Content-Type: text/plain; charset="UTF-8" X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Fri, 9 Oct 2020 at 01:34, Kees Cook wrote: > It turns out that SLUB redzoning ("slub_debug=Z") checks from > s->object_size rather than from s->inuse (which is normally bumped to > make room for the freelist pointer), so a cache created with an object > size less than 24 would have their freelist pointer written beyond > s->object_size, causing the redzone to corrupt the freelist pointer. > This was very visible with "slub_debug=ZF": > > BUG test (Tainted: G B ): Redzone overwritten > ----------------------------------------------------------------------------- > > INFO: 0xffff957ead1c05de-0xffff957ead1c05df @offset=1502. First byte 0x1a instead of 0xbb > INFO: Slab 0xffffef3950b47000 objects=170 used=170 fp=0x0000000000000000 flags=0x8000000000000200 > INFO: Object 0xffff957ead1c05d8 @offset=1496 fp=0xffff957ead1c0620 > > Redzone (____ptrval____): bb bb bb bb bb bb bb bb ........ > Object (____ptrval____): 00 00 00 00 00 f6 f4 a5 ........ > Redzone (____ptrval____): 40 1d e8 1a aa @.... > Padding (____ptrval____): 00 00 00 00 00 00 00 00 ........ > > Adjust the offset to stay within s->object_size. > > (Note that there appear to be no such small-sized caches in the kernel > currently.) > > Reported-by: Marco Elver > Link: https://lore.kernel.org/linux-mm/20200807160627.GA1420741@elver.google.com/ > Fixes: 89b83f282d8b (slub: avoid redzone when choosing freepointer location) > Cc: stable@vger.kernel.org > Signed-off-by: Kees Cook > --- > mm/slub.c | 17 +++++------------ > 1 file changed, 5 insertions(+), 12 deletions(-) Tested-by: Marco Elver Thank you! > diff --git a/mm/slub.c b/mm/slub.c > index 68c02b2eecd9..979f5da26992 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -3641,7 +3641,6 @@ static int calculate_sizes(struct kmem_cache *s, int forced_order) > { > slab_flags_t flags = s->flags; > unsigned int size = s->object_size; > - unsigned int freepointer_area; > unsigned int order; > > /* > @@ -3650,13 +3649,6 @@ static int calculate_sizes(struct kmem_cache *s, int forced_order) > * the possible location of the free pointer. > */ > size = ALIGN(size, sizeof(void *)); > - /* > - * This is the area of the object where a freepointer can be > - * safely written. If redzoning adds more to the inuse size, we > - * can't use that portion for writing the freepointer, so > - * s->offset must be limited within this for the general case. > - */ > - freepointer_area = size; > > #ifdef CONFIG_SLUB_DEBUG > /* > @@ -3682,7 +3674,7 @@ static int calculate_sizes(struct kmem_cache *s, int forced_order) > > /* > * With that we have determined the number of bytes in actual use > - * by the object. This is the potential offset to the free pointer. > + * by the object and redzoning. > */ > s->inuse = size; > > @@ -3694,7 +3686,8 @@ static int calculate_sizes(struct kmem_cache *s, int forced_order) > * kmem_cache_free. > * > * This is the case if we do RCU, have a constructor or > - * destructor or are poisoning the objects. > + * destructor, are poisoning the objects, or are > + * redzoning an object smaller than sizeof(void *). > * > * The assumption that s->offset >= s->inuse means free > * pointer is outside of the object is used in the > @@ -3703,13 +3696,13 @@ static int calculate_sizes(struct kmem_cache *s, int forced_order) > */ > s->offset = size; > size += sizeof(void *); > - } else if (freepointer_area > sizeof(void *)) { > + } else { > /* > * Store freelist pointer near middle of object to keep > * it away from the edges of the object to avoid small > * sized over/underflows from neighboring allocations. > */ > - s->offset = ALIGN(freepointer_area / 2, sizeof(void *)); > + s->offset = ALIGN_DOWN(s->object_size / 2, sizeof(void *)); > } > > #ifdef CONFIG_SLUB_DEBUG > -- > 2.25.1 >