From nobody Wed Dec 17 20:50:19 2025 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 04AC6C61D9C for ; Wed, 22 Nov 2023 10:00:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1343715AbjKVKAc (ORCPT ); Wed, 22 Nov 2023 05:00:32 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58114 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229631AbjKVKAa (ORCPT ); Wed, 22 Nov 2023 05:00:30 -0500 Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 862B5D4A; Wed, 22 Nov 2023 02:00:25 -0800 (PST) Date: Wed, 22 Nov 2023 10:00:22 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1700647223; h=from:from:sender:sender:reply-to: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:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=c/DHVQmUQG5VLUAZjQ7Yb5Js41EBQnmAVL+B/mPHlOM=; b=jg0GP3bM58CcYz+J2syvdsoUXwtOaO/na/tTWq0Ks8AZLpkvyFkVw6i6Eb/NmCqqYDIAN2 kiU4pmi95Bzy4nXDDTtN/gRyQGOuuhRElPDPd5/j6BuBakS2ke+kSkpYV4/Oh77XjR9aUT kT5+3y32ICuOWkxqWtq6mHuGkQkzxYR22AnjSWPJwpiMKqLhFYXOVKcfH9uK81ZnCjT1gJ rfvDSeRjJjqYgrj29S1+i220/9oWl+NLzsS9OaHvRukX1sW6B1vh9j6q6WAJtYVwGL0D7b qwZrGiBe69KLNxld6tHDbNzCkuGcYX+e012HmHYFDtkOQNjdBV23pXZRfcD+MQ== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1700647223; h=from:from:sender:sender:reply-to: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:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=c/DHVQmUQG5VLUAZjQ7Yb5Js41EBQnmAVL+B/mPHlOM=; b=WSyyGKBdJ0rrxBx0pQkST9cAQG1yir2alPhREDWYyrADW57eFtoQ0YfNhceFUJKGAHKKy8 t9rutETUvaEzi/AA== From: "tip-bot2 for Andrzej Hajda" Sender: tip-bot2@linutronix.de Reply-to: linux-kernel@vger.kernel.org To: linux-tip-commits@vger.kernel.org Subject: [tip: core/debugobjects] debugobjects: Stop accessing objects after releasing hash bucket lock Cc: Andrzej Hajda , Thomas Gleixner , x86@kernel.org, linux-kernel@vger.kernel.org In-Reply-To: <20231025-debugobjects_fix-v3-1-2bc3bf7084c2@intel.com> References: <20231025-debugobjects_fix-v3-1-2bc3bf7084c2@intel.com> MIME-Version: 1.0 Message-ID: <170064722243.398.4713756833379834178.tip-bot2@tip-bot2> Robot-ID: Robot-Unsubscribe: Contact to get blacklisted from these emails Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org The following commit has been merged into the core/debugobjects branch of t= ip: Commit-ID: 9bb6362652f3f4d74a87d572a91ee1b38e673ef6 Gitweb: https://git.kernel.org/tip/9bb6362652f3f4d74a87d572a91ee1b38= e673ef6 Author: Andrzej Hajda AuthorDate: Wed, 25 Oct 2023 23:39:07 +02:00 Committer: Thomas Gleixner CommitterDate: Wed, 22 Nov 2023 10:41:46 +01:00 debugobjects: Stop accessing objects after releasing hash bucket lock After release of the hashbucket lock the tracking object can be modified or freed by a concurrent thread. Using it in such a case is error prone, even for printing the object state: 1. T1 tries to deactivate destroyed object, debugobjects detects it, hash bucket lock is released. 2. T2 preempts T1 and frees the tracking object. 3. The freed tracking object is allocated and initialized for a different to be tracked kernel object. 4. T1 resumes and reports error for wrong kernel object. Create a local copy of the tracking object before releasing the hash bucket lock and use the local copy for reporting and fixups to prevent this. Signed-off-by: Andrzej Hajda Signed-off-by: Thomas Gleixner Link: https://lore.kernel.org/r/20231025-debugobjects_fix-v3-1-2bc3bf7084c2= @intel.com --- lib/debugobjects.c | 200 +++++++++++++++++--------------------------- 1 file changed, 78 insertions(+), 122 deletions(-) diff --git a/lib/debugobjects.c b/lib/debugobjects.c index 2a8e9d6..fb12a9b 100644 --- a/lib/debugobjects.c +++ b/lib/debugobjects.c @@ -620,9 +620,8 @@ static void debug_objects_fill_pool(void) static void __debug_object_init(void *addr, const struct debug_obj_descr *descr, int o= nstack) { - enum debug_obj_state state; + struct debug_obj *obj, o; struct debug_bucket *db; - struct debug_obj *obj; unsigned long flags; =20 debug_objects_fill_pool(); @@ -643,24 +642,18 @@ __debug_object_init(void *addr, const struct debug_ob= j_descr *descr, int onstack case ODEBUG_STATE_INIT: case ODEBUG_STATE_INACTIVE: obj->state =3D ODEBUG_STATE_INIT; - break; - - case ODEBUG_STATE_ACTIVE: - state =3D obj->state; - raw_spin_unlock_irqrestore(&db->lock, flags); - debug_print_object(obj, "init"); - debug_object_fixup(descr->fixup_init, addr, state); - return; - - case ODEBUG_STATE_DESTROYED: raw_spin_unlock_irqrestore(&db->lock, flags); - debug_print_object(obj, "init"); return; default: break; } =20 + o =3D *obj; raw_spin_unlock_irqrestore(&db->lock, flags); + debug_print_object(&o, "init"); + + if (o.state =3D=3D ODEBUG_STATE_ACTIVE) + debug_object_fixup(descr->fixup_init, addr, o.state); } =20 /** @@ -701,11 +694,9 @@ EXPORT_SYMBOL_GPL(debug_object_init_on_stack); int debug_object_activate(void *addr, const struct debug_obj_descr *descr) { struct debug_obj o =3D { .object =3D addr, .state =3D ODEBUG_STATE_NOTAVA= ILABLE, .descr =3D descr }; - enum debug_obj_state state; struct debug_bucket *db; struct debug_obj *obj; unsigned long flags; - int ret; =20 if (!debug_objects_enabled) return 0; @@ -717,49 +708,38 @@ int debug_object_activate(void *addr, const struct de= bug_obj_descr *descr) raw_spin_lock_irqsave(&db->lock, flags); =20 obj =3D lookup_object_or_alloc(addr, db, descr, false, true); - if (likely(!IS_ERR_OR_NULL(obj))) { - bool print_object =3D false; - + if (unlikely(!obj)) { + raw_spin_unlock_irqrestore(&db->lock, flags); + debug_objects_oom(); + return 0; + } else if (likely(!IS_ERR(obj))) { switch (obj->state) { - case ODEBUG_STATE_INIT: - case ODEBUG_STATE_INACTIVE: - obj->state =3D ODEBUG_STATE_ACTIVE; - ret =3D 0; - break; - case ODEBUG_STATE_ACTIVE: - state =3D obj->state; - raw_spin_unlock_irqrestore(&db->lock, flags); - debug_print_object(obj, "activate"); - ret =3D debug_object_fixup(descr->fixup_activate, addr, state); - return ret ? 0 : -EINVAL; - case ODEBUG_STATE_DESTROYED: - print_object =3D true; - ret =3D -EINVAL; + o =3D *obj; break; + case ODEBUG_STATE_INIT: + case ODEBUG_STATE_INACTIVE: + obj->state =3D ODEBUG_STATE_ACTIVE; + fallthrough; default: - ret =3D 0; - break; + raw_spin_unlock_irqrestore(&db->lock, flags); + return 0; } - raw_spin_unlock_irqrestore(&db->lock, flags); - if (print_object) - debug_print_object(obj, "activate"); - return ret; } =20 raw_spin_unlock_irqrestore(&db->lock, flags); + debug_print_object(&o, "activate"); =20 - /* If NULL the allocation has hit OOM */ - if (!obj) { - debug_objects_oom(); - return 0; + switch (o.state) { + case ODEBUG_STATE_ACTIVE: + case ODEBUG_STATE_NOTAVAILABLE: + if (debug_object_fixup(descr->fixup_activate, addr, o.state)) + return 0; + fallthrough; + default: + return -EINVAL; } - - /* Object is neither static nor tracked. It's not initialized */ - debug_print_object(&o, "activate"); - ret =3D debug_object_fixup(descr->fixup_activate, addr, ODEBUG_STATE_NOTA= VAILABLE); - return ret ? 0 : -EINVAL; } EXPORT_SYMBOL_GPL(debug_object_activate); =20 @@ -770,10 +750,10 @@ EXPORT_SYMBOL_GPL(debug_object_activate); */ void debug_object_deactivate(void *addr, const struct debug_obj_descr *des= cr) { + struct debug_obj o =3D { .object =3D addr, .state =3D ODEBUG_STATE_NOTAVA= ILABLE, .descr =3D descr }; struct debug_bucket *db; struct debug_obj *obj; unsigned long flags; - bool print_object =3D false; =20 if (!debug_objects_enabled) return; @@ -785,33 +765,24 @@ void debug_object_deactivate(void *addr, const struct= debug_obj_descr *descr) obj =3D lookup_object(addr, db); if (obj) { switch (obj->state) { + case ODEBUG_STATE_DESTROYED: + break; case ODEBUG_STATE_INIT: case ODEBUG_STATE_INACTIVE: case ODEBUG_STATE_ACTIVE: - if (!obj->astate) - obj->state =3D ODEBUG_STATE_INACTIVE; - else - print_object =3D true; - break; - - case ODEBUG_STATE_DESTROYED: - print_object =3D true; - break; + if (obj->astate) + break; + obj->state =3D ODEBUG_STATE_INACTIVE; + fallthrough; default: - break; + raw_spin_unlock_irqrestore(&db->lock, flags); + return; } + o =3D *obj; } =20 raw_spin_unlock_irqrestore(&db->lock, flags); - if (!obj) { - struct debug_obj o =3D { .object =3D addr, - .state =3D ODEBUG_STATE_NOTAVAILABLE, - .descr =3D descr }; - - debug_print_object(&o, "deactivate"); - } else if (print_object) { - debug_print_object(obj, "deactivate"); - } + debug_print_object(&o, "deactivate"); } EXPORT_SYMBOL_GPL(debug_object_deactivate); =20 @@ -822,11 +793,9 @@ EXPORT_SYMBOL_GPL(debug_object_deactivate); */ void debug_object_destroy(void *addr, const struct debug_obj_descr *descr) { - enum debug_obj_state state; + struct debug_obj *obj, o; struct debug_bucket *db; - struct debug_obj *obj; unsigned long flags; - bool print_object =3D false; =20 if (!debug_objects_enabled) return; @@ -836,32 +805,31 @@ void debug_object_destroy(void *addr, const struct de= bug_obj_descr *descr) raw_spin_lock_irqsave(&db->lock, flags); =20 obj =3D lookup_object(addr, db); - if (!obj) - goto out_unlock; + if (!obj) { + raw_spin_unlock_irqrestore(&db->lock, flags); + return; + } =20 switch (obj->state) { + case ODEBUG_STATE_ACTIVE: + case ODEBUG_STATE_DESTROYED: + break; case ODEBUG_STATE_NONE: case ODEBUG_STATE_INIT: case ODEBUG_STATE_INACTIVE: obj->state =3D ODEBUG_STATE_DESTROYED; - break; - case ODEBUG_STATE_ACTIVE: - state =3D obj->state; + fallthrough; + default: raw_spin_unlock_irqrestore(&db->lock, flags); - debug_print_object(obj, "destroy"); - debug_object_fixup(descr->fixup_destroy, addr, state); return; - - case ODEBUG_STATE_DESTROYED: - print_object =3D true; - break; - default: - break; } -out_unlock: + + o =3D *obj; raw_spin_unlock_irqrestore(&db->lock, flags); - if (print_object) - debug_print_object(obj, "destroy"); + debug_print_object(&o, "destroy"); + + if (o.state =3D=3D ODEBUG_STATE_ACTIVE) + debug_object_fixup(descr->fixup_destroy, addr, o.state); } EXPORT_SYMBOL_GPL(debug_object_destroy); =20 @@ -872,9 +840,8 @@ EXPORT_SYMBOL_GPL(debug_object_destroy); */ void debug_object_free(void *addr, const struct debug_obj_descr *descr) { - enum debug_obj_state state; + struct debug_obj *obj, o; struct debug_bucket *db; - struct debug_obj *obj; unsigned long flags; =20 if (!debug_objects_enabled) @@ -885,24 +852,26 @@ void debug_object_free(void *addr, const struct debug= _obj_descr *descr) raw_spin_lock_irqsave(&db->lock, flags); =20 obj =3D lookup_object(addr, db); - if (!obj) - goto out_unlock; + if (!obj) { + raw_spin_unlock_irqrestore(&db->lock, flags); + return; + } =20 switch (obj->state) { case ODEBUG_STATE_ACTIVE: - state =3D obj->state; - raw_spin_unlock_irqrestore(&db->lock, flags); - debug_print_object(obj, "free"); - debug_object_fixup(descr->fixup_free, addr, state); - return; + break; default: hlist_del(&obj->node); raw_spin_unlock_irqrestore(&db->lock, flags); free_object(obj); return; } -out_unlock: + + o =3D *obj; raw_spin_unlock_irqrestore(&db->lock, flags); + debug_print_object(&o, "free"); + + debug_object_fixup(descr->fixup_free, addr, o.state); } EXPORT_SYMBOL_GPL(debug_object_free); =20 @@ -954,10 +923,10 @@ void debug_object_active_state(void *addr, const struct debug_obj_descr *descr, unsigned int expect, unsigned int next) { + struct debug_obj o =3D { .object =3D addr, .state =3D ODEBUG_STATE_NOTAVA= ILABLE, .descr =3D descr }; struct debug_bucket *db; struct debug_obj *obj; unsigned long flags; - bool print_object =3D false; =20 if (!debug_objects_enabled) return; @@ -970,28 +939,19 @@ debug_object_active_state(void *addr, const struct de= bug_obj_descr *descr, if (obj) { switch (obj->state) { case ODEBUG_STATE_ACTIVE: - if (obj->astate =3D=3D expect) - obj->astate =3D next; - else - print_object =3D true; - break; - + if (obj->astate !=3D expect) + break; + obj->astate =3D next; + raw_spin_unlock_irqrestore(&db->lock, flags); + return; default: - print_object =3D true; break; } + o =3D *obj; } =20 raw_spin_unlock_irqrestore(&db->lock, flags); - if (!obj) { - struct debug_obj o =3D { .object =3D addr, - .state =3D ODEBUG_STATE_NOTAVAILABLE, - .descr =3D descr }; - - debug_print_object(&o, "active_state"); - } else if (print_object) { - debug_print_object(obj, "active_state"); - } + debug_print_object(&o, "active_state"); } EXPORT_SYMBOL_GPL(debug_object_active_state); =20 @@ -999,12 +959,10 @@ EXPORT_SYMBOL_GPL(debug_object_active_state); static void __debug_check_no_obj_freed(const void *address, unsigned long = size) { unsigned long flags, oaddr, saddr, eaddr, paddr, chunks; - const struct debug_obj_descr *descr; - enum debug_obj_state state; + int cnt, objs_checked =3D 0; + struct debug_obj *obj, o; struct debug_bucket *db; struct hlist_node *tmp; - struct debug_obj *obj; - int cnt, objs_checked =3D 0; =20 saddr =3D (unsigned long) address; eaddr =3D saddr + size; @@ -1026,12 +984,10 @@ repeat: =20 switch (obj->state) { case ODEBUG_STATE_ACTIVE: - descr =3D obj->descr; - state =3D obj->state; + o =3D *obj; raw_spin_unlock_irqrestore(&db->lock, flags); - debug_print_object(obj, "free"); - debug_object_fixup(descr->fixup_free, - (void *) oaddr, state); + debug_print_object(&o, "free"); + debug_object_fixup(o.descr->fixup_free, (void *)oaddr, o.state); goto repeat; default: hlist_del(&obj->node);