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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id F3E54C54EE9 for ; Tue, 13 Sep 2022 08:53:35 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8E0D28D0001; Tue, 13 Sep 2022 04:53:35 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 890116B0073; Tue, 13 Sep 2022 04:53:35 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 758E38D0001; Tue, 13 Sep 2022 04:53:35 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 66F706B0072 for ; Tue, 13 Sep 2022 04:53:35 -0400 (EDT) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 359B31A0D03 for ; Tue, 13 Sep 2022 08:53:35 +0000 (UTC) X-FDA: 79906448790.11.BE2C1DD Received: from mail-pj1-f51.google.com (mail-pj1-f51.google.com [209.85.216.51]) by imf14.hostedemail.com (Postfix) with ESMTP id E7C83100098 for ; Tue, 13 Sep 2022 08:53:33 +0000 (UTC) Received: by mail-pj1-f51.google.com with SMTP id o70-20020a17090a0a4c00b00202f898fa86so1239073pjo.2 for ; Tue, 13 Sep 2022 01:53:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date; bh=4TPm3yDLRTUXMkcwAeJgiJwg94hbEBwyJGZBnuCnHu4=; b=R/7fEVh0ld47AI+yT4mpK9pSrSlHMlxfAe2/cVTTVBOpEIa4h4GNW+xXfhIzk3QTX+ 6Qlti3fMhSlMHd5FPjj1MFSvMzShflGSaoaJDomDDyPLN6ias0+5ctiuvSZyRtxdd6U3 bVTG7ncvWs/FnrrnyPUAqKq8mfsYV40lfY8oIfdOn5TYX386djn8Uzp7brER/fDfPPWT wVEcmHHguOV00Tr+9cbxRBr+n0fCbPZzTRhadih2qoAIgpj3QLoUTUFec8BEVkRjOhsB eEbs1HrKHjukPKAakVy4LzfHc4vxPB/Qf+5ZqwaQsamDXkklSA8VbmeqDvCx6SXtl3hK F0fA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date; bh=4TPm3yDLRTUXMkcwAeJgiJwg94hbEBwyJGZBnuCnHu4=; b=HhCBdIuXhHpePBaPtU50UhU4CjI5t8j0A/5brWXI950HDT7iuZ7Itltz50sK9dMfHs IYkgvluhvwR6i8EAwDayWEvGRTu3W63gZsasukUT12Nb2aCFXH/OwJHRhlUpF6uT5+Vc 4Be2pCuUp6qmNn9rFnzIrcqTURmne1aaMOOGbwiglzU4JYTPlODSdH7bxz+74X9Nj6L2 YXK9DhTS50VoIIOjPcDr2oHx+XYo02l2CcYwqd8fFCkIu4cq7vn/Cej1kOZRnyQd5g6V fMeZ0RMRAMd273Q7dk1k3mYsH4BfAFM1Br1lpXkbSZkBBbuMtcto3sJE5Rk2qnbAfNev P3SQ== X-Gm-Message-State: ACgBeo38oLaiTLtKVxSu4Fj7hsJRb40p89hpzW3bXZSCT51dqjReGAtS lj50jpR7dz+vsG7GtbiZjRw= X-Google-Smtp-Source: AA6agR7OkLp6prd6BnIGRKEsYjR4a4I4pGQbxWjkrYV9QYCSVPnFfcptq7+ntz6UTmplxN7h8tjqiQ== X-Received: by 2002:a17:902:d48d:b0:178:306d:f75c with SMTP id c13-20020a170902d48d00b00178306df75cmr9442090plg.73.1663059212848; Tue, 13 Sep 2022 01:53:32 -0700 (PDT) Received: from hyeyoo ([114.29.91.56]) by smtp.gmail.com with ESMTPSA id l19-20020a639853000000b0043957e4f85asm16626pgo.12.2022.09.13.01.53.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 13 Sep 2022 01:53:31 -0700 (PDT) Date: Tue, 13 Sep 2022 17:53:25 +0900 From: Hyeonggon Yoo <42.hyeyoo@gmail.com> To: Feng Tang Cc: Andrew Morton , Vlastimil Babka , Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Roman Gushchin , Dmitry Vyukov , Jonathan Corbet , Andrey Konovalov , Dave Hansen , linux-mm@kvack.org, linux-kernel@vger.kernel.org, kasan-dev@googlegroups.com Subject: Re: [PATCH v6 4/4] mm/slub: extend redzone check to extra allocated kmalloc space than requested Message-ID: References: <20220913065423.520159-1-feng.tang@intel.com> <20220913065423.520159-5-feng.tang@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220913065423.520159-5-feng.tang@intel.com> ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1663059213; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=4TPm3yDLRTUXMkcwAeJgiJwg94hbEBwyJGZBnuCnHu4=; b=D0wCEX8mbWm2GIS8cupnCDtodWGFDqTAwIhFnbZw/ToKidlz/5PT0N8UE7g5TSytlaKIWw Qgl5o7nYx6Sxnl6XXiZcmP1UE4YF5B9Z7b+8aspC5olJJDu0xCpahp6Xf040YTe1xTmWvV QNVM+2uTEsXQLwKg75DCTqGse4DC3kg= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b="R/7fEVh0"; spf=pass (imf14.hostedemail.com: domain of 42.hyeyoo@gmail.com designates 209.85.216.51 as permitted sender) smtp.mailfrom=42.hyeyoo@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1663059213; a=rsa-sha256; cv=none; b=A2x1mGD2hJnKUbObf3pO1GA2qsRWZhtbtO0CiKQJ8936TV/DVqy+kW4vfzPbKPN/V1CvlR 7obyIY2nEhFaAtL8Yr5NyABltUTQbEffDpVsjCwTpxtCJ05sXly7ClYz3oGFCz6Lol3lhm b8uhrZeyvAvAu4bowrnitb4vZD0UfYY= X-Stat-Signature: hygmyq1yci1m87q1fx3tbyqocyky4w4c X-Rspamd-Queue-Id: E7C83100098 Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b="R/7fEVh0"; spf=pass (imf14.hostedemail.com: domain of 42.hyeyoo@gmail.com designates 209.85.216.51 as permitted sender) smtp.mailfrom=42.hyeyoo@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-Rspam-User: X-Rspamd-Server: rspam09 X-HE-Tag: 1663059213-6171 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 Tue, Sep 13, 2022 at 02:54:23PM +0800, Feng Tang wrote: > kmalloc will round up the request size to a fixed size (mostly power > of 2), so there could be a extra space than what is requested, whose > size is the actual buffer size minus original request size. > > To better detect out of bound access or abuse of this space, add > redzone sanity check for it. > > And in current kernel, some kmalloc user already knows the existence > of the space and utilizes it after calling 'ksize()' to know the real > size of the allocated buffer. So we skip the sanity check for objects > which have been called with ksize(), as treating them as legitimate > users. > > In some cases, the free pointer could be saved inside the latter > part of object data area, which may overlap the redzone part(for > small sizes of kmalloc objects). As suggested by Hyeonggon Yoo, > force the free pointer to be in meta data area when kmalloc redzone > debug is enabled, to make all kmalloc objects covered by redzone > check. > > Suggested-by: Vlastimil Babka > Signed-off-by: Feng Tang > --- > mm/slab.h | 4 ++++ > mm/slab_common.c | 4 ++++ > mm/slub.c | 51 ++++++++++++++++++++++++++++++++++++++++++++---- > 3 files changed, 55 insertions(+), 4 deletions(-) > > diff --git a/mm/slab.h b/mm/slab.h > index 3cf5adf63f48..5ca04d9c8bf5 100644 > --- a/mm/slab.h > +++ b/mm/slab.h > @@ -881,4 +881,8 @@ void __check_heap_object(const void *ptr, unsigned long n, > } > #endif > > +#ifdef CONFIG_SLUB_DEBUG > +void skip_orig_size_check(struct kmem_cache *s, const void *object); > +#endif > + > #endif /* MM_SLAB_H */ > diff --git a/mm/slab_common.c b/mm/slab_common.c > index 8e13e3aac53f..5106667d6adb 100644 > --- a/mm/slab_common.c > +++ b/mm/slab_common.c > @@ -1001,6 +1001,10 @@ size_t __ksize(const void *object) > return folio_size(folio); > } > > +#ifdef CONFIG_SLUB_DEBUG > + skip_orig_size_check(folio_slab(folio)->slab_cache, object); > +#endif > + > return slab_ksize(folio_slab(folio)->slab_cache); > } > > diff --git a/mm/slub.c b/mm/slub.c > index 6f823e99d8b4..546b30ed5afd 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -812,12 +812,28 @@ static inline void set_orig_size(struct kmem_cache *s, > if (!slub_debug_orig_size(s)) > return; > > +#ifdef CONFIG_KASAN_GENERIC > + /* > + * KASAN could save its free meta data in object's data area at > + * offset 0, if the size is larger than 'orig_size', it could > + * overlap the data redzone(from 'orig_size+1' to 'object_size'), > + * where the check should be skipped. > + */ > + if (s->kasan_info.free_meta_size > orig_size) > + orig_size = s->object_size; > +#endif > + > p += get_info_end(s); > p += sizeof(struct track) * 2; > > *(unsigned int *)p = orig_size; > } > > +void skip_orig_size_check(struct kmem_cache *s, const void *object) > +{ > + set_orig_size(s, (void *)object, s->object_size); > +} > + > static unsigned int get_orig_size(struct kmem_cache *s, void *object) > { > void *p = kasan_reset_tag(object); > @@ -949,13 +965,27 @@ static __printf(3, 4) void slab_err(struct kmem_cache *s, struct slab *slab, > static void init_object(struct kmem_cache *s, void *object, u8 val) > { > u8 *p = kasan_reset_tag(object); > + unsigned int orig_size = s->object_size; > > - if (s->flags & SLAB_RED_ZONE) > + if (s->flags & SLAB_RED_ZONE) { > memset(p - s->red_left_pad, val, s->red_left_pad); > > + if (slub_debug_orig_size(s) && val == SLUB_RED_ACTIVE) { > + orig_size = get_orig_size(s, object); > + > + /* > + * Redzone the extra allocated space by kmalloc > + * than requested. > + */ > + if (orig_size < s->object_size) > + memset(p + orig_size, val, > + s->object_size - orig_size); > + } > + } > + > if (s->flags & __OBJECT_POISON) { > - memset(p, POISON_FREE, s->object_size - 1); > - p[s->object_size - 1] = POISON_END; > + memset(p, POISON_FREE, orig_size - 1); > + p[orig_size - 1] = POISON_END; > } > > if (s->flags & SLAB_RED_ZONE) > @@ -1103,6 +1133,7 @@ static int check_object(struct kmem_cache *s, struct slab *slab, > { > u8 *p = object; > u8 *endobject = object + s->object_size; > + unsigned int orig_size; > > if (s->flags & SLAB_RED_ZONE) { > if (!check_bytes_and_report(s, slab, object, "Left Redzone", > @@ -1112,6 +1143,17 @@ static int check_object(struct kmem_cache *s, struct slab *slab, > if (!check_bytes_and_report(s, slab, object, "Right Redzone", > endobject, val, s->inuse - s->object_size)) > return 0; > + > + if (slub_debug_orig_size(s) && val == SLUB_RED_ACTIVE) { > + orig_size = get_orig_size(s, object); > + > + if (s->object_size > orig_size && > + !check_bytes_and_report(s, slab, object, > + "kmalloc Redzone", p + orig_size, > + val, s->object_size - orig_size)) { > + return 0; > + } > + } > } else { > if ((s->flags & SLAB_POISON) && s->object_size < s->inuse) { > check_bytes_and_report(s, slab, p, "Alignment padding", > @@ -4187,7 +4229,8 @@ static int calculate_sizes(struct kmem_cache *s) > */ > s->inuse = size; > > - if ((flags & (SLAB_TYPESAFE_BY_RCU | SLAB_POISON)) || > + if (slub_debug_orig_size(s) || > + (flags & (SLAB_TYPESAFE_BY_RCU | SLAB_POISON)) || > ((flags & SLAB_RED_ZONE) && s->object_size < sizeof(void *)) || > s->ctor) { > /* > -- > 2.34.1 > For the slab part: Looks good to me. Acked-by: Hyeonggon Yoo <42.hyeyoo@gmail.com> Thanks! -- Thanks, Hyeonggon