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 5EF7DC54EE9 for ; Mon, 19 Sep 2022 12:07:51 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id ECA736B0074; Mon, 19 Sep 2022 08:07:50 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E78406B0075; Mon, 19 Sep 2022 08:07:50 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D3FFE940007; Mon, 19 Sep 2022 08:07:50 -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 C63006B0074 for ; Mon, 19 Sep 2022 08:07:50 -0400 (EDT) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 328D51A1ACE for ; Mon, 19 Sep 2022 12:07:50 +0000 (UTC) X-FDA: 79928711100.18.07A9E8B Received: from mail-pl1-f182.google.com (mail-pl1-f182.google.com [209.85.214.182]) by imf19.hostedemail.com (Postfix) with ESMTP id DA59A1A0022 for ; Mon, 19 Sep 2022 12:07:48 +0000 (UTC) Received: by mail-pl1-f182.google.com with SMTP id v1so27700828plo.9 for ; Mon, 19 Sep 2022 05:07:48 -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=EVK8PXwi2Ipbx2nYX9NE/Drl0fbix3hzVw5SYu8VoBw=; b=Uy179iTKkuUOCxCMlkQ12IjxEwdA3a0PzEjtU5lZ3VlpwlAvZ8F1Lg+9LKggmnub9Z yFy0n0FH7cBE0al3+n9yIWkQI07XZvBOld4wGOd/2ZYt3pNGEgw3aPK+hgY7IT9nozBf ffCrVDMIt7ZRdqBKAk7Dsw1qPNi5hp+s19DMpc/xhaJa+TKFrtyeMDcRvAIso55Kzi4H 2G0I9UTK/poI3loTg3hZf9S7JyDD2/xZ1Exu9vKYsnu+7EKnL96s+zmhHuHKip3PXXiG AFTKYGwYFupRZHl2cDKDDzAaOLVT5ifiTOAMoQp/Y8vbc4GcXtFt1SkchvHIRW71Guur lDsw== 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=EVK8PXwi2Ipbx2nYX9NE/Drl0fbix3hzVw5SYu8VoBw=; b=gVjdOXfYhqzqa/J49LAAdg5yS06JpHicgWhSgj1Orkm22Y+pmmHm/xg2kYLC2r2Pv7 TH7CnruYT2GI/seOeiBx83mjwayvJMWnSvCPQgPkeFfWsWhSA1byN2DYh/30WdxYAVMR GD/1u/NRvL5d80WOh+C+QwlRUYZjIRo9mekZoN7xA/F0FhE9LGoqUebYcEkk6jDd0xsy z5WT1eh7t/2XirEi4/vHv7IToreWhzlb6GZUd/q93/bQ/pjclZ+l7FqBNdS6v38gIP9y QfV5SfkkxBQ8pL7RT/DDk5FbTm/PEd+tO0+BhN893sDE7ji8Z5IppPUhT9FTFHBUNZoz 0ZEg== X-Gm-Message-State: ACrzQf1L6v84Esmv5UcG9xFLWsWjINMqCiV8cGxxz9xmBfGhQtD6cAYN AzQsKmkiELTSEcSXD+HeSMU= X-Google-Smtp-Source: AMsMyM4yleRu5n8257cDzaYvwl74TjEFl3GdKBHK3mvMp8CeoOH/o4MVLXK+6s4AgNL25+UOe1XXKQ== X-Received: by 2002:a17:902:f7ca:b0:178:9c90:b010 with SMTP id h10-20020a170902f7ca00b001789c90b010mr5985752plw.149.1663589267834; Mon, 19 Sep 2022 05:07:47 -0700 (PDT) Received: from hyeyoo ([114.29.91.56]) by smtp.gmail.com with ESMTPSA id x5-20020a628605000000b00540d75197f2sm20956047pfd.143.2022.09.19.05.07.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 19 Sep 2022 05:07:46 -0700 (PDT) Date: Mon, 19 Sep 2022 21:07:41 +0900 From: Hyeonggon Yoo <42.hyeyoo@gmail.com> To: Vlastimil Babka Cc: Feng Tang , Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Roman Gushchin , Andrew Morton , Waiman Long , linux-mm@kvack.org, linux-kernel@vger.kernel.org, kasan-dev@googlegroups.com Subject: Re: [PATCH] mm/slab_common: fix possiable double free of kmem_cache Message-ID: References: <20220919031241.1358001-1-feng.tang@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1663589268; a=rsa-sha256; cv=none; b=wF2wmY+4gj1rP4Cpmpn4GOmeg/smHg01wfs+zVDS4WnGGoOFyZ6CnHJDf2PedFrc3Cg7ok XGQh4IEjTi1BGzqBkFjNSNZ7wtElOxUWzbbnZaonW66pZA7oNjYahYcygSmxIBS3JoUVQH ofkZVY0ud/Vj8XLP9yGj9PqZRSSdk30= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=Uy179iTK; spf=pass (imf19.hostedemail.com: domain of 42.hyeyoo@gmail.com designates 209.85.214.182 as permitted sender) smtp.mailfrom=42.hyeyoo@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1663589268; 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=EVK8PXwi2Ipbx2nYX9NE/Drl0fbix3hzVw5SYu8VoBw=; b=Qky1MHhFA4b3VUa7a50RDPFd1TnW+KRxoWXxGmTMJNOJiGkDudfpJbyOBD0FEj7FoxGYVW uTc9GiU53yStUHwlxOuDC9K8LYOmP3tNgA5TMUEQFq2U4Zmn73p/uwfZUdYxHuGmI9xxfj qtWzoNrJ/+UWC/jrUzN5JtWJC67I+hg= X-Stat-Signature: b5u54c3rtkhdnk9rj4z8jq6u6r1s9jmi X-Rspam-User: Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=Uy179iTK; spf=pass (imf19.hostedemail.com: domain of 42.hyeyoo@gmail.com designates 209.85.214.182 as permitted sender) smtp.mailfrom=42.hyeyoo@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: DA59A1A0022 X-HE-Tag: 1663589268-85398 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 Mon, Sep 19, 2022 at 02:03:15PM +0200, Vlastimil Babka wrote: > On 9/19/22 13:56, Hyeonggon Yoo wrote: > > On Mon, Sep 19, 2022 at 11:12:38AM +0200, Vlastimil Babka wrote: > >> On 9/19/22 05:12, Feng Tang wrote: > >> > When doing slub_debug test, kfence's 'test_memcache_typesafe_by_rcu' > >> > kunit test case cause a use-after-free error: > >> > > > > > If I'm not mistaken, I think the subject should be: > > s/double free/use after free/g > > Well, it's both AFAICS. By the initial use-after-free we can read a wrong > s->flags that was modified since we freed for the first time, and it can > lead to another kmem_cache_release() which is basically a double free. > Yeah, I realized that just after sending the mail ;) it is use-after-free bug that can potentially lead to double free. Thank you for correction! > >> > BUG: KASAN: use-after-free in kobject_del+0x14/0x30 > >> > Read of size 8 at addr ffff888007679090 by task kunit_try_catch/261 > >> > > >> > CPU: 1 PID: 261 Comm: kunit_try_catch Tainted: G B N 6.0.0-rc5-next-20220916 #17 > >> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014 > >> > Call Trace: > >> > > >> > dump_stack_lvl+0x34/0x48 > >> > print_address_description.constprop.0+0x87/0x2a5 > >> > print_report+0x103/0x1ed > >> > kasan_report+0xb7/0x140 > >> > kobject_del+0x14/0x30 > >> > kmem_cache_destroy+0x130/0x170 > >> > test_exit+0x1a/0x30 > >> > kunit_try_run_case+0xad/0xc0 > >> > kunit_generic_run_threadfn_adapter+0x26/0x50 > >> > kthread+0x17b/0x1b0 > >> > > >> > > >> > The cause is inside kmem_cache_destroy(): > >> > > >> > kmem_cache_destroy > >> > acquire lock/mutex > >> > shutdown_cache > >> > schedule_work(kmem_cache_release) (if RCU flag set) > >> > release lock/mutex > >> > kmem_cache_release (if RCU flag set) > >> > >> ^ not set > >> > >> I've fixed that up. > >> > >> > > >> > in some certain timing, the scheduled work could be run before > >> > the next RCU flag checking which will get a wrong state. > >> > > >> > Fix it by caching the RCU flag inside protected area, just like 'refcnt' > > > > Very nice catch, thanks! > > > > Otherwise (and with Vlastimil's fix): > > > > Looks good to me. > > Reviewed-by: Hyeonggon Yoo <42.hyeyoo@gmail.com> > > > >> > > >> > Signed-off-by: Feng Tang > >> > >> Thanks! > >> > >> > --- > >> > > >> > note: > >> > > >> > The error only happens on linux-next tree, and not in Linus' tree, > >> > which already has Waiman's commit: > >> > 0495e337b703 ("mm/slab_common: Deleting kobject in kmem_cache_destroy() > >> > without holding slab_mutex/cpu_hotplug_lock") > >> > >> Actually that commit is already in Linus' rc5 too, so I will send your fix > >> this week too. Added a Fixes: 0495e337b703 (...) too. > >> > >> > mm/slab_common.c | 5 ++++- > >> > 1 file changed, 4 insertions(+), 1 deletion(-) > >> > > >> > diff --git a/mm/slab_common.c b/mm/slab_common.c > >> > index 07b948288f84..ccc02573588f 100644 > >> > --- a/mm/slab_common.c > >> > +++ b/mm/slab_common.c > >> > @@ -475,6 +475,7 @@ void slab_kmem_cache_release(struct kmem_cache *s) > >> > void kmem_cache_destroy(struct kmem_cache *s) > >> > { > >> > int refcnt; > >> > + bool rcu_set; > >> > > >> > if (unlikely(!s) || !kasan_check_byte(s)) > >> > return; > >> > @@ -482,6 +483,8 @@ void kmem_cache_destroy(struct kmem_cache *s) > >> > cpus_read_lock(); > >> > mutex_lock(&slab_mutex); > >> > > >> > + rcu_set = s->flags & SLAB_TYPESAFE_BY_RCU; > >> > + > >> > refcnt = --s->refcount; > >> > if (refcnt) > >> > goto out_unlock; > >> > @@ -492,7 +495,7 @@ void kmem_cache_destroy(struct kmem_cache *s) > >> > out_unlock: > >> > mutex_unlock(&slab_mutex); > >> > cpus_read_unlock(); > >> > - if (!refcnt && !(s->flags & SLAB_TYPESAFE_BY_RCU)) > >> > + if (!refcnt && !rcu_set) > >> > kmem_cache_release(s); > >> > } > >> > EXPORT_SYMBOL(kmem_cache_destroy); > >> > > > -- Thanks, Hyeonggon