From patchwork Mon Aug 12 06:59:47 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tianchen Ding X-Patchwork-Id: 13760215 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 9D28EC3DA7F for ; Mon, 12 Aug 2024 07:00:06 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 28BFC6B009F; Mon, 12 Aug 2024 03:00:06 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 23C466B00A0; Mon, 12 Aug 2024 03:00:06 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 104F36B00A1; Mon, 12 Aug 2024 03:00:06 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id E43596B009F for ; Mon, 12 Aug 2024 03:00:05 -0400 (EDT) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 887D28063C for ; Mon, 12 Aug 2024 07:00:05 +0000 (UTC) X-FDA: 82442693970.13.98417D9 Received: from out30-113.freemail.mail.aliyun.com (out30-113.freemail.mail.aliyun.com [115.124.30.113]) by imf08.hostedemail.com (Postfix) with ESMTP id 0F58C160027 for ; Mon, 12 Aug 2024 07:00:01 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=linux.alibaba.com header.s=default header.b=tDRTblpm; spf=pass (imf08.hostedemail.com: domain of dtcccc@linux.alibaba.com designates 115.124.30.113 as permitted sender) smtp.mailfrom=dtcccc@linux.alibaba.com; dmarc=pass (policy=none) header.from=linux.alibaba.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1723445969; a=rsa-sha256; cv=none; b=Rak/BiEZH3kXtHXFxPVHsSnf3IXJc0GyinGCMoK25UPgpvdmG8ZOhMx5YExsYOwtOvuYKs xR9qHq6H7p6RhAdRTWti5BDg/O+NLEQMYqrEGTYcl7JKbBQzyAktsf+e1TvshlaYXIO5CD lmZVIFnBA7rsY6EELPh5/Gb4vDBXO70= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=linux.alibaba.com header.s=default header.b=tDRTblpm; spf=pass (imf08.hostedemail.com: domain of dtcccc@linux.alibaba.com designates 115.124.30.113 as permitted sender) smtp.mailfrom=dtcccc@linux.alibaba.com; dmarc=pass (policy=none) header.from=linux.alibaba.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1723445968; 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-transfer-encoding:content-transfer-encoding: in-reply-to:references:dkim-signature; bh=2sB5TDcVkqlobWIVqrnBXmc0iuAglrquanWdPCiDDFo=; b=SQ1Cd83m9lPyu8nDlciEt/ivbCucdt+vJflzN/Hsj24yZJ9aOIIMp2JEc/w96K89gHQmt0 Ez86y49FvmEZoYM22mpAymZ2LTlWkmKhJLthumjIGKshXp67IIu0me3dIYORyUSaKoiRDf 6H3hazA1AX65S7sSLReGPF0JOZ3o0JE= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.alibaba.com; s=default; t=1723445997; h=From:To:Subject:Date:Message-Id:MIME-Version; bh=2sB5TDcVkqlobWIVqrnBXmc0iuAglrquanWdPCiDDFo=; b=tDRTblpmIDz98b4RhkqdLxYEMjG81wGkHvQEPT3ea1z6WZQnUkfYBjiGLBSZ8iRiPwvnMVnIHHg+1NhIkN2gHLQKBIdq4xO2+ucM2QNaWvrkKv886mfmujeQAxz5UyDC/hGaPbbSagxP/yngyW+MI7tE3vQZcWD5PudpIzwpI2E= Received: from localhost.localdomain(mailfrom:dtcccc@linux.alibaba.com fp:SMTPD_---0WCaX4Nj_1723445987) by smtp.aliyun-inc.com; Mon, 12 Aug 2024 14:59:55 +0800 From: Tianchen Ding To: linux-kernel@vger.kernel.org Cc: Marco Elver , Alexander Potapenko , Dmitry Vyukov , Andrew Morton , kasan-dev@googlegroups.com, linux-mm@kvack.org Subject: [PATCH] kfence: Save freeing stack trace at calling time instead of freeing time Date: Mon, 12 Aug 2024 14:59:47 +0800 Message-Id: <20240812065947.6104-1-dtcccc@linux.alibaba.com> X-Mailer: git-send-email 2.39.3 MIME-Version: 1.0 X-Stat-Signature: ch56w8tf8un9nrszcfnk6p4hmj5zf5xd X-Rspamd-Queue-Id: 0F58C160027 X-Rspam-User: X-Rspamd-Server: rspam10 X-HE-Tag: 1723446001-492516 X-HE-Meta: U2FsdGVkX18AE8PP4d7q/haQGbZa0VqMUcI7oh+PB3P6qHmbhMyB6/qmDbMQOCf7BXQmG9usR6b7nRBdG6J4bhAtI4Yd/9qVx2sZrf00eNKxO8ZxuOZLve0OeOxkW819JZ9LJER/azgO32aTsXLJx+2sfZPKt7KW+z+/iKSae1xp+LLlHBZtNiTwsTGFeTVRqNhE35aUCrxj0ar2nu3CpkUVLir696LVbfhUuJ/0je4VajS1Jc94PyzhwHJiAKYJiYog6SNQVJPinVWn7WxyFDoytSNPcC3yMtoIMUbUU0lWmifQpPbibTfVjh6UjdtXZNigh7hWHbRHh8tsHXUgXLzaj0zMQk3Px6axlslB4Lu6AiUm7k6LPnugi8YmEjBJVpPbUjuid8eDqozZz+zh3A5uciKHFozCqB/EuN69iAsyZTRXabdKhE8i7AGD3oxiXhWr8R7L+WQ1i9FlGDndBl1ZPqSgtIu5DC4v/OihhlIz7rsyOOiDNiKe3qB+Nz/XGzMQV24kuvsuIFtKenRqa91Xp9/Hb4I7vl2s+RhQ5aL6y8BfYBYdB789TS1Hn3vtZcIvJPwUvlOae7Ah0FiHswyFvWQTZtezprlw/uR1YonlHMNvMcntZAMlAKeVSHsbmqbK3U3y7zJglPaLI6ssd18pQ3EZY6qqk0K6COdo72Tm5yLm7pWr1gV86ww96wKp8pg48cOqBX36t860QIt8hxrwnL8npsCll/k0EfGVqOfiHNvqJNa/y5IWWguN0UUsfDcC+EGoLirTZB3d+Q9rjUylkeCKfc9Q0L6POJOVocUM1o+iA3EBek1crNHR0DxLIn+XpXCDws2SdBHO7UM4ekfCFMfMYuq7FQ95DNp8PyLPVHHJylu1OKnSl5FPYmgw3gRfYSsudfuVi7zVJt7DbiLaChTXmKvAT829qqfsVuqZ4zDj1Ck4UOQnxie97oJGp6Xhwj4d9rAdkacV1t6 9pOAtbj4 u6ugNH78uyPt/AnJWKS4lEGdaZ5DUR8TAcO/6WYRCvoKq21lnTgAI+/fvVgq3DJ1CTnZ/0aZBHXTdRmtOwzTSDn/oI6XFyY/2n31DU3jp+2jUTVsKGsTK/zph2qEpxqOPiM8h7DGEUlEqTfYEJwzZP59uv9MOLjX00bNcv+ysic3Q7t5tHNn1E8oUD39JUqlPP0glwcz2g0O7hUPO8xi2HWFebNsWUQbnEnew 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: For kmem_cache with SLAB_TYPESAFE_BY_RCU, the freeing trace stack at calling kmem_cache_free() is more useful. While the following stack is meaningless and provides no help: freed by task 46 on cpu 0 at 656.840729s: rcu_do_batch+0x1ab/0x540 nocb_cb_wait+0x8f/0x260 rcu_nocb_cb_kthread+0x25/0x80 kthread+0xd2/0x100 ret_from_fork+0x34/0x50 ret_from_fork_asm+0x1a/0x30 Signed-off-by: Tianchen Ding --- I'm not sure whether we should keep KFENCE_OBJECT_FREED info remained (maybe the exact free time can be helpful?). But add a new kfence_track will cost more memory, so I prefer to reuse free_track and drop the info when when KFENCE_OBJECT_RCU_FREEING -> KFENCE_OBJECT_FREED. --- mm/kfence/core.c | 35 ++++++++++++++++++++++++++--------- mm/kfence/kfence.h | 1 + mm/kfence/report.c | 7 ++++--- 3 files changed, 31 insertions(+), 12 deletions(-) diff --git a/mm/kfence/core.c b/mm/kfence/core.c index c5cb54fc696d..89469d4f2d95 100644 --- a/mm/kfence/core.c +++ b/mm/kfence/core.c @@ -269,6 +269,13 @@ static inline unsigned long metadata_to_pageaddr(const struct kfence_metadata *m return pageaddr; } +static bool kfence_obj_inuse(const struct kfence_metadata *meta) +{ + enum kfence_object_state state = READ_ONCE(meta->state); + + return state == KFENCE_OBJECT_ALLOCATED || state == KFENCE_OBJECT_RCU_FREEING; +} + /* * Update the object's metadata state, including updating the alloc/free stacks * depending on the state transition. @@ -278,10 +285,14 @@ metadata_update_state(struct kfence_metadata *meta, enum kfence_object_state nex unsigned long *stack_entries, size_t num_stack_entries) { struct kfence_track *track = - next == KFENCE_OBJECT_FREED ? &meta->free_track : &meta->alloc_track; + next == KFENCE_OBJECT_ALLOCATED ? &meta->alloc_track : &meta->free_track; lockdep_assert_held(&meta->lock); + /* Stack has been saved when calling rcu, skip. */ + if (READ_ONCE(meta->state) == KFENCE_OBJECT_RCU_FREEING) + goto out; + if (stack_entries) { memcpy(track->stack_entries, stack_entries, num_stack_entries * sizeof(stack_entries[0])); @@ -297,6 +308,7 @@ metadata_update_state(struct kfence_metadata *meta, enum kfence_object_state nex track->cpu = raw_smp_processor_id(); track->ts_nsec = local_clock(); /* Same source as printk timestamps. */ +out: /* * Pairs with READ_ONCE() in * kfence_shutdown_cache(), @@ -502,7 +514,7 @@ static void kfence_guarded_free(void *addr, struct kfence_metadata *meta, bool z raw_spin_lock_irqsave(&meta->lock, flags); - if (meta->state != KFENCE_OBJECT_ALLOCATED || meta->addr != (unsigned long)addr) { + if (!kfence_obj_inuse(meta) || meta->addr != (unsigned long)addr) { /* Invalid or double-free, bail out. */ atomic_long_inc(&counters[KFENCE_COUNTER_BUGS]); kfence_report_error((unsigned long)addr, false, NULL, meta, @@ -780,7 +792,7 @@ static void kfence_check_all_canary(void) for (i = 0; i < CONFIG_KFENCE_NUM_OBJECTS; i++) { struct kfence_metadata *meta = &kfence_metadata[i]; - if (meta->state == KFENCE_OBJECT_ALLOCATED) + if (kfence_obj_inuse(meta)) check_canary(meta); } } @@ -1006,12 +1018,11 @@ void kfence_shutdown_cache(struct kmem_cache *s) * the lock will not help, as different critical section * serialization will have the same outcome. */ - if (READ_ONCE(meta->cache) != s || - READ_ONCE(meta->state) != KFENCE_OBJECT_ALLOCATED) + if (READ_ONCE(meta->cache) != s || !kfence_obj_inuse(meta)) continue; raw_spin_lock_irqsave(&meta->lock, flags); - in_use = meta->cache == s && meta->state == KFENCE_OBJECT_ALLOCATED; + in_use = meta->cache == s && kfence_obj_inuse(meta); raw_spin_unlock_irqrestore(&meta->lock, flags); if (in_use) { @@ -1145,6 +1156,7 @@ void *kfence_object_start(const void *addr) void __kfence_free(void *addr) { struct kfence_metadata *meta = addr_to_metadata((unsigned long)addr); + unsigned long flags; #ifdef CONFIG_MEMCG KFENCE_WARN_ON(meta->obj_exts.objcg); @@ -1154,9 +1166,14 @@ void __kfence_free(void *addr) * the object, as the object page may be recycled for other-typed * objects once it has been freed. meta->cache may be NULL if the cache * was destroyed. + * Save the stack trace here. It is more useful. */ - if (unlikely(meta->cache && (meta->cache->flags & SLAB_TYPESAFE_BY_RCU))) + if (unlikely(meta->cache && (meta->cache->flags & SLAB_TYPESAFE_BY_RCU))) { + raw_spin_lock_irqsave(&meta->lock, flags); + metadata_update_state(meta, KFENCE_OBJECT_RCU_FREEING, NULL, 0); + raw_spin_unlock_irqrestore(&meta->lock, flags); call_rcu(&meta->rcu_head, rcu_guarded_free); + } else kfence_guarded_free(addr, meta, false); } @@ -1182,14 +1199,14 @@ bool kfence_handle_page_fault(unsigned long addr, bool is_write, struct pt_regs int distance = 0; meta = addr_to_metadata(addr - PAGE_SIZE); - if (meta && READ_ONCE(meta->state) == KFENCE_OBJECT_ALLOCATED) { + if (meta && kfence_obj_inuse(meta)) { to_report = meta; /* Data race ok; distance calculation approximate. */ distance = addr - data_race(meta->addr + meta->size); } meta = addr_to_metadata(addr + PAGE_SIZE); - if (meta && READ_ONCE(meta->state) == KFENCE_OBJECT_ALLOCATED) { + if (meta && kfence_obj_inuse(meta)) { /* Data race ok; distance calculation approximate. */ if (!to_report || distance > data_race(meta->addr) - addr) to_report = meta; diff --git a/mm/kfence/kfence.h b/mm/kfence/kfence.h index db87a05047bd..dfba5ea06b01 100644 --- a/mm/kfence/kfence.h +++ b/mm/kfence/kfence.h @@ -38,6 +38,7 @@ enum kfence_object_state { KFENCE_OBJECT_UNUSED, /* Object is unused. */ KFENCE_OBJECT_ALLOCATED, /* Object is currently allocated. */ + KFENCE_OBJECT_RCU_FREEING, /* Object was allocated, and then being freed by rcu. */ KFENCE_OBJECT_FREED, /* Object was allocated, and then freed. */ }; diff --git a/mm/kfence/report.c b/mm/kfence/report.c index 73a6fe42845a..451991a3a8f2 100644 --- a/mm/kfence/report.c +++ b/mm/kfence/report.c @@ -114,7 +114,8 @@ static void kfence_print_stack(struct seq_file *seq, const struct kfence_metadat /* Timestamp matches printk timestamp format. */ seq_con_printf(seq, "%s by task %d on cpu %d at %lu.%06lus (%lu.%06lus ago):\n", - show_alloc ? "allocated" : "freed", track->pid, + show_alloc ? "allocated" : meta->state == KFENCE_OBJECT_RCU_FREEING ? + "rcu freeing" : "freed", track->pid, track->cpu, (unsigned long)ts_sec, rem_nsec / 1000, (unsigned long)interval_nsec, rem_interval_nsec / 1000); @@ -149,7 +150,7 @@ void kfence_print_object(struct seq_file *seq, const struct kfence_metadata *met kfence_print_stack(seq, meta, true); - if (meta->state == KFENCE_OBJECT_FREED) { + if (meta->state == KFENCE_OBJECT_FREED || meta->state == KFENCE_OBJECT_RCU_FREEING) { seq_con_printf(seq, "\n"); kfence_print_stack(seq, meta, false); } @@ -318,7 +319,7 @@ bool __kfence_obj_info(struct kmem_obj_info *kpp, void *object, struct slab *sla kpp->kp_slab_cache = meta->cache; kpp->kp_objp = (void *)meta->addr; kfence_to_kp_stack(&meta->alloc_track, kpp->kp_stack); - if (meta->state == KFENCE_OBJECT_FREED) + if (meta->state == KFENCE_OBJECT_FREED || meta->state == KFENCE_OBJECT_RCU_FREEING) kfence_to_kp_stack(&meta->free_track, kpp->kp_free_stack); /* get_stack_skipnr() ensures the first entry is outside allocator. */ kpp->kp_ret = kpp->kp_stack[0];