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 9000FC19F2B for ; Thu, 4 Aug 2022 10:48:14 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id AB5FE8E0002; Thu, 4 Aug 2022 06:48:13 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A65BD8E0001; Thu, 4 Aug 2022 06:48:13 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 906538E0002; Thu, 4 Aug 2022 06:48:13 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 7F3CA8E0001 for ; Thu, 4 Aug 2022 06:48:13 -0400 (EDT) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 51533AB027 for ; Thu, 4 Aug 2022 10:48:13 +0000 (UTC) X-FDA: 79761585666.18.86663AB Received: from mail-lf1-f48.google.com (mail-lf1-f48.google.com [209.85.167.48]) by imf28.hostedemail.com (Postfix) with ESMTP id 9F682C0114 for ; Thu, 4 Aug 2022 10:48:12 +0000 (UTC) Received: by mail-lf1-f48.google.com with SMTP id m22so18412446lfl.9 for ; Thu, 04 Aug 2022 03:48:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc; bh=eegZBp28eQmH3OhuZfCVY16Y84OTpGQZtSHItuJdUI0=; b=LBXwMho0naCRAn0uLfy0/shvSkniUOYXZOse7ecIpBosuFqQK30HwYCSpjmed5m1Pk Beav35IYNT0r0MaaEh2ir6p3mNW6An4tqXR2sHJRK3H08h8vmfVuWqUvxjLUX/9k86ot kGq34sHJN/VRKF/73MVN+s1qgZrppUjyfw1yURt2rMc4BiiWNLgF6Z7RkKK7gLXEprwm RuXbEOsS+Q/ZAtzLeNAs4BRCcynwrYWZl8c1lzRBg+SVa4aoV8mecs4nC9pALmcnWRnk poVgCteiyEPAnaAUPBjznLxxQF/eLppVvXHF5Vi+kEARnKrwsil3QpqXTrykjI9V6KZB usnQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc; bh=eegZBp28eQmH3OhuZfCVY16Y84OTpGQZtSHItuJdUI0=; b=k39/EjiY1ALnIHekCRv1ZDehH+0MJXP+CLzg1Pg0cemL8kh80sxrDwHT/Gm8ZU6KY5 A+UhlombEhXLfjan7iUfe+deWk4kLzY3DozaZeleOqDi2XrQD30V30pjeRvJEMMuqSAQ o5UifpBIDsi8TT0KfhphuDARzXQmkzzh96dLrHzuMSTCt+bW0n857EI/QwhSJ8/So5Hj L0Ju8ymSRjrKF1LmBhZuJ2JvTAdxaKMPuoHEfqWKBjvcJufFDZ/VvTdCTF7Qifd+8Z1A DNA/IQbNHHcvfiX6YOml+nNt49YFmhQay9QGo5KumQLhTyQaIRO+v1T9nfN/SaTh5z2F GwIA== X-Gm-Message-State: ACgBeo3FQh/n8qY83rhMBVnW3NK5Ex9/glI3xRKh2WQ0WwsHbBNjN9o1 vaPJODojAj9wifWB1VzILglf4mCvMteEaD8NTGQAjA== X-Google-Smtp-Source: AA6agR4sbvFnha5ksT3ZlL3sUCWj6dLhquYwtG8AA19v9az2AcMHq+TuC0J8RgPecOYrD2Xa9R5XGidkqyspyIZE0PM= X-Received: by 2002:a05:6512:1086:b0:48b:27a4:5059 with SMTP id j6-20020a056512108600b0048b27a45059mr492779lfg.540.1659610090741; Thu, 04 Aug 2022 03:48:10 -0700 (PDT) MIME-Version: 1.0 References: <0e545088-d140-4c84-bbb2-a3be669740b2@suse.cz> <85ec4ea8-ae4c-3592-5491-3db6d0ad8c59@suse.cz> In-Reply-To: From: Dmitry Vyukov Date: Thu, 4 Aug 2022 12:47:58 +0200 Message-ID: Subject: Re: [mm/slub] 3616799128: BUG_kmalloc-#(Not_tainted):kmalloc_Redzone_overwritten To: Feng Tang Cc: Vlastimil Babka , "Sang, Oliver" , lkp , LKML , "linux-mm@kvack.org" , "lkp@lists.01.org" , Andrew Morton , Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Roman Gushchin , Hyeonggon Yoo <42.hyeyoo@gmail.com>, "Hansen, Dave" , Robin Murphy , John Garry , Kefeng Wang , Andrey Konovalov , Andrey Ryabinin , Alexander Potapenko , "kasan-dev@googlegroups.com" Content-Type: text/plain; charset="UTF-8" ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1659610092; 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=eegZBp28eQmH3OhuZfCVY16Y84OTpGQZtSHItuJdUI0=; b=4PnmuW9lioAHWr0p0jCicfE6hZ95YzjDaOEtrag2NXmsreOtua3zD5RxvUPNd/zojrsqvV 7NS0xwOEukwGsTAuuC+LFQOPec+cx2+9wGxJD1eXEZEJ9iS9yZ5XPuOwJrw52Wnys+aSfC DRziuueikgjF4DQavXI6MmYKEzDsK0Y= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=LBXwMho0; spf=pass (imf28.hostedemail.com: domain of dvyukov@google.com designates 209.85.167.48 as permitted sender) smtp.mailfrom=dvyukov@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1659610092; a=rsa-sha256; cv=none; b=pty4/jW73CHLGQtdUEBt7MFS7NN5fo5Mi8XAIXFRQBY9isozlsQtcd2sELDXgpb6JfESB+ aL2otta4q1QjTcWK8xNsq7sb3eie5b4T0aJX6T9h0x1vFc6e4eH21+DIQKGgRtDgi/swmy sMCyhnO7yfz+WG4xCrDyHsoovxHlba8= X-Stat-Signature: a5eyg6hb8wx1wgh6ne6919zgxoxuu5sw X-Rspam-User: X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 9F682C0114 Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=LBXwMho0; spf=pass (imf28.hostedemail.com: domain of dvyukov@google.com designates 209.85.167.48 as permitted sender) smtp.mailfrom=dvyukov@google.com; dmarc=pass (policy=reject) header.from=google.com X-HE-Tag: 1659610092-96146 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 Thu, 4 Aug 2022 at 08:29, Feng Tang wrote: > > > > .On Tue, 2 Aug 2022 at 11:43, Vlastimil Babka wrote: > > > > > > > > > > On 8/2/22 09:06, Dmitry Vyukov wrote: > > > > > > On Tue, 2 Aug 2022 at 08:55, Feng Tang wrote: > > > > > >> > > > > > >> On Mon, Aug 01, 2022 at 10:23:23PM +0800, Vlastimil Babka wrote: > > > > > >> > On 8/1/22 08:21, Feng Tang wrote: > > > > > >> [snip] > > > > > >> > > Cc kansan mail list. > > > > > >> > > > > > > > >> > > This is really related with KASAN debug, that in free path, some > > > > > >> > > kmalloc redzone ([orig_size+1, object_size]) area is written by > > > > > >> > > kasan to save free meta info. > > > > > >> > > > > > > > >> > > The callstack is: > > > > > >> > > > > > > > >> > > kfree > > > > > >> > > slab_free > > > > > >> > > slab_free_freelist_hook > > > > > >> > > slab_free_hook > > > > > >> > > __kasan_slab_free > > > > > >> > > ____kasan_slab_free > > > > > >> > > kasan_set_free_info > > > > > >> > > kasan_set_track > > > > > >> > > > > > > > >> > > And this issue only happens with "kmalloc-16" slab. Kasan has 2 > > > > > >> > > tracks: alloc_track and free_track, for x86_64 test platform, most > > > > > >> > > of the slabs will reserve space for alloc_track, and reuse the > > > > > >> > > 'object' area for free_track. The kasan free_track is 16 bytes > > > > > >> > > large, that it will occupy the whole 'kmalloc-16's object area, > > > > > >> > > so when kmalloc-redzone is enabled by this patch, the 'overwritten' > > > > > >> > > error is triggered. > > > > > >> > > > > > > > >> > > But it won't hurt other kmalloc slabs, as kasan's free meta won't > > > > > >> > > conflict with kmalloc-redzone which stay in the latter part of > > > > > >> > > kmalloc area. > > > > > >> > > > > > > > >> > > So the solution I can think of is: > > > > > >> > > * skip the kmalloc-redzone for kmalloc-16 only, or > > > > > >> > > * skip kmalloc-redzone if kasan is enabled, or > > > > > >> > > * let kasan reserve the free meta (16 bytes) outside of object > > > > > >> > > just like for alloc meta > > > > > >> > > > > > > >> > Maybe we could add some hack that if both kasan and SLAB_STORE_USER is > > > > > >> > enabled, we bump the stored orig_size from <16 to 16? Similar to what > > > > > >> > __ksize() does. > > > > > >> > > > > > >> How about the following patch: > > > > > >> > > > > > >> --- > > > > > >> diff --git a/mm/slub.c b/mm/slub.c > > > > > >> index added2653bb0..33bbac2afaef 100644 > > > > > >> --- a/mm/slub.c > > > > > >> +++ b/mm/slub.c > > > > > >> @@ -830,6 +830,16 @@ static inline void set_orig_size(struct kmem_cache *s, > > > > > >> if (!slub_debug_orig_size(s)) > > > > > >> return; > > > > > >> > > > > > >> +#ifdef CONFIG_KASAN > > > > > >> + /* > > > > > >> + * When kasan is enabled, it could save its free meta data in the > > > > > >> + * start part of object area, so skip the kmalloc redzone check > > > > > >> + * for small kmalloc slabs to avoid the data conflict. > > > > > >> + */ > > > > > >> + if (s->object_size <= 32) > > > > > >> + orig_size = s->object_size; > > > > > >> +#endif > > > > > >> + > > > > > >> p += get_info_end(s); > > > > > >> p += sizeof(struct track) * 2; > > > > > >> > > > > > >> I extend the size to 32 for potential's kasan meta data size increase. > > > > > >> This is tested locally, if people are OK with it, I can ask for 0Day's > > > > > >> help to verify this. > > > > > > > > > > Is there maybe some KASAN macro we can use instead of hardcoding 32? > > > > > > > > kasan_free_meta is placed in the object data after freeing, so it can > > > > be sizeof(kasan_free_meta) > > > > > > 'kasan_free_meta' is defined in mm/kasan/kasan.h, to use it we need to > > > include "../kasan/kasan.h" in slub.c, or move its definition to > > > "include/linux/kasan.h" > > > > > > Another idea is to save the info in kasan_info, like: > > > > > > --- > > > diff --git a/include/linux/kasan.h b/include/linux/kasan.h > > > index b092277bf48d..97e899948d0b 100644 > > > --- a/include/linux/kasan.h > > > +++ b/include/linux/kasan.h > > > @@ -100,6 +100,7 @@ static inline bool kasan_has_integrated_init(void) > > > struct kasan_cache { > > > int alloc_meta_offset; > > > int free_meta_offset; > > > + int free_meta_size; > > > > Storing it here looks fine to me. > > But I would name it based on the meaning for external users (i.e. that > > many bytes are occupied by kasan in freed objects). For some caches > > KASAN does not store anything in freed objects at all. > > OK, please review the below patch, thanks! > > - Feng > > ---8<--- > From c4fc739ea4d5222f0aba4b42b59668d64a010082 Mon Sep 17 00:00:00 2001 > From: Feng Tang > Date: Thu, 4 Aug 2022 13:25:35 +0800 > Subject: [PATCH] mm: kasan: Add free_meta size info in struct kasan_cache > > When kasan is enabled for slab/slub, it may save kasan' free_meta > data in the former part of slab object data area in slab object > free path, which works fine. > > There is ongoing effort to extend slub's debug function which will > redzone the latter part of kmalloc object area, and when both of > the debug are enabled, there is possible conflict, especially when > the kmalloc object has small size, as caught by 0Day bot [1] > > For better information for slab/slub, add free_meta's data size > info 'kasan_cache', so that its users can take right action to > avoid data conflict. > > [1]. https://lore.kernel.org/lkml/YuYm3dWwpZwH58Hu@xsang-OptiPlex-9020/ > Reported-by: kernel test robot > Signed-off-by: Feng Tang Acked-by: Dmitry Vyukov I assume there will be a second patch that uses free_meta_size_in_object in slub debug code. > --- > include/linux/kasan.h | 2 ++ > mm/kasan/common.c | 2 ++ > 2 files changed, 4 insertions(+) > > diff --git a/include/linux/kasan.h b/include/linux/kasan.h > index b092277bf48d..293bdaa0ba09 100644 > --- a/include/linux/kasan.h > +++ b/include/linux/kasan.h > @@ -100,6 +100,8 @@ static inline bool kasan_has_integrated_init(void) > struct kasan_cache { > int alloc_meta_offset; > int free_meta_offset; > + /* size of free_meta data saved in object's data area */ > + int free_meta_size_in_object; > bool is_kmalloc; > }; > > diff --git a/mm/kasan/common.c b/mm/kasan/common.c > index 78be2beb7453..a627efa267d1 100644 > --- a/mm/kasan/common.c > +++ b/mm/kasan/common.c > @@ -201,6 +201,8 @@ void __kasan_cache_create(struct kmem_cache *cache, unsigned int *size, > cache->kasan_info.free_meta_offset = KASAN_NO_FREE_META; > *size = ok_size; > } > + } else { > + cache->kasan_info.free_meta_size_in_object = sizeof(struct kasan_free_meta); > } > > /* Calculate size with optimal redzone. */ > -- > 2.27.0