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 BFCA6C52D6D for ; Fri, 2 Aug 2024 11:23:06 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 349146B0089; Fri, 2 Aug 2024 07:23:06 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2F9826B008A; Fri, 2 Aug 2024 07:23:06 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1C10C6B008C; Fri, 2 Aug 2024 07:23:06 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id F21E76B0089 for ; Fri, 2 Aug 2024 07:23:05 -0400 (EDT) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id B4CD21C499E for ; Fri, 2 Aug 2024 11:23:05 +0000 (UTC) X-FDA: 82407068730.15.DE22824 Received: from mail-ed1-f44.google.com (mail-ed1-f44.google.com [209.85.208.44]) by imf23.hostedemail.com (Postfix) with ESMTP id BF6EE140016 for ; Fri, 2 Aug 2024 11:23:03 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=cM7jIdow; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf23.hostedemail.com: domain of jannh@google.com designates 209.85.208.44 as permitted sender) smtp.mailfrom=jannh@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1722597719; 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=TXuC7eL0okWOe+3ruv5rtgdzN2mMwl0ZfQ5u9zC8B54=; b=rb4N/rSQCasWGIWxBEAEBVvk1I55aNuqdiomF7PIewHokKygNdS1oiC6+yLETPv8SBmvEQ LOAeNVTCAVwb9bR8nPkxaVHMv+OboAC12TKRFJIwMZQJZn+4YHSKxzlejnkh+F8iAAlwY5 2qJRNsp3qi6RO68TDkpoQgGbz470Pmo= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1722597719; a=rsa-sha256; cv=none; b=DJHDjYU4O/nce2hdBeeMlIHgSHCHYddl9/QaK6adHPr8whJcM1Y37VKe05Sf5n1zEFzgCy Frfuv3R/n67xtHvy5CLMmgxnrAT7u78gtjSR8kKuMb9Vg2oZQDR1oEXkegpcYb5aWTfmso LNnIGdrHWgygq8iw7cGSWCo0eT465vQ= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=cM7jIdow; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf23.hostedemail.com: domain of jannh@google.com designates 209.85.208.44 as permitted sender) smtp.mailfrom=jannh@google.com Received: by mail-ed1-f44.google.com with SMTP id 4fb4d7f45d1cf-5a869e3e9dfso50940a12.0 for ; Fri, 02 Aug 2024 04:23:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1722597782; x=1723202582; 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=TXuC7eL0okWOe+3ruv5rtgdzN2mMwl0ZfQ5u9zC8B54=; b=cM7jIdowH8R6XoXFJp8W4Rv60UNdEu5EM9sNhLrw0N42ZtBnWzUebnJMMzdt/ik20A 5n1yXcft1y+pVBHHK6uHWP9p0BAmTzUdOC2vRobtwb0T6eZlzCyJD3aqkGUQDLD5jUyI MGjvgfcAjpCXRC/I1LZj+DLD3mI8jBWCg2KCCVnYqNKEY2hT+0qxK80jZqykZc2xp3rb O+TiUCH69IkKSrgq4hVHEalhbz8XadHAs+IH58ZWN/k9hPquYYzgrgdJfbY4ucsl7K/D spMZ1RKW4ykjw+nXNRf5xZU2O/Jo9A2uHOoj+p8ot8V+I2QkPwovDMfZpmYh9oOflH/v JbWg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1722597782; x=1723202582; 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=TXuC7eL0okWOe+3ruv5rtgdzN2mMwl0ZfQ5u9zC8B54=; b=SRgMaM5JIOheM9IDAZxbByQZThAQs4el3+f4iEHoVKHasSfTlTKlAR9SXZRLEsSKJ9 lLZldKmdYAU0dX2CKAlUAbghh+K5XymSNgnVuIhgqGQr4DiHUsdUkue5rYwoRJxz6mOj Wcvn6Oo/S9k+ywwQ767H1Cs4ZUH1FFZ4ymKQYlxIPJX2mJAqMy0Dc8V3foO16fKl0N4B P6BB7IKDYnKREu7/4T/oULtKWlhDerUCGUONNq2TcWVmJ5URGJlk+eXzUppG8OWQbxkm D3DqbJYGymotQL5J/YMApkF4JL6FOQQyeVBYl0JT9++v9AsB7bI8cPEEGHLJQ968ZLjy guRg== X-Forwarded-Encrypted: i=1; AJvYcCViNmR80HCdJgQUrFO/2YgJ86RuMZysdKzyBamHBT7288/bi+Fhlxy0w60lNltXSOYU7r9IVWcPMc2e1OdUw1DIvus= X-Gm-Message-State: AOJu0YyGlFl2E7oz6FuHSi8V/stl4AvUTjgs7B2xA5KA1IeEPS1Xse5p hXFeqh7cwRyMu6rqXHBwLtffoenqOgDps0RXm5rGFMA8FRl+3bEaO7ULFPqshMvT5yvtbuSm6gx ZepRcW6wR4ZWAMTeW6681zBr0iw2dDlOo1ryd X-Google-Smtp-Source: AGHT+IEdePClV7Jvqw70Ip55InNeH7ZpGzuDQIu2n1gbdG50BV+x8Jh6WjYdxKBCFXgtjGe6LPugeYAsGgDfbVtUUXU= X-Received: by 2002:a05:6402:40cf:b0:5ac:4ce3:8f6a with SMTP id 4fb4d7f45d1cf-5b86bf8e337mr112549a12.6.1722597781429; Fri, 02 Aug 2024 04:23:01 -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 13:22:23 +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: rspam07 X-Rspamd-Queue-Id: BF6EE140016 X-Stat-Signature: fbywwms373q7ctg5qe4oyuzgit93b53y X-Rspam-User: X-HE-Tag: 1722597783-930748 X-HE-Meta: U2FsdGVkX18dQc7B0YOgYs+GRM2B5zzmLgRwcxhTSKB1C4K4MLz661VBdzqJUbP/oHE5dwACf+YZhkeqKmF4MjQGizVbcRuYYOarsp04V3QavaiUA26ZGSguhgBWP57pi+1jh6A/VTeaTGBSLQUUJCjd7bqO5RO2qz1SmRWzezz4f5PioUQGb4yXyqJ+yBcYe5XotriZv/tnnrte9Gr4O+KhmXp3gRSARjB17MF7f/Nq7gNOkC0gxnOBbxOx6PbYXtyj2OPYcJyYLrKfh5km+0vlFT2dUZaHqZoq/wcFDLZXqN/Xn9wKrLmnQCpcc3ExZj5ToDXCNtb15aDZLRIB8p7X/FQJ8UtTHVEJ7MpyuDpN4FKIyCuU8FF+TtjguvUfxfYvkhjFyDeseCfN32FvwXyA/tfya2N7lrZDpKm0WUst5h1G9kyEavHpDQvxBHyFznmSAzQ2RdtCsxxSgA8M5UGiDcD8f3LbAc5ekOrApiAZ2pTEouQw9EDhoF1Ktp8UtAg3uau4O/5dzJA6QoDLmiuZTqS59K0/FR4PQzNXXFqj0pMKoxwm7HJ363NYgau4P1Cql4aaygaQQiPJKTZyAdI4oD3J6zo8Go6DnVnoMdgTCTF1aGxq/SFdwOihjy8pHEUGNLSbASfnZqLX8UAnRFyDaQmHeD+GwN/A9WayfOi3+83+AApznJeAd000Ho4Hzs4ofgVCKaWqsadNztiez8AdyXOu3k15u1K6aqzqr0tJ6A/OpBex0+1QomVQwSn0dDi2osBkq2PTHliUlaanbsYpfNRGld3zeFC/uXSFyguRqGEQuhtuFqNVVh8HC/ia3XwfWkp3wNgV0vOp40VYnAznAnz9UMsJH668Jgf1xiT25UhsLa+wMqFUYb0W7Ekd5k9411q6/PH4SxFOVfQB/cq0+IEtOoGV/fTXreo09MRQLSMLnJG/6iXrw1A61xNuoHALjmwnneR6VCkcqAE FdbakCo1 1YQPPh+UTmct0nIKkI/f9uIW63QIPscVX8IQE8j8a5+D5k/iScbQsFNhchgyqWfte86D+vnZHvE+/q1bpIXGIhmVkYAthrkzNrBSkIg9dsHMJjE3JBKh6bsUGfF8/MWizzcNFYWpu5rSG30nDvpU1H5aE13/LpOayIe7CWOpf2BsKEm71kjmzYnwpW/EJBuByXAgbEckoxSMqEyrq8jB3QKsrl2R6136+/QogGpEIu9277qkEQkPSoh/rmCSakgKb3FqVCRT/PKHXrI2STEoSB9fsKimd65O7mATHw5o1P43Psq2zU6maJ5Qv0Q== 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 Fri, Aug 2, 2024 at 11:09=E2=80=AFAM Jann Horn wrote: > 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? Regarding this, I think I'll add something like this on top of this patch i= n v6: diff --git a/include/linux/kasan.h b/include/linux/kasan.h index b63f5351c5f3..50bad011352e 100644 --- a/include/linux/kasan.h +++ b/include/linux/kasan.h @@ -201,16 +201,17 @@ bool __kasan_slab_free(struct kmem_cache *s, void *object, bool init, /** * kasan_slab_free - Possibly handle slab object freeing. * @object: Object to free. + * @still_accessible: Whether the object contents are still accessible. * * This hook is called from the slab allocator to give KASAN a chance to t= ake * ownership of the object and handle its freeing. * kasan_slab_pre_free() must have already been called on the same object. * * @Return true if KASAN took ownership of the object; false otherwise. */ static __always_inline bool kasan_slab_free(struct kmem_cache *s, void *object, bool init, - bool after_rcu_delay) + bool still_accessible) { if (kasan_enabled()) return __kasan_slab_free(s, object, init, after_rcu_delay); @@ -410,7 +411,7 @@ static inline bool kasan_slab_pre_free(struct kmem_cache *s, void *object) } static inline bool kasan_slab_free(struct kmem_cache *s, void *object, - bool init, bool after_rcu_delay) + bool init, bool still_accessible) { return false; } diff --git a/mm/kasan/common.c b/mm/kasan/common.c index 71a20818b122..ed4873e18c75 100644 --- a/mm/kasan/common.c +++ b/mm/kasan/common.c @@ -230,14 +230,14 @@ static bool check_slab_allocation(struct kmem_cache *cache, void *object, } static inline void poison_slab_object(struct kmem_cache *cache, void *obje= ct, - bool init, bool after_rcu_delay) + bool init, bool still_accessible) { void *tagged_object =3D object; object =3D kasan_reset_tag(object); /* RCU slabs could be legally used after free within the RCU period= . */ - if (unlikely(cache->flags & SLAB_TYPESAFE_BY_RCU) && !after_rcu_del= ay) + if (unlikely(still_accessible)) return; kasan_poison(object, round_up(cache->object_size, KASAN_GRANULE_SIZ= E), @@ -256,12 +256,12 @@ bool __kasan_slab_pre_free(struct kmem_cache *cache, void *object, } bool __kasan_slab_free(struct kmem_cache *cache, void *object, bool init, - bool after_rcu_delay) + bool still_accessible) { if (!kasan_arch_is_ready() || is_kfence_address(object)) return false; - poison_slab_object(cache, object, init, after_rcu_delay); + poison_slab_object(cache, object, init, still_accessible); /* * If the object is put into quarantine, do not let slab put the ob= ject diff --git a/mm/slub.c b/mm/slub.c index 49571d5ded75..a89f2006d46e 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -2221,31 +2221,34 @@ static __always_inline bool slab_free_hook(struct kmem_cache *s, void *x, bool init, bool after_rcu_delay) { + /* Are the object contents still accessible? */ + bool still_accessible =3D (s->flags & SLAB_TYPESAFE_BY_RCU) && !after_rcu_delay; + kmemleak_free_recursive(x, s->flags); kmsan_slab_free(s, x); debug_check_no_locks_freed(x, s->object_size); if (!(s->flags & SLAB_DEBUG_OBJECTS)) debug_check_no_obj_freed(x, s->object_size); /* Use KCSAN to help debug racy use-after-free. */ - if (!(s->flags & SLAB_TYPESAFE_BY_RCU) || after_rcu_delay) + if (!still_accessible) __kcsan_check_access(x, s->object_size, KCSAN_ACCESS_WRITE | KCSAN_ACCESS_ASSE= RT); if (kfence_free(x)) return false; /* * Give KASAN a chance to notice an invalid free operation before w= e * modify the object. */ if (kasan_slab_pre_free(s, x)) return false; #ifdef CONFIG_SLUB_RCU_DEBUG - if ((s->flags & SLAB_TYPESAFE_BY_RCU) && !after_rcu_delay) { + if (still_accessible) { struct rcu_delayed_free *delayed_free; delayed_free =3D kmalloc(sizeof(*delayed_free), GFP_NOWAIT)= ; @@ -2289,7 +2292,7 @@ bool slab_free_hook(struct kmem_cache *s, void *x, bool init, s->size - inuse - rsize); } /* KASAN might put x into memory quarantine, delaying its reuse. */ - return !kasan_slab_free(s, x, init, after_rcu_delay); + return !kasan_slab_free(s, x, init, still_accessible); } static __fastpath_inline