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 X-Spam-Level: X-Spam-Status: No, score=-10.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 850B0C4361B for ; Thu, 10 Dec 2020 11:42:24 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 12F3623D56 for ; Thu, 10 Dec 2020 11:42:24 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 12F3623D56 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 7F5B56B006E; Thu, 10 Dec 2020 06:42:23 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 7CE146B0070; Thu, 10 Dec 2020 06:42:23 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6E37D6B0071; Thu, 10 Dec 2020 06:42:23 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0023.hostedemail.com [216.40.44.23]) by kanga.kvack.org (Postfix) with ESMTP id 595686B006E for ; Thu, 10 Dec 2020 06:42:23 -0500 (EST) Received: from smtpin03.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 1077C1EE6 for ; Thu, 10 Dec 2020 11:42:23 +0000 (UTC) X-FDA: 77577184566.03.frogs99_3f03a38273f8 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin03.hostedemail.com (Postfix) with ESMTP id E26B628A4E8 for ; Thu, 10 Dec 2020 11:42:22 +0000 (UTC) X-HE-Tag: frogs99_3f03a38273f8 X-Filterd-Recvd-Size: 8201 Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by imf13.hostedemail.com (Postfix) with ESMTP for ; Thu, 10 Dec 2020 11:42:22 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 177E4AE4A; Thu, 10 Dec 2020 11:42:19 +0000 (UTC) To: paulmck@kernel.org Cc: rcu@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-team@fb.com, mingo@kernel.org, jiangshanlai@gmail.com, akpm@linux-foundation.org, mathieu.desnoyers@efficios.com, josh@joshtriplett.org, tglx@linutronix.de, peterz@infradead.org, rostedt@goodmis.org, dhowells@redhat.com, edumazet@google.com, fweisbec@gmail.com, oleg@redhat.com, joel@joelfernandes.org, iamjoonsoo.kim@lge.com, andrii@kernel.org, Christoph Lameter , Pekka Enberg , David Rientjes , linux-mm@kvack.org References: <20201209011124.GA31164@paulmck-ThinkPad-P72> <20201209011303.32737-1-paulmck@kernel.org> <8ea31887-8cc3-24cc-82e8-779290c61c2c@suse.cz> <20201209230442.GH2657@paulmck-ThinkPad-P72> From: Vlastimil Babka Subject: Re: [PATCH v2 sl-b 1/5] mm: Add mem_dump_obj() to print source of memory block Message-ID: <211cc2a9-5d94-cb0f-91bf-3415510b7dae@suse.cz> Date: Thu, 10 Dec 2020 11:48:26 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.5.1 MIME-Version: 1.0 In-Reply-To: <20201209230442.GH2657@paulmck-ThinkPad-P72> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable 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: On 12/10/20 12:04 AM, Paul E. McKenney wrote: >> > +/** >> > + * kmem_valid_obj - does the pointer reference a valid slab object? >> > + * @object: pointer to query. >> > + * >> > + * Return: %true if the pointer is to a not-yet-freed object from >> > + * kmalloc() or kmem_cache_alloc(), either %true or %false if the p= ointer >> > + * is to an already-freed object, and %false otherwise. >> > + */ >>=20 >> It should be possible to find out more about object being free or not,= than you >> currently do. At least to find out if it's definitely free. When it ap= pears >> allocated, it can be actually still free in some kind of e.g. per-cpu = or >> per-node cache that would be infeasible to check. But that improvement= to the >> output can be also added later. Also SLUB stores the freeing stacktrac= e, which >> might be useful... >=20 > I can see how this could help debugging a use-after-free situation, > at least as long as the poor sap that subsequently allocated it doesn't > free it. >=20 > I can easily add more fields to the kmem_provenance structure. Maybe > it would make sense to have another exported API that you provide a > kmem_provenance structure to, and it fills it in. >=20 > One caution though... I rely on the object being allocated. > If it officially might already be freed, complex and high-overhead > synchronization seems to be required to safely access the various data > structures. Good point! It's easy to forget that when being used to similar digging i= n a crash dump, where nothing changes. > So any use on an already-freed object is on a "you break it you get to > keep the pieces" basis. On the other hand, if you are dealing with a > use-after-free situation, life is hard anyway. Yeah, even now I think it's potentially dangerous, as you can get kmem_valid_obj() as true because PageSlab(page) is true. But the object m= ight be already free, so as soon as another CPU frees another object from the sam= e slab page, the page gets also freed... or it was already freed and then alloca= ted by another slab so it's PageSlab() again. I guess at least some safety could be achieved by pinning the page with get_page_unless_zero. But maybe your current implementation is already sa= fe, need to check in detail. > Or am I missing your point? >=20 >> > +bool kmem_valid_obj(void *object) >> > +{ >> > + struct page *page; >> > + >> > + if (!virt_addr_valid(object)) >> > + return false; >> > + page =3D virt_to_head_page(object); >> > + return PageSlab(page); >> > +} >> > +EXPORT_SYMBOL_GPL(kmem_valid_obj); >> > + >> > +/** >> > + * kmem_dump_obj - Print available slab provenance information >> > + * @object: slab object for which to find provenance information. >> > + * >> > + * This function uses pr_cont(), so that the caller is expected to = have >> > + * printed out whatever preamble is appropriate. The provenance in= formation >> > + * depends on the type of object and on how much debugging is enabl= ed. >> > + * For a slab-cache object, the fact that it is a slab object is pr= inted, >> > + * and, if available, the slab name, return address, and stack trac= e from >> > + * the allocation of that object. >> > + * >> > + * This function will splat if passed a pointer to a non-slab objec= t. >> > + * If you are not sure what type of object you have, you should ins= tead >> > + * use mem_dump_obj(). >> > + */ >> > +void kmem_dump_obj(void *object) >> > +{ >> > + int i; >> > + struct page *page; >> > + struct kmem_provenance kp; >> > + >> > + if (WARN_ON_ONCE(!virt_addr_valid(object))) >> > + return; >> > + page =3D virt_to_head_page(object); >> > + if (WARN_ON_ONCE(!PageSlab(page))) { >> > + pr_cont(" non-slab memory.\n"); >> > + return; >> > + } >> > + kp.kp_ptr =3D object; >> > + kp.kp_page =3D page; >> > + kp.kp_nstack =3D KS_ADDRS_COUNT; >> > + kmem_provenance(&kp); >>=20 >> You don't seem to be printing kp.kp_objp anywhere? (unless in later pa= tch, but >> would make sense in this patch already). >=20 > Good point! >=20 > However, please note that the various debugging options that reserve > space at the beginning. This can make the meaning of kp.kp_objp a bit > different than one might expect. Yeah, I think the best would be to match the address that kmalloc/kmem_cache_alloc() would return, thus the beginning of the object itself, so you can calculate the offset within it, etc. >> > --- a/mm/util.c >> > +++ b/mm/util.c >>=20 >> I think mm/debug.c is a better fit as it already has dump_page() of a = similar >> nature. Also you can call that from from mem_dump_obj() at least in ca= se when >> the more specific handlers fail. It will even include page_owner info = if enabled! :) >=20 > I will count this as one vote for mm/debug.c. >=20 > Two things to consider, though... First, Joonsoo suggests that the fac= t > that this produces useful information without any debugging information > enabled makes it not be debugging as such. Well there's already dump_page() which also produces information without = special configs. We're not the best subsystem in this kind of consistency... > Second, mm/debug.c does > not include either slab.h or vmalloc.h. The second might not be a > showstopper, but I was interpreting this to mean that its role was > less central. I think it can include whatever becomes needed there :) > Thanx, Paul >=20 >> Thanks, >> Vlastimil >>=20 >> > @@ -970,3 +970,28 @@ int __weak memcmp_pages(struct page *page1, str= uct page *page2) >> > kunmap_atomic(addr1); >> > return ret; >> > } >> > + >> > +/** >> > + * mem_dump_obj - Print available provenance information >> > + * @object: object for which to find provenance information. >> > + * >> > + * This function uses pr_cont(), so that the caller is expected to = have >> > + * printed out whatever preamble is appropriate. The provenance in= formation >> > + * depends on the type of object and on how much debugging is enabl= ed. >> > + * For example, for a slab-cache object, the slab name is printed, = and, >> > + * if available, the return address and stack trace from the alloca= tion >> > + * of that object. >> > + */ >> > +void mem_dump_obj(void *object) >> > +{ >> > + if (!virt_addr_valid(object)) { >> > + pr_cont(" non-paged (local) memory.\n"); >> > + return; >> > + } >> > + if (kmem_valid_obj(object)) { >> > + kmem_dump_obj(object); >> > + return; >> > + } >> > + pr_cont(" non-slab memory.\n"); >> > +} >> > +EXPORT_SYMBOL_GPL(mem_dump_obj); >> >=20 >>=20 >=20