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 95342C3DA49 for ; Thu, 25 Jul 2024 14:35:27 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 17F706B0085; Thu, 25 Jul 2024 10:35:27 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 130116B0092; Thu, 25 Jul 2024 10:35:27 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id F13AC6B0096; Thu, 25 Jul 2024 10:35:26 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id D18566B0085 for ; Thu, 25 Jul 2024 10:35:26 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 77C5D8111E for ; Thu, 25 Jul 2024 14:35:26 +0000 (UTC) X-FDA: 82378523052.07.BD2D52D Received: from mail-ed1-f44.google.com (mail-ed1-f44.google.com [209.85.208.44]) by imf03.hostedemail.com (Postfix) with ESMTP id 952A720029 for ; Thu, 25 Jul 2024 14:35:23 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=Djjof1FE; spf=pass (imf03.hostedemail.com: domain of jannh@google.com designates 209.85.208.44 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=1721918074; 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=D+McA66aY10VJ6ghs/3u7Af6ixrbJytWpoKwjU1ZAzo=; b=lUtm+90PVGT/4f+CZXwVTZem+Dgvx32zzPr6Op9D/2hJ7QTSZsvtWVywOD/RNSP1TRyAT3 mCEr3mU0cgIYwZbdVqWM8wIy/w2o4lsanMVeZSzReSeOf5vspCCnCLN/xcGx4GeqoJbZz/ pF92HtPiNwYOYczFpiztoB1vWMoRnjo= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1721918074; a=rsa-sha256; cv=none; b=1/DTOoYtOdKvWatG63fdSrhAW4qHxaZB0vYadwoSxQHNhvnqagOD+qGEc8hvsH/lyIAS9M NI6r3JJigpydAYhA9iZGJxHnoaJ8uCqwt5NsT9pra4TThxazF+hvNBqwa/iGxgt8cRT6h8 UxCNS8P/t6q9+JbjeGJSe7H0pLs80DI= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=Djjof1FE; spf=pass (imf03.hostedemail.com: domain of jannh@google.com designates 209.85.208.44 as permitted sender) smtp.mailfrom=jannh@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-ed1-f44.google.com with SMTP id 4fb4d7f45d1cf-5a28b61b880so13198a12.1 for ; Thu, 25 Jul 2024 07:35:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1721918122; x=1722522922; 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=D+McA66aY10VJ6ghs/3u7Af6ixrbJytWpoKwjU1ZAzo=; b=Djjof1FEZkY4rF6PaUwxN9YEKm+ysSM/b8a9ryMJhJdKACLI+pDouxyt0TFdWjTYJo g0qd4RMgIbbJZoe7l6o7i9jrA6Oq1cSvCgL6gN/Lab7h6MABBGDp2HSFwayj9j3UEEIc Ruz8xSk1JzjZWjYytXRUDAqAgbf6fM5KrgLtqyRCLw1Dmp6aJ8dIojm4uKBqnwDIMbiO BLZxDrUReZPuC1xGHspcErd8l75dRlVTdU2jCFST5xfNJdgOCnlvkS0yRgqbphW17kPT Gdiq7NedyoQY/uO700h04HrPwPJQJgnxouoeBIgCTPuR1n3VQ7NifSqqkFUOw+1yx+fv 71AA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1721918122; x=1722522922; 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=D+McA66aY10VJ6ghs/3u7Af6ixrbJytWpoKwjU1ZAzo=; b=QPXm4F4CeAqXTMtCRxYFUHAU2IAuR/uihGetSW+VdQ1qlZPuxLWQHZTZXgBi/OP7ro xAaCx0TUfswZPvBPwg/0qiKlUZG+tXrAsUWUiykgAJGBWo7VOAyVFntEqGbAiz4PtEwc 0A24x2LEsSVKoWWeh0mDoE+6y0+2HzHUU1dBJjVDAC2xBdJFSEbaxXzttMzBYKjXJGqH lDMrHZX1h3XCrE1jKpoJ7+ZSjASjU37kTJE82Gbwz2HMrvwMFeE5RyrABKyU2ExLiJsP TOXuJgFGak4G4tvMfTQNndGRXvJSTqBXHSOBpCbwcdFPhMhTbcxyJq3LWmZ2yZMyz+5j QrqA== X-Forwarded-Encrypted: i=1; AJvYcCWS91Mvc3Acc3UTupniybYpoYNdW2TCkmmH9gH0ueJGGr+jxz60+56ksRiCbqIAwUiir5hFNHIX07WgarHBdGXDddM= X-Gm-Message-State: AOJu0YzZSvwJ2ZovJaUzsOUChsaP37MbcrfryJau72cTEz4ouDpEAcNU I/QnJ0VoIFHFsfBEiKBTnvkDmVjpI7lKsaFlRcgYPcnHaDkwRuN4OQCkZ+G7hO00+fXklGWrDzf gAxiTnO6RcSfslrdphxbdwxhrnjsvhdaOnc5q X-Google-Smtp-Source: AGHT+IFsQcKHyP28uWVcrlW4h2tyYXKvjJGcqJ9fm224GrZ5xXpHACWdB9BeRTGwkeXGJ2yqTYjtxASYez2n61NSBbE= X-Received: by 2002:a05:6402:50d4:b0:58b:93:b623 with SMTP id 4fb4d7f45d1cf-5ac2ca7f225mr247791a12.5.1721918121258; Thu, 25 Jul 2024 07:35:21 -0700 (PDT) MIME-Version: 1.0 References: <20240724-kasan-tsbrcu-v2-0-45f898064468@google.com> <20240724-kasan-tsbrcu-v2-2-45f898064468@google.com> <9e05f9be-9e75-4b4d-84a4-1da52591574b@suse.cz> In-Reply-To: <9e05f9be-9e75-4b4d-84a4-1da52591574b@suse.cz> From: Jann Horn Date: Thu, 25 Jul 2024 16:34:44 +0200 Message-ID: Subject: Re: [PATCH v2 2/2] slub: Introduce CONFIG_SLUB_RCU_DEBUG To: Vlastimil Babka Cc: Andrey Ryabinin , Alexander Potapenko , Andrey Konovalov , Dmitry Vyukov , Vincenzo Frascino , Andrew Morton , Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , 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-Stat-Signature: uirx5b3azrx5boz8j3kficcz4rg8gh4m X-Rspamd-Queue-Id: 952A720029 X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1721918123-539963 X-HE-Meta: U2FsdGVkX1+bRgbIDE3b6vVPJYNkH2JG8QbPD3zvthpeXhUgTWXVfOcXBnPkdlFsrJv/0KINzTwdANTOxli4twYG/C2x0A6oP3eudZUNybp9Wz7bWYudfSnC1bTFxva6bsBPklYAzMtA0rPOAvu7CmlLY1zszAIKPtyIB1caIXhoR2R8d64em72GzX5E05Tvx5npHxhgdd2tF5Zhv6mMUOyUvFy/7Q3ZhZPHtrk3KfO6nWTHGtlZL9S2INaRsfLdn/iYJ5JY4VxvPvZazlX8xPUTPPORCMAAxOCeiKFmc6iRC1R6yELELMvq7xAQew0RfTXGJ1MkGN06VQWtiDkhI5hjR9/lvogUhIx2NJXwu0pABzpgmuQi/ORhRV3Wo/qju4XmhLs9eByFzSzGAMzjx64assMnHsza9Q+fBzmpCZrWTt6kCdwXds7N47zdcWRUPE9I/S9XKrSyJe61x+u9/UIYWbhKBWHA0JYnzRUvI2Nr9DPa03o/p0mDv0Jf8KZf87Y318o58UZK5BmCSe6wjPjJWnv/+JKN91YY5d7GrhRcYQ67ija7iVwVuzUONOPD1afA00uxWfThSChqclBB6QUSlewBNvPga2XBKg2irdFoW5Nfx99u19xtxmnAQgCLSvbiOuzXmUhRFmKy1Kp9pEk2IhDrg2kPAF43QQJqSeiwdW5PMNPNJsrTxsIYS9FRRciaN3UXrLPnrstieS/6GiZBPmuvLgCFcyiPnHFB05faf8cW2gh9Oqn+JO5xVk+de/GajgBxRBfRnnxSVqDwAacjsLLLgICHzq01Rfz8H4cZgdcT6sPaNHRgnhf20IX302/0V6ovummsdKyGRhHf1b4Zn9cLkroo2IqgCfTxur7PaVBjqjd6NRZBqBwVlUYCy2Kns80a63ZiubukGu26MNoDRtD4hyFuoqsgZ2BogIQ5t22P/4g1KF+QebZVuCG2Rp3Qgeaf6yXxLEdt+M9 5C/mvvl4 ehDQH7x9Ri81mpxy30/HzgaW5QfAQ7nfGiIrroey0nhyMU3We6tAFtbEYmM/xSGIfI90ECm8rZStQxlaVnBbL0wTufrQWk7foXdizKNCR6mngoukBXsj8jZ1ARfkX/Q9R9YZYvflenvagJsXCXeDA5VrgC86yigAHE5XMD+caMIWxg78FEjHe+NeN0+ym7WUcrJxh+eNl61GMmfHvAGujTckt26GjO9ht5bObk2CZyhciQsnFaX0IRKdqsmxrxJmCUsLKLDnKPh7/nwziHUSv6v6B/5jganECU8FHD3lS1MWSsJg3MFVxnnAuJ1ddOZ6r1EV+UFNR65Bb6q2AbgqkBnMoYP1eXrqKUPp3QcFsKCTL4zoX84utA2eJOQ== 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, Jul 25, 2024 at 3:28=E2=80=AFPM Vlastimil Babka wr= ote: > On 7/24/24 6:34 PM, Jann Horn wrote: > > 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. > > > > Note that this creates an aligned 16-byte area in the middle of the sla= b > > metadata area, which kinda sucks but seems to be necessary in order to = be > > able to store an rcu_head in there that can be unpoisoned while the RCU > > callback is pending. > > An alternative could be a head-less variant of kfree_rcu_mightsleep() tha= t > would fail instead of go to reclaim if it can't allocate, and upon failur= e > we would fall back ot the old behavior and give up on checking that objec= t? Yes, true, that would be an option... behaving differently under memory pressure seems a little weird to me, but it would probably do the job... I've now tried implementing it roughly as you suggested; the diffstat for that (on top of the existing series) looks like this: include/linux/kasan.h | 24 +++++++++--------------- mm/kasan/common.c | 23 +++++++---------------- mm/slab.h | 3 --- mm/slub.c | 46 +++++++++++++++++++--------------------------- 4 files changed, 35 insertions(+), 61 deletions(-) Basically it gets rid of all the plumbing I added to stuff more things into the metadata area, but it has to add a flag to kasan_slab_free() to tell it whether the call is happening after RCU delay or not. I'm changing slab_free_hook() to allocate an instance of the struct struct rcu_delayed_free { struct rcu_head head; void *object; }; with kmalloc(sizeof(*delayed_free), GFP_NOWAIT), and then if that works, I use that to RCU-delay the freeing. I think this looks a bit nicer than my original version; I'll go run the test suite and then send it out as v3. > But maybe it's just too complicated and we just pay the overhead. At leas= t > this doesn't concern kmalloc caches with their power-of-two alignment > guarantees where extra metadata blows things up more. If we wanted to compress the slab metadata for this down a bit, we could probably also overlap the out-of-line freepointer with the rcu_head, since the freepointer can't be in use while the rcu_head is active... but I figured that since this is a debug feature mainly intended for ASAN builds, keeping things simple is more important. > > (metadata_access_enable/disable doesn't work here because while the RCU > > callback is pending, it will be accessed by asynchronous RCU processing= .) > > To be able to re-poison the area after the RCU callback is done executi= ng, > > a new helper kasan_poison_range_as_redzone() is necessary. > > > > 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: Vlastimil Babka #slab > > ... > > > --- a/mm/slab_common.c > > +++ b/mm/slab_common.c > > @@ -450,6 +450,18 @@ static void slab_caches_to_rcu_destroy_workfn(stru= ct work_struct *work) > > > > static int shutdown_cache(struct kmem_cache *s) > > { > > + if (IS_ENABLED(CONFIG_SLUB_RCU_DEBUG) && > > + (s->flags & SLAB_TYPESAFE_BY_RCU)) { > > + /* > > + * Under CONFIG_SLUB_RCU_DEBUG, when objects in a > > + * SLAB_TYPESAFE_BY_RCU slab are freed, SLUB will interna= lly > > + * defer their freeing with call_rcu(). > > + * Wait for such call_rcu() invocations here before actua= lly > > + * destroying the cache. > > + */ > > + rcu_barrier(); > > + } > > I think once we have the series [1] settled (patch 5/6 specifically), the > delayed destruction could handle this case too? > > [1] > https://lore.kernel.org/linux-mm/20240715-b4-slab-kfree_rcu-destroy-v1-0-= 46b2984c2205@suse.cz/ Ah, thanks for the pointer, I hadn't seen that one. > > + > > /* free asan quarantined objects */ > > kasan_cache_shutdown(s); > > > > diff --git a/mm/slub.c b/mm/slub.c > > index 34724704c52d..999afdc1cffb 100644 > >