From patchwork Mon Apr 24 18:13:23 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thomas Gleixner X-Patchwork-Id: 87132 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp2932787vqo; Mon, 24 Apr 2023 11:28:43 -0700 (PDT) X-Google-Smtp-Source: AKy350YTDnu5tHqN2Hc8FQUSJ0zUdT1szRYOl4EJaFv4D1fvNDmRbDZvXqqz+WnGZR2IHs9nH/Y2 X-Received: by 2002:a05:6a00:178b:b0:63a:fb40:891a with SMTP id s11-20020a056a00178b00b0063afb40891amr22392583pfg.2.1682360923713; Mon, 24 Apr 2023 11:28:43 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1682360923; cv=none; d=google.com; s=arc-20160816; b=C07nd7JVPzm1b65mcYNDABIvrMyRRf9dSuPYNwMDuB4rWG1J2LNJi3L5UUEQzGSD/J 3t17oxozCqCeNLE9dy03vRLJ4ZzrC7VuwlQWXijqfaz1UjKHoIwSN6npwKIARHjRmcw3 hY+7jt2WNcMbyD9zx2odB6r3r/o/g+wtUvYqNIEOk1qztGGhjJnxXU0Q4pZ2CEJpPA0B faYZUdMuPV3dEAZR/lkRc995uh5BAsAO44HwvMoJLeZgHCmymIJEXCgH6ArLmeT29+X/ pInGvuijpOtPsk4UIeZ5JeyxksJeftvBxY9Yykc+rMEmM8uYNkdQd9xNkpp9hr0b/dz0 bCig== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:date:mime-version:content-transfer-encoding :message-id:subject:cc:to:dkim-signature:dkim-signature:from; bh=K9W8NDx80UQqvWkG+atar6d9T4uMp1T5rDH7i9DlOhk=; b=B0pNLmstNM4neVhkhRRL5pIC0SVfmhTA2hfc1Pzu5VrlyUrUTpcw721An4SacV7qMZ wtreMV9FP9KoqsBE6JUdI9LjXMQoAprym2GP1j5yvbsnbZyuZEkbtxffAoHvWMY7LEf+ 4VmR4I99zg2Xsww6si5J+CRWzGoWeG/xI0iz3tMtPv9Iof7s43/+AhjPNXc51dPF4hWN QuKXx4rFqNkAPBtlLiSamYqfN2P5/XxVaNmoHXN6+qiwzmnNgK7Yc7TWixjjPifeJXOu Ir2S/M7xuEVNp8nahwQgjuxwrBB/9byWtmTDmxel+udmFlX9Cf6959vDhY/SBHUVEJrZ Sh/A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linutronix.de header.s=2020 header.b=qiVeV3A4; dkim=neutral (no key) header.i=@linutronix.de header.b=+HqOswjY; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=linutronix.de Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id l123-20020a622581000000b0063d232464absi11623347pfl.214.2023.04.24.11.28.29; Mon, 24 Apr 2023 11:28:43 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@linutronix.de header.s=2020 header.b=qiVeV3A4; dkim=neutral (no key) header.i=@linutronix.de header.b=+HqOswjY; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=linutronix.de Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232302AbjDXSNa (ORCPT + 99 others); Mon, 24 Apr 2023 14:13:30 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51756 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231985AbjDXSN2 (ORCPT ); Mon, 24 Apr 2023 14:13:28 -0400 Received: from galois.linutronix.de (Galois.linutronix.de [IPv6:2a0a:51c0:0:12e:550::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7E3A849FF for ; Mon, 24 Apr 2023 11:13:25 -0700 (PDT) From: Thomas Gleixner DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1682360003; h=from:from: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; bh=K9W8NDx80UQqvWkG+atar6d9T4uMp1T5rDH7i9DlOhk=; b=qiVeV3A4OAwU/xPLlZC/66pUD4SIx2FYDMlcM0Wp3JwJNCAQwhyHXbTiGD99eO31OzYMGs It4ljeSJzIlLsI/kPeh/Nc92uGoOfNj2DT5eCRaKVUFVMljhsEftvikFFeSbkvLNKzx6C8 9NQoa6gsKqAkrCiZtMQ1sttNosj02nOtmHRVN2iTmiTvpCiHWwFJ5KYnyKk7jXaCm2A9fq 8Sb82q8xIiW+NEXFy0z80bMZ2p+cmkdGAwJA1mY7eb52gvQNjXmvIm+tGntv1sc7RuTyhg UpGZpCoVGaJS5r8cRAv/7MiprEsjn45hgdDJA4kL6oiFYWWJDF1Xy9Ciy2Pf1Q== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1682360003; h=from:from: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; bh=K9W8NDx80UQqvWkG+atar6d9T4uMp1T5rDH7i9DlOhk=; b=+HqOswjYocaiyCLvot1o/An5CP6/O+5EPKlFdWYR6FBzwYTda3FJ78S2aiqLGfarxguh3p mu5IuYbdZs8HzLAQ== To: Linus Torvalds Cc: linux-kernel@vger.kernel.org, x86@kernel.org Subject: [GIT pull] core/debugobjects for 6.4-rc1 Message-ID: <168235968801.840202.17752066425816055574.tglx@xen13> MIME-Version: 1.0 Date: Mon, 24 Apr 2023 20:13:23 +0200 (CEST) X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1764083288341790204?= X-GMAIL-MSGID: =?utf-8?q?1764083288341790204?= Linus, please pull the latest core/debugobjects branch from: git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git core-debugobjects-2023-04-24 up to: 63a759694eed: debugobject: Prevent init race with static objects A single update to debugobjects: Prevent a race vs. statically initialized objects. Such objects are usually not initialized via an init() function. They are special cased and detected on first use under the assumption that they are already correctly initialized via the static initializer. This works correctly unless there are two concurrent debug object operations on such an object. The first one detects that the object is not yet tracked and tries to establish a tracking object after dropping the debug objects hash bucket lock. The concurrent operation does the same. The one which wins the race ends up modifying the state of the object which makes the other one fail resulting in a bogus debug objects warning. Prevent this by making the detection of a static object and the allocation of a tracking object atomic under the hash bucket lock. So the first one to acquire the hash bucket lock will succeed and the second one will observe the correct tracking state. This race existed forever but was only exposed when the timer wheel code added a debug_object_assert_init() call outside of the timer base locked region. This replaced the previous warning about timer::function being NULL which had to be removed when the timer_shutdown() mechanics were added. Thanks, tglx ------------------> Thomas Gleixner (1): debugobject: Prevent init race with static objects lib/debugobjects.c | 125 ++++++++++++++++++++++++++++------------------------- 1 file changed, 66 insertions(+), 59 deletions(-) diff --git a/lib/debugobjects.c b/lib/debugobjects.c index df86e649d8be..b796799fadb2 100644 --- a/lib/debugobjects.c +++ b/lib/debugobjects.c @@ -216,10 +216,6 @@ static struct debug_obj *__alloc_object(struct hlist_head *list) return obj; } -/* - * Allocate a new object. If the pool is empty, switch off the debugger. - * Must be called with interrupts disabled. - */ static struct debug_obj * alloc_object(void *addr, struct debug_bucket *b, const struct debug_obj_descr *descr) { @@ -552,11 +548,49 @@ static void debug_object_is_on_stack(void *addr, int onstack) WARN_ON(1); } +static struct debug_obj *lookup_object_or_alloc(void *addr, struct debug_bucket *b, + const struct debug_obj_descr *descr, + bool onstack, bool alloc_ifstatic) +{ + struct debug_obj *obj = lookup_object(addr, b); + enum debug_obj_state state = ODEBUG_STATE_NONE; + + if (likely(obj)) + return obj; + + /* + * debug_object_init() unconditionally allocates untracked + * objects. It does not matter whether it is a static object or + * not. + * + * debug_object_assert_init() and debug_object_activate() allow + * allocation only if the descriptor callback confirms that the + * object is static and considered initialized. For non-static + * objects the allocation needs to be done from the fixup callback. + */ + if (unlikely(alloc_ifstatic)) { + if (!descr->is_static_object || !descr->is_static_object(addr)) + return ERR_PTR(-ENOENT); + /* Statically allocated objects are considered initialized */ + state = ODEBUG_STATE_INIT; + } + + obj = alloc_object(addr, b, descr); + if (likely(obj)) { + obj->state = state; + debug_object_is_on_stack(addr, onstack); + return obj; + } + + /* Out of memory. Do the cleanup outside of the locked region */ + debug_objects_enabled = 0; + return NULL; +} + static void __debug_object_init(void *addr, const struct debug_obj_descr *descr, int onstack) { enum debug_obj_state state; - bool check_stack = false; struct debug_bucket *db; struct debug_obj *obj; unsigned long flags; @@ -572,16 +606,11 @@ __debug_object_init(void *addr, const struct debug_obj_descr *descr, int onstack raw_spin_lock_irqsave(&db->lock, flags); - obj = lookup_object(addr, db); - if (!obj) { - obj = alloc_object(addr, db, descr); - if (!obj) { - debug_objects_enabled = 0; - raw_spin_unlock_irqrestore(&db->lock, flags); - debug_objects_oom(); - return; - } - check_stack = true; + obj = lookup_object_or_alloc(addr, db, descr, onstack, false); + if (unlikely(!obj)) { + raw_spin_unlock_irqrestore(&db->lock, flags); + debug_objects_oom(); + return; } switch (obj->state) { @@ -607,8 +636,6 @@ __debug_object_init(void *addr, const struct debug_obj_descr *descr, int onstack } raw_spin_unlock_irqrestore(&db->lock, flags); - if (check_stack) - debug_object_is_on_stack(addr, onstack); } /** @@ -648,14 +675,12 @@ EXPORT_SYMBOL_GPL(debug_object_init_on_stack); */ int debug_object_activate(void *addr, const struct debug_obj_descr *descr) { + struct debug_obj o = { .object = addr, .state = ODEBUG_STATE_NOTAVAILABLE, .descr = descr }; enum debug_obj_state state; struct debug_bucket *db; struct debug_obj *obj; unsigned long flags; int ret; - struct debug_obj o = { .object = addr, - .state = ODEBUG_STATE_NOTAVAILABLE, - .descr = descr }; if (!debug_objects_enabled) return 0; @@ -664,8 +689,8 @@ int debug_object_activate(void *addr, const struct debug_obj_descr *descr) raw_spin_lock_irqsave(&db->lock, flags); - obj = lookup_object(addr, db); - if (obj) { + obj = lookup_object_or_alloc(addr, db, descr, false, true); + if (likely(!IS_ERR_OR_NULL(obj))) { bool print_object = false; switch (obj->state) { @@ -698,24 +723,16 @@ int debug_object_activate(void *addr, const struct debug_obj_descr *descr) raw_spin_unlock_irqrestore(&db->lock, flags); - /* - * We are here when a static object is activated. We - * let the type specific code confirm whether this is - * true or not. if true, we just make sure that the - * static object is tracked in the object tracker. If - * not, this must be a bug, so we try to fix it up. - */ - if (descr->is_static_object && descr->is_static_object(addr)) { - /* track this static object */ - debug_object_init(addr, descr); - debug_object_activate(addr, descr); - } else { - debug_print_object(&o, "activate"); - ret = debug_object_fixup(descr->fixup_activate, addr, - ODEBUG_STATE_NOTAVAILABLE); - return ret ? 0 : -EINVAL; + /* If NULL the allocation has hit OOM */ + if (!obj) { + debug_objects_oom(); + return 0; } - return 0; + + /* Object is neither static nor tracked. It's not initialized */ + debug_print_object(&o, "activate"); + ret = debug_object_fixup(descr->fixup_activate, addr, ODEBUG_STATE_NOTAVAILABLE); + return ret ? 0 : -EINVAL; } EXPORT_SYMBOL_GPL(debug_object_activate); @@ -869,6 +886,7 @@ EXPORT_SYMBOL_GPL(debug_object_free); */ void debug_object_assert_init(void *addr, const struct debug_obj_descr *descr) { + struct debug_obj o = { .object = addr, .state = ODEBUG_STATE_NOTAVAILABLE, .descr = descr }; struct debug_bucket *db; struct debug_obj *obj; unsigned long flags; @@ -879,31 +897,20 @@ void debug_object_assert_init(void *addr, const struct debug_obj_descr *descr) db = get_bucket((unsigned long) addr); raw_spin_lock_irqsave(&db->lock, flags); + obj = lookup_object_or_alloc(addr, db, descr, false, true); + raw_spin_unlock_irqrestore(&db->lock, flags); + if (likely(!IS_ERR_OR_NULL(obj))) + return; - obj = lookup_object(addr, db); + /* If NULL the allocation has hit OOM */ if (!obj) { - struct debug_obj o = { .object = addr, - .state = ODEBUG_STATE_NOTAVAILABLE, - .descr = descr }; - - raw_spin_unlock_irqrestore(&db->lock, flags); - /* - * Maybe the object is static, and we let the type specific - * code confirm. Track this static object if true, else invoke - * fixup. - */ - if (descr->is_static_object && descr->is_static_object(addr)) { - /* Track this static object */ - debug_object_init(addr, descr); - } else { - debug_print_object(&o, "assert_init"); - debug_object_fixup(descr->fixup_assert_init, addr, - ODEBUG_STATE_NOTAVAILABLE); - } + debug_objects_oom(); return; } - raw_spin_unlock_irqrestore(&db->lock, flags); + /* Object is neither tracked nor static. It's not initialized. */ + debug_print_object(&o, "assert_init"); + debug_object_fixup(descr->fixup_assert_init, addr, ODEBUG_STATE_NOTAVAILABLE); } EXPORT_SYMBOL_GPL(debug_object_assert_init);