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 5C2B8CE7A81 for ; Mon, 25 Sep 2023 13:26:22 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CE5A18D0028; Mon, 25 Sep 2023 09:26:21 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C93BA8D0001; Mon, 25 Sep 2023 09:26:21 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B34AE8D0028; Mon, 25 Sep 2023 09:26:21 -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 A1CA08D0001 for ; Mon, 25 Sep 2023 09:26:21 -0400 (EDT) Received: from smtpin30.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 725BDB36A0 for ; Mon, 25 Sep 2023 13:26:21 +0000 (UTC) X-FDA: 81275193762.30.A636CCF Received: from mgamail.intel.com (mgamail.intel.com [192.55.52.43]) by imf02.hostedemail.com (Postfix) with ESMTP id B576D8001B for ; Mon, 25 Sep 2023 13:26:18 +0000 (UTC) Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=U40heKOD; dmarc=pass (policy=none) header.from=intel.com; spf=pass (imf02.hostedemail.com: domain of andrzej.hajda@intel.com designates 192.55.52.43 as permitted sender) smtp.mailfrom=andrzej.hajda@intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1695648379; 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=zHAIz7Q+I+ZNd6T6yn13O/cxMeBGEQSCsMxZfBsxITA=; b=lIV+sMWtYlkEJo/Roi0QVT9R0DdIg9iYf6WzUGkYRLcKxHC8ugR2lhLtLgtH020dWKEV7B AjTga180Opu1t6wsFSEHot+hdbRxazxyGlaBxfA8lm0eAbDVR6ei1SDpoal4qNeF/LXoqm RaQ0/xXwJdwv2PppX6PK52tS4SQM6Y4= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=U40heKOD; dmarc=pass (policy=none) header.from=intel.com; spf=pass (imf02.hostedemail.com: domain of andrzej.hajda@intel.com designates 192.55.52.43 as permitted sender) smtp.mailfrom=andrzej.hajda@intel.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1695648379; a=rsa-sha256; cv=none; b=5lQBKGFowK1GbHdG8QuEAvd9NnTRwalzR/gGR6gcFKsyV6jOtsvGm0oF7iXSnsq0t155mf TmZsBXMpGInZ5hctV3GvUa/iA3oHSZY3iUgA5Zr+hq/LeCBJJWSlKxNlWobhId4JuQBIiw mUr6iTX+BeeBCTOJzCSQ7yytH+qCxrA= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1695648378; x=1727184378; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=Y3phxFH2l+gWamkxBycz06x54qFJ+isInMU3YYySyng=; b=U40heKODHVDShPy9cARmYXUCTFfa0lt0ZpamKhvWJAvsj7QXFDoEehKF dNKILaQnkOEBpqiFqp+PnGKizYDoC8lreCVnrI0RfVIuJGjVzogOE/tre ypaeYgjnAi0HffWZ/ps7aGMioL+n5oi3efvTJX2NCedi2OA6CntflXIwC QYfKC43tjiqsbbDkY8+ZV+VxjP8iw/lBKLR7F8T4QX+OPxULrx9aoyswO 0zDVOCSEx4bhroIwn2M1CCHqfcuHvwIaGYiVftHfmOVNNfvbmNzG+TgXU P5IF8FT8r9IzPCJXuH8WevnEheAAHPSJFcZy0cd4M3PPuelzx1YosEG9Q g==; X-IronPort-AV: E=McAfee;i="6600,9927,10843"; a="467537230" X-IronPort-AV: E=Sophos;i="6.03,175,1694761200"; d="scan'208";a="467537230" Received: from orsmga007.jf.intel.com ([10.7.209.58]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Sep 2023 06:14:13 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10843"; a="741918877" X-IronPort-AV: E=Sophos;i="6.03,175,1694761200"; d="scan'208";a="741918877" Received: from lab-ah.igk.intel.com (HELO lab-ah.corp.intel.com) ([10.102.138.202]) by orsmga007-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Sep 2023 06:14:10 -0700 From: Andrzej Hajda To: linux-kernel@vger.kernel.org, intel-gfx@lists.freedesktop.org, linux-mm@kvack.org, Thomas Gleixner Cc: Andrzej Hajda , Andi Shyti , Nirmoy Das , Janusz Krzysztofik Subject: [PATCH v2] debugobjects: stop accessing objects after releasing spinlock Date: Mon, 25 Sep 2023 15:13:59 +0200 Message-Id: <20230925131359.2948827-1-andrzej.hajda@intel.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 Organization: Intel Technology Poland sp. z o.o. - ul. Slowackiego 173, 80-298 Gdansk - KRS 101882 - NIP 957-07-52-316 Content-Transfer-Encoding: 8bit X-Rspam-User: X-Stat-Signature: cdptjwaeg6f3ecw6bticxzs5ziufwp44 X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: B576D8001B X-HE-Tag: 1695648378-723437 X-HE-Meta: U2FsdGVkX1+VwnQVl1PHBr7sKPf3HL/axkhpyGxinOv736COFJSKJTV4rvdXKtNH1ZqToEKi1xcPbJaWJZaKaAKrI8Z6XEIhnxspi2DFNd7j0m7xgkppaudSjP8Sjj9YjlULVIskVrKCql1mpbZz/ujLhU+NN2bCaY3mdtqhS0Zg46l1VkV7kpKeoeSf5Sle/4ypkkZMYyUQ0yZ2xiiKqu0fK+v6D2PgcyOPO7ZuRsBM/gwwb3X7hjx7psxFY/r2moz2oGawbgseIt8oiP05TgdaY/8wAdLsyoWWNTTGScldShY9t+aiY2eETsgLhrOhOW8RWe7LmIyfjvhEwVk4RCx0uO8ValIcdSbIyD+6AtwyRpnYFqs80CkyC+ObUHRC1dHyb716MKSDXVdo0dS4BrDyGLSPqpqREf2RIF/6eW0V4yWV6zMQrmc8skcTH0bWh9g14mNXqAp244bgBV+PZ0uV5Qw0jWpeST6f6DqmmIDbPvufNRAwX3RTu7TMzZjPw7JeldKO1VTubTIdTyQvsjy9OgRdRRDruiN/WvsLN8ROImna2jQSHKDyf6hY2AS0y2fQQhZMS357NhwNK0LqMTlWYk+9bG1psg/svtKSughnSW3+3FphJdVsxULxSnGMxVRxuoU7J1bALl73jCubNcAp77Us6bIBgRJawmxt/tbcrFzAk7LU3bbi4nKk2lVozKDEzXeLXeqQitBcDLqSC5EkTapgHUGEo1veWr1Tw0BvsiMmsXF9yv+ATISslh1e1bzJjW6f+OwB/kZsT3MwkwXzx1XZC3tJ7wL/46V0ICvFrlTv39drmh8pUsw457t0bibceGggJUYyuJ0LGe0ksi1q0a3iGKA1kCbp2+4eaol7RR8rhrKF5O8mOluTABGAbjz+t6P9CG1dHED+XfFTZEN0ZSzYUOok/LfJHWRsaj3QfQURkAWTldq0qRbJ74arbAS9AGH9VqgJEBFyT9f 6K9u5Np5 aWtSU7ZWKdPgf6TUMcAon8NzpVqRRoHXHu1wDRYwmhCjdZaOn5CUxmex3gXZ+DGjIgmVbPyOoXBGq1SigVg5OWj6tFmiXg1WhXOKzfpfhlZixuUMK0re/8Ga7Z+0XUY1DOt4QpZWecozJ0AEvw2OvdIsBjXLXZsifh6gtikfv27OgRIVCTchzctbcSw== 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: After spinlock release object can be modified/freed by concurrent thread. Using it in such case is error prone, even for printing object state. To avoid such situation local copy of the object is created if necessary. Signed-off-by: Andrzej Hajda --- v2: add missing switch breaks --- lib/debugobjects.c | 206 +++++++++++++++++++++------------------------ 1 file changed, 97 insertions(+), 109 deletions(-) diff --git a/lib/debugobjects.c b/lib/debugobjects.c index a517256a270b71..3afff2f668fc1e 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 onstack) { - enum debug_obj_state state; struct debug_bucket *db; - struct debug_obj *obj; + struct debug_obj *obj, o; unsigned long flags; debug_objects_fill_pool(); @@ -644,23 +643,19 @@ __debug_object_init(void *addr, const struct debug_obj_descr *descr, int onstack case ODEBUG_STATE_INACTIVE: obj->state = ODEBUG_STATE_INIT; break; - - case ODEBUG_STATE_ACTIVE: - state = 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; + o = *obj; + obj = NULL; } raw_spin_unlock_irqrestore(&db->lock, flags); + + if (obj) + return; + + debug_print_object(&o, "init"); + if (o.state == ODEBUG_STATE_ACTIVE) + debug_object_fixup(descr->fixup_init, addr, o.state); } /** @@ -700,12 +695,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 = { .object = addr, .state = ODEBUG_STATE_NOTAVAILABLE, .descr = descr }; - enum debug_obj_state state; struct debug_bucket *db; - struct debug_obj *obj; + struct debug_obj *obj, o; unsigned long flags; - int ret; if (!debug_objects_enabled) return 0; @@ -717,49 +709,47 @@ int debug_object_activate(void *addr, const struct debug_obj_descr *descr) raw_spin_lock_irqsave(&db->lock, flags); obj = lookup_object_or_alloc(addr, db, descr, false, true); - if (likely(!IS_ERR_OR_NULL(obj))) { - bool print_object = 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 = ODEBUG_STATE_ACTIVE; - ret = 0; break; - case ODEBUG_STATE_ACTIVE: - state = obj->state; - raw_spin_unlock_irqrestore(&db->lock, flags); - debug_print_object(obj, "activate"); - ret = debug_object_fixup(descr->fixup_activate, addr, state); - return ret ? 0 : -EINVAL; - case ODEBUG_STATE_DESTROYED: - print_object = true; - ret = -EINVAL; + o = *obj; + obj = NULL; break; default: - ret = 0; break; } - raw_spin_unlock_irqrestore(&db->lock, flags); - if (print_object) - debug_print_object(obj, "activate"); - return ret; + } else { + o.object = addr; + o.state = ODEBUG_STATE_NOTAVAILABLE; + o.descr = descr; + obj = NULL; } raw_spin_unlock_irqrestore(&db->lock, flags); - /* If NULL the allocation has hit OOM */ - if (!obj) { - debug_objects_oom(); + if (obj) 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; + + 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; + } } EXPORT_SYMBOL_GPL(debug_object_activate); @@ -771,9 +761,8 @@ EXPORT_SYMBOL_GPL(debug_object_activate); void debug_object_deactivate(void *addr, const struct debug_obj_descr *descr) { struct debug_bucket *db; - struct debug_obj *obj; + struct debug_obj *obj, o; unsigned long flags; - bool print_object = false; if (!debug_objects_enabled) return; @@ -788,30 +777,29 @@ void debug_object_deactivate(void *addr, const struct debug_obj_descr *descr) case ODEBUG_STATE_INIT: case ODEBUG_STATE_INACTIVE: case ODEBUG_STATE_ACTIVE: - if (!obj->astate) + if (!obj->astate) { obj->state = ODEBUG_STATE_INACTIVE; - else - print_object = true; - break; - + break; + } + fallthrough; case ODEBUG_STATE_DESTROYED: - print_object = true; + o = *obj; + obj = NULL; break; default: break; } + } else { + o.object = addr; + o.state = ODEBUG_STATE_NOTAVAILABLE; + o.descr = descr; + obj = NULL; } raw_spin_unlock_irqrestore(&db->lock, flags); - if (!obj) { - struct debug_obj o = { .object = addr, - .state = ODEBUG_STATE_NOTAVAILABLE, - .descr = descr }; + if (!obj) debug_print_object(&o, "deactivate"); - } else if (print_object) { - debug_print_object(obj, "deactivate"); - } } EXPORT_SYMBOL_GPL(debug_object_deactivate); @@ -822,11 +810,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_bucket *db; - struct debug_obj *obj; + struct debug_obj *obj, o; unsigned long flags; - bool print_object = false; if (!debug_objects_enabled) return; @@ -836,8 +822,10 @@ void debug_object_destroy(void *addr, const struct debug_obj_descr *descr) raw_spin_lock_irqsave(&db->lock, flags); obj = lookup_object(addr, db); - if (!obj) - goto out_unlock; + if (!obj) { + raw_spin_unlock_irqrestore(&db->lock, flags); + return; + } switch (obj->state) { case ODEBUG_STATE_NONE: @@ -846,22 +834,23 @@ void debug_object_destroy(void *addr, const struct debug_obj_descr *descr) obj->state = ODEBUG_STATE_DESTROYED; break; case ODEBUG_STATE_ACTIVE: - state = obj->state; - 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 = true; + o = *obj; + obj = NULL; break; default: break; } -out_unlock: + raw_spin_unlock_irqrestore(&db->lock, flags); - if (print_object) - debug_print_object(obj, "destroy"); + + if (obj) + return; + + debug_print_object(&o, "destroy"); + + if (o.state == ODEBUG_STATE_ACTIVE) + debug_object_fixup(descr->fixup_destroy, addr, o.state); } EXPORT_SYMBOL_GPL(debug_object_destroy); @@ -872,9 +861,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_bucket *db; - struct debug_obj *obj; + struct debug_obj *obj, o; unsigned long flags; if (!debug_objects_enabled) @@ -885,24 +873,29 @@ void debug_object_free(void *addr, const struct debug_obj_descr *descr) raw_spin_lock_irqsave(&db->lock, flags); obj = lookup_object(addr, db); - if (!obj) - goto out_unlock; + if (!obj) { + raw_spin_unlock_irqrestore(&db->lock, flags); + return; + } switch (obj->state) { case ODEBUG_STATE_ACTIVE: - state = obj->state; - raw_spin_unlock_irqrestore(&db->lock, flags); - debug_print_object(obj, "free"); - debug_object_fixup(descr->fixup_free, addr, state); - return; + o = *obj; + obj = NULL; + break; default: hlist_del(&obj->node); - raw_spin_unlock_irqrestore(&db->lock, flags); + } + + raw_spin_unlock_irqrestore(&db->lock, flags); + + if (obj) { free_object(obj); return; } -out_unlock: - 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); @@ -955,9 +948,8 @@ debug_object_active_state(void *addr, const struct debug_obj_descr *descr, unsigned int expect, unsigned int next) { struct debug_bucket *db; - struct debug_obj *obj; + struct debug_obj *obj, o; unsigned long flags; - bool print_object = false; if (!debug_objects_enabled) return; @@ -970,28 +962,27 @@ debug_object_active_state(void *addr, const struct debug_obj_descr *descr, if (obj) { switch (obj->state) { case ODEBUG_STATE_ACTIVE: - if (obj->astate == expect) + if (obj->astate == expect) { obj->astate = next; - else - print_object = true; - break; - + break; + } + fallthrough; default: - print_object = true; + o = *obj; + obj = NULL; break; } + } else { + o.object = addr; + o.state = ODEBUG_STATE_NOTAVAILABLE; + o.descr = descr; + obj = NULL; } raw_spin_unlock_irqrestore(&db->lock, flags); - if (!obj) { - struct debug_obj o = { .object = addr, - .state = ODEBUG_STATE_NOTAVAILABLE, - .descr = descr }; + if (!obj) debug_print_object(&o, "active_state"); - } else if (print_object) { - debug_print_object(obj, "active_state"); - } } EXPORT_SYMBOL_GPL(debug_object_active_state); @@ -999,11 +990,9 @@ 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; struct debug_bucket *db; struct hlist_node *tmp; - struct debug_obj *obj; + struct debug_obj *obj, o; int cnt, objs_checked = 0; saddr = (unsigned long) address; @@ -1026,12 +1015,11 @@ static void __debug_check_no_obj_freed(const void *address, unsigned long size) switch (obj->state) { case ODEBUG_STATE_ACTIVE: - descr = obj->descr; - state = obj->state; + o = *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); -- 2.34.1