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 50A80C5478C for ; Wed, 28 Feb 2024 19:15:13 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id BAE9B6B0095; Wed, 28 Feb 2024 14:15:12 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id B5DDA6B0098; Wed, 28 Feb 2024 14:15:12 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9FEC26B009C; Wed, 28 Feb 2024 14:15:12 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 8E8D16B0095 for ; Wed, 28 Feb 2024 14:15:12 -0500 (EST) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 3CDD840C25 for ; Wed, 28 Feb 2024 19:15:12 +0000 (UTC) X-FDA: 81842165664.20.64F7B45 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf10.hostedemail.com (Postfix) with ESMTP id ADD63C0007 for ; Wed, 28 Feb 2024 19:15:10 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b="LeizH/TK"; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf10.hostedemail.com: domain of longman@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=longman@redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1709147710; 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=MljlOyunqu2jlYMmKl9aQOW/JRFU3MLIYOB1XnVuGaw=; b=2N5ZVx6Y6WrAyMCZIZ9iLdbPxrw2LwrVJc3Gyyi80x0mFchbKVMS0n2k7mXUteETGhshSK XPERYhWWCUi+ki6qj0Ml1qJZPskadQz7D4BT89OT+ZylunixU3cyQ38T9bwtvF3/fXqE1l YsHFDZsUHOSs5Y7ThposvHFLstI45zI= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b="LeizH/TK"; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf10.hostedemail.com: domain of longman@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=longman@redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1709147710; a=rsa-sha256; cv=none; b=20cjRA1tOWvb/kI1evRk1V6mWX1kh+XvxPNSvMy9b+4r3E4mjRiwHmJZRdzQCp0E48853G U4PMWHbt6a1ADll1Y8xuEuoY/qN9VTLu5DrurteU143sjZ76z9NtEVwJzr2cPi4hzB/qaC jVklHt3vQ7TiTl/gpGS7jCjmHmMwHFo= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1709147710; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=MljlOyunqu2jlYMmKl9aQOW/JRFU3MLIYOB1XnVuGaw=; b=LeizH/TKs3LRj5LpcSHrV57tcyFqofuzcnrkckI8if8V9S/0uFBO6gNvIZKgDMHOpjR0wB hTzbP+3IcocHDEiQHEt+2mhPk4JUQFnverk718L+VlDC+TGYrDb6Llr177amC52fjLsW6v +30SL7V9x40EgmptaK4afat3kWh37ZI= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-333-nZC1LXXDMsqMC0i9D60IlA-1; Wed, 28 Feb 2024 14:15:06 -0500 X-MC-Unique: nZC1LXXDMsqMC0i9D60IlA-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 91FC71C592A2; Wed, 28 Feb 2024 19:15:04 +0000 (UTC) Received: from llong.com (unknown [10.22.9.68]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6548A40C94AB; Wed, 28 Feb 2024 19:15:03 +0000 (UTC) From: Waiman Long To: Catalin Marinas , Andrew Morton Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Audra Mitchell , Waiman Long Subject: [PATCH] mm/kmemleak: Don't hold kmemleak_lock when calling printk() Date: Wed, 28 Feb 2024 14:14:44 -0500 Message-Id: <20240228191444.481048-1-longman@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.2 X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: ADD63C0007 X-Stat-Signature: 47nh3fjp43ckedw7zk98qubewakuut5j X-HE-Tag: 1709147710-315957 X-HE-Meta: U2FsdGVkX18S+tB4qI5jTek1in+26w6G/c1cFpj/fI45JfVy6VeFGNQVCpHimclFrk2MH8jBdJbrKvnwdAsWbtd/J2ocyZqkLHi8JNfgEou6yCb+1tAtxKoFuEr23C+g4nh/OtVyRrUBrbK3DOUdN5URAmqj20xrVu3MOQ0DL2GHEJQEBW9fN2Hqfdh9J4ysjz3IPT6fsVSdhBsSkE6A/1HpXj+yOnruYua4CfRgXv9ryL4Gq2ze5VBN/NWaSd+FHLk81GZy43vc6RTBfh3z7Is3Z482eB6u92y5zz4OjbArrGn7mQNvON/5TosOHERtQ+kK8qez3rKoGhh5fEUtALQ+IEQDGNAmIUCRxypqrl77BzckG4WaOatcR4rk5glO1IBF+jL4hbYwfepQqb4PywyEziOPdgIkFL6kxwyFXegi8U8gr16NA7bBtEh4Ka6egoJaeZtDrRTBV30j0e71n7jduzVe1kEA5WYctsEpoqfF7nGGn9CjQQd09o5jcAXUiXft3PMQ7ZIA8u/V6g8/lT36HDxmm32EEIValqQKk6IbyoAv0emJ7yHYAmQpw525owqsaK27OOIHl8wLm/QWHBAwkRePBzjTAn6VUCLiOs7vy0YtzikrkI756YKZAO8FISvzlopY2utcFhv8MEhdlFT2MTKyQ+XDhzrLLO4K4dG20+ZCUSsHjAA1Osr+jKhEm0BaGF+/d0upuBSZJsa1A1q8J7yTz1icEbt8iRcxlIzABusS7dDeJrfBT3+90yC5GWnpzqhVreqchwzWQCNcAlO6cyJ11TNLfVAqU0QMcXNj85BDOOs8y3FkAK98CYVof4U50gAHJqUIMYWHcCSh1aFGeAbIbXcwPT8XQJCtybqZG+Wk4S0u2FuUMjs53BxrSg4h3ArhzgkGZB3cwir2iPFnj2Viwcc277hjhUUVAWOEdUp3Wwa+FT0Wt9ybOxbB 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: When some error conditions happen (like OOM), some kmemleak functions call printk() to dump out some useful debugging information while holding the kmemleak_lock. This may cause deadlock as the printk() function may need to allocate additional memory leading to a create_object() call acquiring kmemleak_lock again. Fix this deadlock issue by making sure that printk() is only called after releasing the kmemleak_lock. Signed-off-by: Waiman Long --- mm/kmemleak.c | 64 ++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 46 insertions(+), 18 deletions(-) diff --git a/mm/kmemleak.c b/mm/kmemleak.c index 6a540c2b27c5..acd8742c80b5 100644 --- a/mm/kmemleak.c +++ b/mm/kmemleak.c @@ -401,6 +401,19 @@ static struct rb_root *object_tree(unsigned long objflags) return &object_tree_root; } +/* + * Increment the object use_count. Return 1 if successful or 0 otherwise. Note + * that once an object's use_count reached 0, the RCU freeing was already + * registered and the object should no longer be used. This function must be + * called under the protection of rcu_read_lock(). + */ +static int get_object(struct kmemleak_object *object) +{ + return atomic_inc_not_zero(&object->use_count); +} + +static void put_object(struct kmemleak_object *object); + /* * Look-up a memory block metadata (kmemleak_object) in the object search * tree based on a pointer value. If alias is 0, only values pointing to the @@ -413,6 +426,8 @@ static struct kmemleak_object *__lookup_object(unsigned long ptr, int alias, struct rb_node *rb = object_tree(objflags)->rb_node; unsigned long untagged_ptr = (unsigned long)kasan_reset_tag((void *)ptr); + lockdep_assert_held(&kmemleak_lock); + while (rb) { struct kmemleak_object *object; unsigned long untagged_objp; @@ -427,9 +442,20 @@ static struct kmemleak_object *__lookup_object(unsigned long ptr, int alias, else if (untagged_objp == untagged_ptr || alias) return object; else { + if (!get_object(object)) + break; + /* + * Release kmemleak_lock and acquire object->lock + * temporarily to avoid deadlock in printk(). + */ + raw_spin_unlock(&kmemleak_lock); kmemleak_warn("Found object by alias at 0x%08lx\n", ptr); + raw_spin_lock(&object->lock); dump_object_info(object); + raw_spin_unlock(&object->lock); + put_object(object); + raw_spin_lock(&kmemleak_lock); break; } } @@ -442,22 +468,12 @@ static struct kmemleak_object *lookup_object(unsigned long ptr, int alias) return __lookup_object(ptr, alias, 0); } -/* - * Increment the object use_count. Return 1 if successful or 0 otherwise. Note - * that once an object's use_count reached 0, the RCU freeing was already - * registered and the object should no longer be used. This function must be - * called under the protection of rcu_read_lock(). - */ -static int get_object(struct kmemleak_object *object) -{ - return atomic_inc_not_zero(&object->use_count); -} - /* * Memory pool allocation and freeing. kmemleak_lock must not be held. */ static struct kmemleak_object *mem_pool_alloc(gfp_t gfp) { + bool warn = false; unsigned long flags; struct kmemleak_object *object; @@ -477,9 +493,11 @@ static struct kmemleak_object *mem_pool_alloc(gfp_t gfp) else if (mem_pool_free_count) object = &mem_pool[--mem_pool_free_count]; else - pr_warn_once("Memory pool empty, consider increasing CONFIG_DEBUG_KMEMLEAK_MEM_POOL_SIZE\n"); + warn = true; raw_spin_unlock_irqrestore(&kmemleak_lock, flags); + if (unlikely(warn)) + pr_warn_once("Memory pool empty, consider increasing CONFIG_DEBUG_KMEMLEAK_MEM_POOL_SIZE\n"); return object; } @@ -692,6 +710,8 @@ static int __link_object(struct kmemleak_object *object, unsigned long ptr, unsigned long untagged_ptr; unsigned long untagged_objp; + lockdep_assert_held(&kmemleak_lock); + object->flags = OBJECT_ALLOCATED | objflags; object->pointer = ptr; object->size = kfence_ksize((void *)ptr) ?: size; @@ -718,13 +738,21 @@ static int __link_object(struct kmemleak_object *object, unsigned long ptr, else if (untagged_objp + parent->size <= untagged_ptr) link = &parent->rb_node.rb_right; else { - kmemleak_stop("Cannot insert 0x%lx into the object search tree (overlaps existing)\n", - ptr); + if (!get_object(parent)) + return -EEXIST; /* - * No need for parent->lock here since "parent" cannot - * be freed while the kmemleak_lock is held. + * Release kmemleak_lock & acquire parent->lock + * temporarily to avoid deadlock in printk(). */ + raw_spin_unlock(&kmemleak_lock); + + kmemleak_stop("Cannot insert 0x%lx into the object search tree (overlaps existing)\n", + ptr); + raw_spin_lock(&parent->lock); dump_object_info(parent); + raw_spin_unlock(&parent->lock); + put_object(parent); + raw_spin_lock(&kmemleak_lock); return -EEXIST; } } @@ -839,11 +867,12 @@ static void delete_object_part(unsigned long ptr, size_t size, raw_spin_lock_irqsave(&kmemleak_lock, flags); object = __find_and_remove_object(ptr, 1, objflags); if (!object) { + raw_spin_unlock_irqrestore(&kmemleak_lock, flags); #ifdef DEBUG kmemleak_warn("Partially freeing unknown object at 0x%08lx (size %zu)\n", ptr, size); #endif - goto unlock; + goto out; } /* @@ -862,7 +891,6 @@ static void delete_object_part(unsigned long ptr, size_t size, object->min_count, objflags)) object_r = NULL; -unlock: raw_spin_unlock_irqrestore(&kmemleak_lock, flags); if (object) __delete_object(object); -- 2.39.3