From patchwork Mon Jan 29 10:07:02 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marco Elver X-Patchwork-Id: 13535352 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 B46B8C47422 for ; Mon, 29 Jan 2024 10:07:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 47C816B009A; Mon, 29 Jan 2024 05:07:19 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 42AE66B009B; Mon, 29 Jan 2024 05:07:19 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2A54B6B009C; Mon, 29 Jan 2024 05:07:19 -0500 (EST) 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 18E496B009A for ; Mon, 29 Jan 2024 05:07:19 -0500 (EST) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id EAC61A027F for ; Mon, 29 Jan 2024 10:07:18 +0000 (UTC) X-FDA: 81731920956.14.203C904 Received: from mail-ej1-f74.google.com (mail-ej1-f74.google.com [209.85.218.74]) by imf10.hostedemail.com (Postfix) with ESMTP id 03A2AC001D for ; Mon, 29 Jan 2024 10:07:16 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=bEAet3qA; spf=pass (imf10.hostedemail.com: domain of 303i3ZQUKCNY6DN6J8GG8D6.4GEDAFMP-EECN24C.GJ8@flex--elver.bounces.google.com designates 209.85.218.74 as permitted sender) smtp.mailfrom=303i3ZQUKCNY6DN6J8GG8D6.4GEDAFMP-EECN24C.GJ8@flex--elver.bounces.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=1706522837; 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=+qmvJzBHNfclU7yyqgUvOeu3NcuQrq93zAHIMt+tsRw=; b=BX6kMRvLeMfSXgZvxaB0ZdD1kFncA/9a8FclLvT9wvRQqvrfiUsgpK5adu7s2kLfPl9FyO jFdZnCdYi6rI75MH5SDjRYcrqqealCTnvwOBmbfQlhBD7YBmSN/eL4sWlgAjo0A/ji/sB7 G5MOkU/aVj0tKwzxfQ5D6mV8iCDtCaA= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1706522837; a=rsa-sha256; cv=none; b=DY8udpAN9wpnJxpOdFvffyQQug96BZSwozIXKMLNADecQCmfM7CZN21Po1SCqO85aFIR83 E3Jni1IWNq+2+qB5m6d7OCDb/GUnfz8Ix9s7LQj9nLL3BAZDYDYGvh478sgMBffx11JEWv KUr4Vb4EQDBluKT9Lfu1fYKcwiLVKP0= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=bEAet3qA; spf=pass (imf10.hostedemail.com: domain of 303i3ZQUKCNY6DN6J8GG8D6.4GEDAFMP-EECN24C.GJ8@flex--elver.bounces.google.com designates 209.85.218.74 as permitted sender) smtp.mailfrom=303i3ZQUKCNY6DN6J8GG8D6.4GEDAFMP-EECN24C.GJ8@flex--elver.bounces.google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-ej1-f74.google.com with SMTP id a640c23a62f3a-a35992e7b85so41907966b.3 for ; Mon, 29 Jan 2024 02:07:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1706522835; x=1707127635; darn=kvack.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=+qmvJzBHNfclU7yyqgUvOeu3NcuQrq93zAHIMt+tsRw=; b=bEAet3qATR0L28ZMwsu3JUTX+LcnTg6iw9O6UCaZXiCDqPFrMAeVWymF/tDChY0+3k lxaxAqMcOuQUHXusxaaoATR0bWoc/s7N6ERYF5caUWlmcq25yT5k/WphZmLJmqgtuV04 4spsGHJGz2t7Ag3/XyT8McwRAltzCu1yby/LzsOiXp7mmHuBtZ7UsG1/SKDHWCtCxvoD xStSbwHBvC1forI1+5yY+mkVFEdVOjWXzgH8McNO4KeBH7unkMrm+vfgkqvMVX3m/XEj WEUK/n8ESGKhMHfjCyTz3fRMUgow64T9RzdGvv24txkFwL+s61krw6yOSO0kQ/4WznXP TbWQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706522835; x=1707127635; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=+qmvJzBHNfclU7yyqgUvOeu3NcuQrq93zAHIMt+tsRw=; b=S79Hg6iGWDTGzgpnDyRxsRGGs8Q7DEFgxkNmerx27/p2dfnm+o+damVFM9W54dGWlr dV/Q3JRfy/SYeG5jvDkZcoBSc4+qa4e7VUnoY+ef1y1D64QI0jxJuCLzraekfuj5iM1C 9+QfKelzWx+TmP9xk45jOrs4XAwC6zmU32JG57iiaqYUJOYYuUH++q8JjCgSPF7K3t+m Oleog/cQRLTzkb6NHKsNsjPieKjoCNiIfaBAdHTBEWkIy7sUSRX0pXVuKInv8l6rPZYc Md4s1ZZGMRD29+XAFCUhen6kwDl4ocpL0AF8HRvXJMRj+dtRLmW2twdRTnhu/gQKykEj /iDg== X-Gm-Message-State: AOJu0YyOFCTPL53l4Oo/hje679fF6HuafOQDlxvs56OheWifEDEapzlA 9CZ/CZ32pC627MPnABuGFdZrYXtXWMzECAZnKX+NCN6QJb7efcBOdixhMOL+Drfe25R3LsBjsg= = X-Google-Smtp-Source: AGHT+IHIRiwVczTCgdLDQkw1luqJF+eErra1V7ugAChJcNyWNYV7q5bYyJtHQxsUyCAGi9B4qX9lmCGDsg== X-Received: from elver.muc.corp.google.com ([2a00:79e0:9c:201:cb16:eb72:6e81:bff1]) (user=elver job=sendgmr) by 2002:a17:906:c097:b0:a34:a9e3:5524 with SMTP id f23-20020a170906c09700b00a34a9e35524mr27051ejz.5.1706522835498; Mon, 29 Jan 2024 02:07:15 -0800 (PST) Date: Mon, 29 Jan 2024 11:07:02 +0100 In-Reply-To: <20240129100708.39460-1-elver@google.com> Mime-Version: 1.0 References: <20240129100708.39460-1-elver@google.com> X-Mailer: git-send-email 2.43.0.429.g432eaa2c6b-goog Message-ID: <20240129100708.39460-2-elver@google.com> Subject: [PATCH v2 2/2] kasan: revert eviction of stack traces in generic mode From: Marco Elver To: elver@google.com, Andrew Morton Cc: Andrey Konovalov , Alexander Potapenko , Dmitry Vyukov , Vlastimil Babka , Andrey Ryabinin , Vincenzo Frascino , linux-kernel@vger.kernel.org, kasan-dev@googlegroups.com, linux-mm@kvack.org X-Stat-Signature: yfknthijb6cqprroa8idz9i6uof89ut4 X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: 03A2AC001D X-Rspam-User: X-HE-Tag: 1706522836-393078 X-HE-Meta: U2FsdGVkX18GAeSp5B96EvZQm9C3rr++00S2erm7q7CcEfVQZ/8/vPnXKP9ZnYixChpsVlkWUr9Gc3bB5L0s8zzg2jMoIZ5Eo2/39G/6HJS4oTc9GojnYv/HZznmbH3bP8gFfviGXP/nZSHO+MlAexn8GcYqiPWytHeojqSYip9tEIQkXfjXIaidQf8pnkrwunBo2rjOwv1VDKBU0VpubW7npiQmZ4P63v/n/I3HSu/LirzdMo+AUEgMTE8f2O8f2oUvTZCv+y2c0UDYLTrAwYhW6EIrB2rFRN63wy1pb+pYAlgLXhD8oBz/ysvuAc3q8s4rLG+nAhzJr09q089xrJZYGs3HzPKBDIR2FWa/LNOjQoxbIUoyNAeH8i2SWtt+Olqz+oB4pXxbCJqMGafAoM+TmnI3lSSbT+fTIH3UKB0Yvj5gvBBIF1X2+2vBnf25zG9Jh8hw6+kW363Nj2uu19jlKnDPW95JxQLUQdPtLartM8B2ZU9GJL6UBrVtWfhqIGszyGKYFNlZlYWlS8YVxfIfiX9+LiB9eZT8md2mIaSx1LmQ3cT/ua/9jmd8I2ikdEQryarqm1u0iCye/X4bqCuduW7NZJYptc9RpK0hjd4Pef//NKABaMJ+7vKvb7aILE7grKAIauZwRu8EeRaa5JMwnVUjJ/8GNZmvJ7zAQsXZiDrvS9ttN3e1KPu1NRUBFDWvX+OovvlO6HBPyMItXMllzprJiOF96JVUdkt4UlMk6tFpQm2K/7/sFLIQkE+LfoW+mzrc0GHos7iFszffnfu1PZdr4UPFiLltJucn1E3+V7TvqD6KGhuujP0qWJeR4Axnk48M+63Re9Pay2DGs+377dKKJ9ASHQt8aw6vMzk6bw6JriYOz5qfjbQcEGt3spM9H9zRIafPfw3ZNC7FOkeriD9NPVLoIp/kx8iJwdOaWKLKNmKVOkDoxV3gaabGn/38B0qHnVfhT/3GYzi Nv1L7lnR ivfo7+SKqfgAjoA4lgTK2H6GKZaNZaQ6rn3cmB8lBng1+2kIsUlJeXDEv8MSHPDC/++lq0SdBBL1CuK72T5RKxQWlfb/DcB0xDBg67vAXT+vyqXzKCGIVPcDft+lMc6yHwPi38nZrbLVr33bDQWKEb2ZUVvpBY6EhW6cSkaNq81ZT7xQ/NdRT94ctpsoSbB/VrubAe++dTWYOIqLZbUoFVdLvZlztM26Jw1iFoBPHVj7tzHumkvUCtxdTVncjCTbOR/g+inUAa5SgatOES+lwXyDXa/xjJRWE35OLMwfSTXM/N0+tOSbQ3bDEcAnXzY+N93hkKjGTOc4ylEIwl0g0FrhbKi1Kscj8ekrGZ776+YB82OXwkLSMMKRnMrHgi6FKjEbuMxHMefY/iIkS/L1QFuWiWtS9KlcShybzBLSyunkkXV2xy+HTMfZEmDScHwFAIjCkoaPMG9iGQhGHVuaBRLMqQ92xpV/9PfVG/XSzO/YEd1IqrKAp0j1w0Gb8SjSnls/WUdaq4STG800kHb6fP0WoDEuDn1OpjRVO9VfNoCE/R64XpeeBmlCAn0KQL7pnbJiCzsB2FdKTLL5gybIpcL66irdBHAFYR2y8 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: This partially reverts commits cc478e0b6bdf, 63b85ac56a64, 08d7c94d9635, a414d4286f34, and 773688a6cb24 to make use of variable-sized stack depot records, since eviction of stack entries from stack depot forces fixed- sized stack records. Care was taken to retain the code cleanups by the above commits. Eviction was added to generic KASAN as a response to alleviating the additional memory usage from fixed-sized stack records, but this still uses more memory than previously. With the re-introduction of variable-sized records for stack depot, we can just switch back to non-evictable stack records again, and return back to the previous performance and memory usage baseline. Before (observed after a KASAN kernel boot): pools: 597 refcounted_allocations: 17547 refcounted_frees: 6477 refcounted_in_use: 11070 freelist_size: 3497 persistent_count: 12163 persistent_bytes: 1717008 After: pools: 319 refcounted_allocations: 0 refcounted_frees: 0 refcounted_in_use: 0 freelist_size: 0 persistent_count: 29397 persistent_bytes: 5183536 As can be seen from the counters, with a generic KASAN config, refcounted allocations and evictions are no longer used. Due to using variable-sized records, I observe a reduction of 278 stack depot pools (saving 4448 KiB) with my test setup. Fixes: cc478e0b6bdf ("kasan: avoid resetting aux_lock") Fixes: 63b85ac56a64 ("kasan: stop leaking stack trace handles") Fixes: 08d7c94d9635 ("kasan: memset free track in qlink_free") Fixes: a414d4286f34 ("kasan: handle concurrent kasan_record_aux_stack calls") Fixes: 773688a6cb24 ("kasan: use stack_depot_put for Generic mode") Signed-off-by: Marco Elver Reviewed-by: Andrey Konovalov Cc: Alexander Potapenko Cc: Dmitry Vyukov --- v2: * Revert kasan_release_object_meta() as well (no longer needed) to catch use-after-free-before-realloc bugs. * Add more comments. --- mm/kasan/common.c | 8 ++--- mm/kasan/generic.c | 68 +++++-------------------------------------- mm/kasan/kasan.h | 10 ------- mm/kasan/quarantine.c | 5 +++- 4 files changed, 14 insertions(+), 77 deletions(-) diff --git a/mm/kasan/common.c b/mm/kasan/common.c index 610efae91220..6ca63e8dda74 100644 --- a/mm/kasan/common.c +++ b/mm/kasan/common.c @@ -65,8 +65,7 @@ void kasan_save_track(struct kasan_track *track, gfp_t flags) { depot_stack_handle_t stack; - stack = kasan_save_stack(flags, - STACK_DEPOT_FLAG_CAN_ALLOC | STACK_DEPOT_FLAG_GET); + stack = kasan_save_stack(flags, STACK_DEPOT_FLAG_CAN_ALLOC); kasan_set_track(track, stack); } @@ -266,10 +265,9 @@ bool __kasan_slab_free(struct kmem_cache *cache, void *object, return true; /* - * If the object is not put into quarantine, it will likely be quickly - * reallocated. Thus, release its metadata now. + * Note: Keep per-object metadata to allow KASAN print stack traces for + * use-after-free-before-realloc bugs. */ - kasan_release_object_meta(cache, object); /* Let slab put the object onto the freelist. */ return false; diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c index df6627f62402..fc9cf1860efb 100644 --- a/mm/kasan/generic.c +++ b/mm/kasan/generic.c @@ -485,16 +485,6 @@ void kasan_init_object_meta(struct kmem_cache *cache, const void *object) if (alloc_meta) { /* Zero out alloc meta to mark it as invalid. */ __memset(alloc_meta, 0, sizeof(*alloc_meta)); - - /* - * Prepare the lock for saving auxiliary stack traces. - * Temporarily disable KASAN bug reporting to allow instrumented - * raw_spin_lock_init to access aux_lock, which resides inside - * of a redzone. - */ - kasan_disable_current(); - raw_spin_lock_init(&alloc_meta->aux_lock); - kasan_enable_current(); } /* @@ -506,18 +496,8 @@ void kasan_init_object_meta(struct kmem_cache *cache, const void *object) static void release_alloc_meta(struct kasan_alloc_meta *meta) { - /* Evict the stack traces from stack depot. */ - stack_depot_put(meta->alloc_track.stack); - stack_depot_put(meta->aux_stack[0]); - stack_depot_put(meta->aux_stack[1]); - - /* - * Zero out alloc meta to mark it as invalid but keep aux_lock - * initialized to avoid having to reinitialize it when another object - * is allocated in the same slot. - */ - __memset(&meta->alloc_track, 0, sizeof(meta->alloc_track)); - __memset(meta->aux_stack, 0, sizeof(meta->aux_stack)); + /* Zero out alloc meta to mark it as invalid. */ + __memset(meta, 0, sizeof(*meta)); } static void release_free_meta(const void *object, struct kasan_free_meta *meta) @@ -526,27 +506,10 @@ static void release_free_meta(const void *object, struct kasan_free_meta *meta) if (*(u8 *)kasan_mem_to_shadow(object) != KASAN_SLAB_FREE_META) return; - /* Evict the stack trace from the stack depot. */ - stack_depot_put(meta->free_track.stack); - /* Mark free meta as invalid. */ *(u8 *)kasan_mem_to_shadow(object) = KASAN_SLAB_FREE; } -void kasan_release_object_meta(struct kmem_cache *cache, const void *object) -{ - struct kasan_alloc_meta *alloc_meta; - struct kasan_free_meta *free_meta; - - alloc_meta = kasan_get_alloc_meta(cache, object); - if (alloc_meta) - release_alloc_meta(alloc_meta); - - free_meta = kasan_get_free_meta(cache, object); - if (free_meta) - release_free_meta(object, free_meta); -} - size_t kasan_metadata_size(struct kmem_cache *cache, bool in_object) { struct kasan_cache *info = &cache->kasan_info; @@ -571,8 +534,6 @@ static void __kasan_record_aux_stack(void *addr, depot_flags_t depot_flags) struct kmem_cache *cache; struct kasan_alloc_meta *alloc_meta; void *object; - depot_stack_handle_t new_handle, old_handle; - unsigned long flags; if (is_kfence_address(addr) || !slab) return; @@ -583,33 +544,18 @@ static void __kasan_record_aux_stack(void *addr, depot_flags_t depot_flags) if (!alloc_meta) return; - new_handle = kasan_save_stack(0, depot_flags); - - /* - * Temporarily disable KASAN bug reporting to allow instrumented - * spinlock functions to access aux_lock, which resides inside of a - * redzone. - */ - kasan_disable_current(); - raw_spin_lock_irqsave(&alloc_meta->aux_lock, flags); - old_handle = alloc_meta->aux_stack[1]; alloc_meta->aux_stack[1] = alloc_meta->aux_stack[0]; - alloc_meta->aux_stack[0] = new_handle; - raw_spin_unlock_irqrestore(&alloc_meta->aux_lock, flags); - kasan_enable_current(); - - stack_depot_put(old_handle); + alloc_meta->aux_stack[0] = kasan_save_stack(0, depot_flags); } void kasan_record_aux_stack(void *addr) { - return __kasan_record_aux_stack(addr, - STACK_DEPOT_FLAG_CAN_ALLOC | STACK_DEPOT_FLAG_GET); + return __kasan_record_aux_stack(addr, STACK_DEPOT_FLAG_CAN_ALLOC); } void kasan_record_aux_stack_noalloc(void *addr) { - return __kasan_record_aux_stack(addr, STACK_DEPOT_FLAG_GET); + return __kasan_record_aux_stack(addr, 0); } void kasan_save_alloc_info(struct kmem_cache *cache, void *object, gfp_t flags) @@ -620,7 +566,7 @@ void kasan_save_alloc_info(struct kmem_cache *cache, void *object, gfp_t flags) if (!alloc_meta) return; - /* Evict previous stack traces (might exist for krealloc or mempool). */ + /* Invalidate previous stack traces (might exist for krealloc or mempool). */ release_alloc_meta(alloc_meta); kasan_save_track(&alloc_meta->alloc_track, flags); @@ -634,7 +580,7 @@ void kasan_save_free_info(struct kmem_cache *cache, void *object) if (!free_meta) return; - /* Evict previous stack trace (might exist for mempool). */ + /* Invalidate previous stack trace (might exist for mempool). */ release_free_meta(object, free_meta); kasan_save_track(&free_meta->free_track, 0); diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h index d0f172f2b978..fb2b9ac0659a 100644 --- a/mm/kasan/kasan.h +++ b/mm/kasan/kasan.h @@ -6,7 +6,6 @@ #include #include #include -#include #include #if defined(CONFIG_KASAN_SW_TAGS) || defined(CONFIG_KASAN_HW_TAGS) @@ -265,13 +264,6 @@ struct kasan_global { struct kasan_alloc_meta { struct kasan_track alloc_track; /* Free track is stored in kasan_free_meta. */ - /* - * aux_lock protects aux_stack from accesses from concurrent - * kasan_record_aux_stack calls. It is a raw spinlock to avoid sleeping - * on RT kernels, as kasan_record_aux_stack_noalloc can be called from - * non-sleepable contexts. - */ - raw_spinlock_t aux_lock; depot_stack_handle_t aux_stack[2]; }; @@ -398,10 +390,8 @@ struct kasan_alloc_meta *kasan_get_alloc_meta(struct kmem_cache *cache, struct kasan_free_meta *kasan_get_free_meta(struct kmem_cache *cache, const void *object); void kasan_init_object_meta(struct kmem_cache *cache, const void *object); -void kasan_release_object_meta(struct kmem_cache *cache, const void *object); #else static inline void kasan_init_object_meta(struct kmem_cache *cache, const void *object) { } -static inline void kasan_release_object_meta(struct kmem_cache *cache, const void *object) { } #endif depot_stack_handle_t kasan_save_stack(gfp_t flags, depot_flags_t depot_flags); diff --git a/mm/kasan/quarantine.c b/mm/kasan/quarantine.c index 3ba02efb952a..6958aa713c67 100644 --- a/mm/kasan/quarantine.c +++ b/mm/kasan/quarantine.c @@ -145,7 +145,10 @@ static void qlink_free(struct qlist_node *qlink, struct kmem_cache *cache) void *object = qlink_to_object(qlink, cache); struct kasan_free_meta *free_meta = kasan_get_free_meta(cache, object); - kasan_release_object_meta(cache, object); + /* + * Note: Keep per-object metadata to allow KASAN print stack traces for + * use-after-free-before-realloc bugs. + */ /* * If init_on_free is enabled and KASAN's free metadata is stored in