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 C9C8AC3DA4A for ; Fri, 2 Aug 2024 09:10:35 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5A5126B007B; Fri, 2 Aug 2024 05:10:35 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 554906B0083; Fri, 2 Aug 2024 05:10:35 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 41C0B6B0085; Fri, 2 Aug 2024 05:10: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 24D596B007B for ; Fri, 2 Aug 2024 05:10:35 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 95727A7C8C for ; Fri, 2 Aug 2024 09:10:34 +0000 (UTC) X-FDA: 82406734788.22.EF1616A Received: from mail-ed1-f42.google.com (mail-ed1-f42.google.com [209.85.208.42]) by imf28.hostedemail.com (Postfix) with ESMTP id BE3D4C002A for ; Fri, 2 Aug 2024 09:10:32 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=U5gje7xQ; spf=pass (imf28.hostedemail.com: domain of jannh@google.com designates 209.85.208.42 as permitted sender) smtp.mailfrom=jannh@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1722589804; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=BdfVeEEYqxRlg+wsbTTfgEpqIH6PLQRg/ETvJe3ixN4=; b=kIJXzCnTr2yAnA4fSTHLpDx7q5pGaLHS/BtBYSDI+SAz14cU0j8oXlKfUchakVm1hvS2of G4hLULyIBCEDFuriNd264ZAp7sI4ogUc5TiY4/5sOh+qInlFa9qbQAhi8yjmMBd/Hj5ptU LxoMAeHHkzhFAvivcRIW0BeXpwwWFsA= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=U5gje7xQ; spf=pass (imf28.hostedemail.com: domain of jannh@google.com designates 209.85.208.42 as permitted sender) smtp.mailfrom=jannh@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1722589804; a=rsa-sha256; cv=none; b=IF1/MZ0+6gWr7tCiBd9vleJx/2rmIH/SeN6iD1UkCmFxfWHJXqkLHnkfv/xqxVPICyZnJ0 SL3ZD7uV4kOG1geUwonmwwUg6wSvHAaRWsyvfHBK/GsVaedHUq1QmvDesXlNYgZ+l1YL3V Dpn7G80NRDBZdApix+CXdhCg0pQCD0U= Received: by mail-ed1-f42.google.com with SMTP id 4fb4d7f45d1cf-5a28b61b880so48092a12.1 for ; Fri, 02 Aug 2024 02:10:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1722589831; x=1723194631; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=BdfVeEEYqxRlg+wsbTTfgEpqIH6PLQRg/ETvJe3ixN4=; b=U5gje7xQ7yXzC9dk8hwGOyguTKeohYOPGuTp+hqVlpuU/TCurWllopwZGXiqobaIiW M1LCaTwNJQgUpxAmGVzLieIEy8na4MHyMgtVUlkxez3y0m0ouxFpDXckcpwKWM9UwKlK KNmzfI1oibNuYIUXf4goONi8vxwr5cFmLNOIeNR1a81Flb3SgUzx1COdFMqPLKpiaIav vYjlfkYmMPZEflE/DerI3ZALM9fpBvj7pXDK1+U8LxHpYtTakWvSGxAgK3v/p5IXsK+c t/2gSEjJthrDQ5TXrlcLPDy5fMj0sXxAHyOXTnVKrCXUJw+iwAMWu3lcxUALe1TA+EvD SFLg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1722589831; x=1723194631; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=BdfVeEEYqxRlg+wsbTTfgEpqIH6PLQRg/ETvJe3ixN4=; b=SE5R5VbiXfzL2ObzSJYSSogRmCak7oUKvv52ueR8j00YllpmF661UuxP1HbmwTSFix ssdeLQC/7mPmxA4kSbxjZfTCUXu3+YrBRA/hq4s8uO+/6RjLAJJ2/mi8aWFXgN49U/Wn W4tcQsHUwuYy13bPSjLOSCS/L2wEFQKPdpECdW/3EOFyFBtn6TgQlOrf2wuxXgVaekrF sCXkguCbaoG4HCY5IXyfaiLE+bYAFIlCeHqlEB/PYCAI7/ET8Uv9RMHBnNpmfeMcAHSC v8YO/z5jfEBX5GsIa9K0tX+585l2VjMtoTuaBxHTpvzWOFdxDaYrWn0CD0/oZ4fiWoVR U+/A== X-Forwarded-Encrypted: i=1; AJvYcCX8SUdBcvlQhUfbicUJoUwmwgKCTcScI+9UynY4fEEjeCCjKVVzUnHIEUYR3P0R7HSbdE6RAhx6ZKjyTKrqFzoqvv0= X-Gm-Message-State: AOJu0YxM4OYHJ2d8JMpDwCoV/PbbpYkGkwqNRJW9eYm1iTA8U+rEaliy Cxy0kh55wUQrtBP0XfX4OHDj+jC/7B1krGxq8SESu9YWlSSi9tW9ROJbyylCURMZpgMVxh7y1W6 ngCxGL8O49lMJyJr+7l0RpwUvKChsMwNZokj2 X-Google-Smtp-Source: AGHT+IGugm+hPfyfakoQaLgMekarHdSbFiVnUtaejE1K1ZAA7bomT9rRqhXgPo0o8RdCEcX65x57QeQ+/tw0S8F9fiM= X-Received: by 2002:a05:6402:1747:b0:5b8:ccae:a8b8 with SMTP id 4fb4d7f45d1cf-5b8ccaeab1bmr32772a12.3.1722589830338; Fri, 02 Aug 2024 02:10:30 -0700 (PDT) MIME-Version: 1.0 References: <20240730-kasan-tsbrcu-v5-0-48d3cbdfccc5@google.com> <20240730-kasan-tsbrcu-v5-2-48d3cbdfccc5@google.com> In-Reply-To: From: Jann Horn Date: Fri, 2 Aug 2024 11:09:52 +0200 Message-ID: Subject: Re: [PATCH v5 2/2] slub: Introduce CONFIG_SLUB_RCU_DEBUG To: Andrey Konovalov Cc: Andrey Ryabinin , Alexander Potapenko , Dmitry Vyukov , Vincenzo Frascino , Andrew Morton , Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Vlastimil Babka , Roman Gushchin , Hyeonggon Yoo <42.hyeyoo@gmail.com>, Marco Elver , kasan-dev@googlegroups.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam03 X-Rspam-User: X-Rspamd-Queue-Id: BE3D4C002A X-Stat-Signature: uzer8geobeqjiwy5ur9cid969fgzqk4y X-HE-Tag: 1722589832-216699 X-HE-Meta: U2FsdGVkX19rLCeCrvAekoLr4DBrcXYSVCLmV9cQ1VQidjefE1I6DuxspYTpDZArw5nTIx+QIi6F4/QBLnUNb03mnT+Y/VOPn0qqJvN++Zb815x8oe7pW8BhdMNv71zQBPFsyQTtIHrjk/waBo3gG7xAVRtvBnglfpvSLrX3oNXO5OaWVDCjDn0i1K8L9PDDXPRAL1+lLYYB5EAdIcFWxN1K5EAxABRx+qGdQyL0zqW+DtrWBvqhPWaVPvd4v0uRTjkqY8Kt4C4er+F62i2wNTukVJo3HlL7EVa+gLhjIWI/eaLOGqCuYIJm+zRURZfF5ly8xiJ9FzAgKRho0tvgx/RZHrczh2HkvjmL/tXYmejVZgwJSDHDFCRvgaFetnF5ITdQh+XB2KMS0/wMk3bJ+XngEIDKGEnhLi9BQmsAF+tz4cy2ER+D0p89kydt5nUzuxDA6JX/XjbvyOm+o5FKejNHW65Um26uw67s6yiTVd5vxx4qoJsSCy3ZMfU2UzCuj4CQugdqb631+b1FiS/1i0lJBFD+mXqUVzU0bPDzecS0qcpmdfM5lteguEZptIOGc8jq0t3lih7CL5ApvQys6KIbdCEHqtYk8yKPRazt1bbAdaLBrmai3LivPLto6cV6LaTNCmtKMwB+qLynwdCReevoRVMBt1BJ5gG0fE7ucG3rVdb0JxbTKuyYuH/6I/bcaELDrmD9Z+hcnUBVwpDeWqjEc6fKDvHi8swUKOzZIGzrfs162UCPnld/qPdcf+J/MtGCA4q5AKCXlQazcG8D/j48JF4Av0rA4YN+2WB6OUKA+kVOXVUGs4xCtaGaGSRBdRTNmIp+GEVRINLGcaYYycYXmnJO2ikrJXuQFfv09cAzVaMFaMORhq/dkJo8XaTIQyUebf2EQgHYsjYDMd1a1YqtU1dH55rh8V1InYKuWqCIRQFKS31ymqgtnYamVlCBGCXLDznaJgFNX1+l8De iEBxUZ2/ Q8ULcPrCdd7UKp7uOF3zcHsh+c6FeV9SihsPlbh1X2p9tKPSmM1u2wGuO8k6S3cAUuAc9Kdlk5FhgOcnN9O76aXaBkIa+uyCa0BfnHXCnV24GP5hZcx52ejf3odz3mCMVXecOMLsEQURgk4Wdxmvv+yaKekyUIOvm0ox/JKk3Fs3qeHMKd8zTYOq7dL/wkuK70P+/UtAqB0mWNJNqlbd+VXFVakEWhniFcqtjrBck6C6q3kW8uz9DikeSs9UY76vOa4LLFojs3IU3aRcGqPeofYGD41wJc1K4QPszjtv8nai9TNWC9X9rhTM3hMtkQ7vO3OneZzH/OSQrXDQIqHWU37SnLA== 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: List-Subscribe: List-Unsubscribe: On Thu, Aug 1, 2024 at 2:23=E2=80=AFAM Andrey Konovalov wrote: > On Tue, Jul 30, 2024 at 1:06=E2=80=AFPM Jann Horn wrot= e: > > Currently, KASAN is unable to catch use-after-free in SLAB_TYPESAFE_BY_= RCU > > slabs because use-after-free is allowed within the RCU grace period by > > design. > > > > Add a SLUB debugging feature which RCU-delays every individual > > kmem_cache_free() before either actually freeing the object or handing = it > > off to KASAN, and change KASAN to poison freed objects as normal when t= his > > option is enabled. > > > > For now I've configured Kconfig.debug to default-enable this feature in= the > > KASAN GENERIC and SW_TAGS modes; I'm not enabling it by default in HW_T= AGS > > mode because I'm not sure if it might have unwanted performance degrada= tion > > effects there. > > > > Note that this is mostly useful with KASAN in the quarantine-based GENE= RIC > > mode; SLAB_TYPESAFE_BY_RCU slabs are basically always also slabs with a > > ->ctor, and KASAN's assign_tag() currently has to assign fixed tags for > > those, reducing the effectiveness of SW_TAGS/HW_TAGS mode. > > (A possible future extension of this work would be to also let SLUB cal= l > > the ->ctor() on every allocation instead of only when the slab page is > > allocated; then tag-based modes would be able to assign new tags on eve= ry > > reallocation.) > > > > Signed-off-by: Jann Horn > > Acked-by: Andrey Konovalov > > But see a comment below. > > > --- > > include/linux/kasan.h | 11 +++++--- > > mm/Kconfig.debug | 30 ++++++++++++++++++++ > > mm/kasan/common.c | 11 ++++---- > > mm/kasan/kasan_test.c | 46 +++++++++++++++++++++++++++++++ > > mm/slab_common.c | 12 ++++++++ > > mm/slub.c | 76 +++++++++++++++++++++++++++++++++++++++++++= ++------ > > 6 files changed, 169 insertions(+), 17 deletions(-) > > > > diff --git a/include/linux/kasan.h b/include/linux/kasan.h > > index 34cb7a25aacb..0b952e11c7a0 100644 > > --- a/include/linux/kasan.h > > +++ b/include/linux/kasan.h > > @@ -194,28 +194,30 @@ static __always_inline bool kasan_slab_pre_free(s= truct kmem_cache *s, > > { > > if (kasan_enabled()) > > return __kasan_slab_pre_free(s, object, _RET_IP_); > > return false; > > } > > > > -bool __kasan_slab_free(struct kmem_cache *s, void *object, bool init); > > +bool __kasan_slab_free(struct kmem_cache *s, void *object, bool init, > > + bool after_rcu_delay); > > What do you think about renaming this argument to poison_rcu? I think > it makes the intention more clear from the KASAN's point of view. Hm - my thinking here was that the hook is an API between SLUB and KASAN, and so the hook definition should reflect the API contract that both SLUB and KASAN have to know - and in my head, this contract is that the parameter says whether SLUB guarantees that an RCU delay has happened after kmem_cache_free() was called. In my mind, SLUB tells KASAN what is going on and gives KASAN a chance to take ownership of the object, but doesn't instruct KASAN to do anything specific. And "poison" is ambiguous - in SLUB, "poison" normally refers to overwriting object contents with a poison value, which currently wouldn't be allowed here due to constructor slabs. I guess another way to describe the meaning of the argument with its current value would be something like "even though the object is an object with RCU lifetime, the object is guaranteed to no longer be in use". But I think the simplest way to describe the argument as currently defined is "an RCU grace period has passed since kmem_cache_free() was called" (which I guess I'll add to the kasan_slab_free doc comment if we keep the current naming). I guess I could also change the API to pass something different - like a flag meaning "the object is guaranteed to no longer be in use". There is already code in slab_free_hook() that computes this expression, so we could easily pass that to KASAN and then avoid doing the same logic in KASAN again... I think that would be the most elegant approach? > > /** > > * kasan_slab_free - Possibly handle slab object freeing. > > * @object: Object to free. > > @poison_rcu - Whether to skip poisoning for SLAB_TYPESAFE_BY_RCU caches. > > And also update the reworded comment from the previous patch: > > This function poisons a slab object and saves a free stack trace for > it, except for SLAB_TYPESAFE_BY_RCU caches when @poison_rcu is false. I think that's a KASAN implementation detail, so I would prefer not putting that in this header.