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 5EF29C52D7F for ; Wed, 14 Aug 2024 06:50:17 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E529A6B0082; Wed, 14 Aug 2024 02:50:16 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E01686B0083; Wed, 14 Aug 2024 02:50:16 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CC91E6B0085; Wed, 14 Aug 2024 02:50:16 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id AFA636B0082 for ; Wed, 14 Aug 2024 02:50:16 -0400 (EDT) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 5D7BB14083B for ; Wed, 14 Aug 2024 06:50:16 +0000 (UTC) X-FDA: 82449926832.04.4383345 Received: from mail-vs1-f44.google.com (mail-vs1-f44.google.com [209.85.217.44]) by imf14.hostedemail.com (Postfix) with ESMTP id 9D94310000E for ; Wed, 14 Aug 2024 06:50:14 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=Qw3iwaGQ; spf=pass (imf14.hostedemail.com: domain of elver@google.com designates 209.85.217.44 as permitted sender) smtp.mailfrom=elver@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=1723618143; 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=kGYIVRMx7xSAnTXSGLiynA60xaTatAN/kEWNV4u5Zsk=; b=ECfGcj7vxW9x8vXXSDAJf9OzFFsrretcH7qR1C5tH0Wx0FKpFveF27VtkWFk63fnRIxM2l 6TH4iC8URHAI2Cy9ELVg3sZg3OVU4hHXZLFHcr4Zr8Q2pyVthE0NA7n5tvgbagVcbz99GI VESeOZ7gaOSylcaKfbi/Zcnm00ny9yU= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1723618143; a=rsa-sha256; cv=none; b=RwoYoNyJbsKOrEO0PRz1JOwUgCrdG3gyMEMBm9lJWzCCi8JZhk9MokqiNyXWfuLWVxonfG wgjtjMfOlAqHleZHcjxuya1evas9TKAuzUOgKMuZNJ+g/c9paVdHcH+rmm/N1Qvcua3ViD kvE9yEM9CdUsD0N8gROdwzHfI+ZOvOo= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=Qw3iwaGQ; spf=pass (imf14.hostedemail.com: domain of elver@google.com designates 209.85.217.44 as permitted sender) smtp.mailfrom=elver@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-vs1-f44.google.com with SMTP id ada2fe7eead31-492aae5fd78so2222004137.2 for ; Tue, 13 Aug 2024 23:50:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1723618213; x=1724223013; darn=kvack.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=kGYIVRMx7xSAnTXSGLiynA60xaTatAN/kEWNV4u5Zsk=; b=Qw3iwaGQrIItHShEBBag91VQeDFAjOVjwtrZijhnrq5LgKmcSNSqxJ1sFzm32GO4Bz JmSmgIAz3JQMn+eGuqytPRKh/l6N4w6TVao++s04KjXvz4sXU2S/Po8KDDITwpA0JCJ3 QXockaU4zagedf6vbG8dR6d5nBmWm2TbfvBJVPW8S0Gu/hHW0TL2CePsZ1o8rg6eF2og 1JuXCnIDV7IMVp/N2zhSI9CRuVm9jDAqRt4t+I+AaiCsfmtDUTwNaAxR20Y4S/mRh19N FGznCeCy5EcClaB7ePXGA96TFe3kKd4onOfoTnFoyPOxEO+akvUKgEoLD/I2xPjwi48T +YAw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723618213; x=1724223013; h=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=kGYIVRMx7xSAnTXSGLiynA60xaTatAN/kEWNV4u5Zsk=; b=xArrC6B2W9srfgNhpbHy8NYdXFZgxAChbEaOdwx9masB3dlr4u31NiAjBb3WHMVDS3 A1Qt73rHKcfz3FU7ox9gBhKIqp6MpQ21GviLIyhihcGToHEj1ai68ceWvJzHM/nQG78J ihDZ279fDYWUyJXIpU2lfS+0Wc/rSlCjiVDizRjxDgwQJmjMX2lc0di5tmTdm+OF7sUI 8EeKji6j1ow2uyoqmoF/2Udc7QCvyPWYhxtgMSTkFTFdB0RtHd9YOlxVzvuNM6tXUEkK /9t/arkgNW2c5ZCo9j2D23osR6JiBbpkZ4U+qUnh48zGnRFJ0o5VER6nxB0MGzq6n81F Ykog== X-Forwarded-Encrypted: i=1; AJvYcCWH+YYhD8wwRhkQoPykk75gHdZwdrPyUoko2QWBKvzxAaVsbV6aQjYHvMs34IDU59GbWfIHSTIruQtxiwdVsOAmSKY= X-Gm-Message-State: AOJu0YzI2y0qH3XNCDXnQbA8mwXWOxFOFTPNppPOgcsgIswn3SwQhQCO Nc4xnC9NxDah14ArnbkoeA7ea6fKE3YaNuTp9hmoxQ6zG+S9W4kmkbs1/l761GMVH0PkJSrYr6+ I6ukGl8AaE5lccDjdMMWIenwrD+1hG+XHyzUP X-Google-Smtp-Source: AGHT+IHFef/Y7C8vlNKsrswVF7Px7lxQgeUBx+R+dkNvPnYvcUA1CJlMFcH4ruFwu/5f65IQU7ns0H8DyCAc3UM8/UU= X-Received: by 2002:a05:6102:e06:b0:493:de37:b3ef with SMTP id ada2fe7eead31-49759913349mr2343105137.13.1723618213265; Tue, 13 Aug 2024 23:50:13 -0700 (PDT) MIME-Version: 1.0 References: <20240812095517.2357-1-dtcccc@linux.alibaba.com> In-Reply-To: <20240812095517.2357-1-dtcccc@linux.alibaba.com> From: Marco Elver Date: Wed, 14 Aug 2024 08:49:37 +0200 Message-ID: Subject: Re: [PATCH v2] kfence: Save freeing stack trace at calling time instead of freeing time To: Tianchen Ding Cc: linux-kernel@vger.kernel.org, Alexander Potapenko , Dmitry Vyukov , Andrew Morton , kasan-dev@googlegroups.com, linux-mm@kvack.org Content-Type: text/plain; charset="UTF-8" X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 9D94310000E X-Stat-Signature: cqnzd6wh54ezebxa1owauqgp1oydy8de X-HE-Tag: 1723618214-272150 X-HE-Meta: U2FsdGVkX19ouQxcLXNYqEqSXF6ZY/YWprhatEXyPub8p26sUH6NWbfMsDBkvHVG78lsAg3R4BL/+DgGZuhEcxfiqMr09R/h7v7vuSUET2KQU7l95M/BTA7easAtxaK9Eyi8b2PaXOG+XgHWJKJRjVekaOPnh01b2VEgGeZcWehAtAURf/6qxGqNrXerp0wYX1GzWmqp++pn4KJ2g6xt7IWL9iTK12jGa6o0p2cquV/K0rvAPTjGYLClD9IdX1uBDTLz6d10qO/Fm9Qrp9L2c+YRloBkHpPRRkPvfadBWy6dOcu+VO/Dk8A/h24hzGyKRO773/PYMGHbi7IcIGFjX2rXGwKb2I8Zvn8/I0ftPagdWcfY59PpsbiG6voKeTeEw5q2H/y0KnFuiYMqrZxhSHxlgoKzHXIHTRGfQf5DKexT1RtI+RHsPQU4xsqidEMtZqo8AWdgiowMrdM2HBpWfi0ZRWqrLZl2EkdDsl94NgxRDeRwO24gxIgZDhfQNwhQ0up0ZXUucKQ6oVnUsPBxMeXaYVqemNFHEu69d0I5gKgIF3E8inqZyJazobVyKDX4KKzkSN88w0lniwLkOvM8GkU9iGhXnKGcr+R7RubQFDE3S4Ab4HGdrkHCYkMonMQJHGH4GOoZrh97V+NNRVZ+Oj9V3Wr0EsPhaY/RWRLsMXHsN9MDRaxu19f6bxiiy6fWOzKlai6kkIJArVnRX9J2v8MHHV7aKFyunSY4BVPU4RzEW2Fp0mQ8G+UgaFgQsxHJXseeQO+k3VQ27HZOu5YQxe9najNDoTn1woxuUgM7BLiOOu+AJAgPrqbH7ANrFXf5Dy4L/uH5uJdmECGV38EbNJHRkcf9kfv2z3sV2/hYt87iCpNqXuNoj0AuZsGuccwo4+1Pc01bkFH3iZnYsspv7zmU3yFSsK4eqOn7DTvp2Dmcrirrsi5xdeHmJGLfmV7j+H+bpcf/ZGIouGNF35z lBHLYhri DmJMSrF71S+guceZLK4+N2u4gUZ0bZ9+55/Jc+m58N+0SNtYDMaqLnY7/PeWpOWlqogUCdSk4oa6TnKshi//72cA9vSGkTA879H6EakhnNQvXAYT8+n+27NEzS2RFv6amwkAoyUxKnbujl5tpzm+m0PwmFKEyXkSY+h+EB0q8DLiK1efZIaP9b8Yy0s6aWAGmzp1S9Cjz/f93yMDlhwH6Sh3eV0+6Wc6zVDuiuv+P6AIbSufLN5o7Yd6rCwXfPLdazzYUgxFjEV44+AbheAgE1EmBimeOC1BN8jx+I54brg1Uqicn/MWl6Ap2POUtoSPTzpzQDmXYkrqIsVU2zoybQtnvbnY166xkVIY5EtIKGaAZhsCqJfAI9vF2qRmtRTdllBeo 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 Mon, 12 Aug 2024 at 11:55, Tianchen Ding wrote: > > 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 Reviewed-by: Marco Elver Thanks for the patch! > --- > v2: > Rename and inline tiny helper kfence_obj_allocated(). > Improve code style and comments. > > v1: https://lore.kernel.org/all/20240812065947.6104-1-dtcccc@linux.alibaba.com/ > --- > mm/kfence/core.c | 39 +++++++++++++++++++++++++++++---------- > mm/kfence/kfence.h | 1 + > mm/kfence/report.c | 7 ++++--- > 3 files changed, 34 insertions(+), 13 deletions(-) > > diff --git a/mm/kfence/core.c b/mm/kfence/core.c > index c3ef7eb8d4dc..67fc321db79b 100644 > --- a/mm/kfence/core.c > +++ b/mm/kfence/core.c > @@ -273,6 +273,13 @@ static inline unsigned long metadata_to_pageaddr(const struct kfence_metadata *m > return pageaddr; > } > > +static inline bool kfence_obj_allocated(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. > @@ -282,10 +289,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])); > @@ -301,6 +312,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(), > @@ -506,7 +518,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_allocated(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, > @@ -784,7 +796,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_allocated(meta)) > check_canary(meta); > } > } > @@ -1010,12 +1022,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_allocated(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_allocated(meta); > raw_spin_unlock_irqrestore(&meta->lock, flags); > > if (in_use) { > @@ -1160,11 +1171,19 @@ 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 so that reports show where the user freed > + * the object. > */ > - if (unlikely(meta->cache && (meta->cache->flags & SLAB_TYPESAFE_BY_RCU))) > + if (unlikely(meta->cache && (meta->cache->flags & SLAB_TYPESAFE_BY_RCU))) { > + unsigned long flags; > + > + 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 > + } else { > kfence_guarded_free(addr, meta, false); > + } > } > > bool kfence_handle_page_fault(unsigned long addr, bool is_write, struct pt_regs *regs) > @@ -1188,14 +1207,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_allocated(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_allocated(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]; > -- > 2.39.3 > > -- > You received this message because you are subscribed to the Google Groups "kasan-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20240812095517.2357-1-dtcccc%40linux.alibaba.com.