From patchwork Mon Sep 25 09:16:00 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrzej Hajda X-Patchwork-Id: 13397558 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 6886FCE7A8C for ; Mon, 25 Sep 2023 09:16:37 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D42368D001E; Mon, 25 Sep 2023 05:16:36 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id CF1518D0001; Mon, 25 Sep 2023 05:16:36 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BB8D18D001E; Mon, 25 Sep 2023 05:16:36 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id A82258D0001 for ; Mon, 25 Sep 2023 05:16:36 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 7BF83C0B4C for ; Mon, 25 Sep 2023 09:16:36 +0000 (UTC) X-FDA: 81274564392.07.37F85FE Received: from mgamail.intel.com (mgamail.intel.com [192.55.52.115]) by imf16.hostedemail.com (Postfix) with ESMTP id 43D20180003 for ; Mon, 25 Sep 2023 09:16:33 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=GkOkjtuf; spf=pass (imf16.hostedemail.com: domain of andrzej.hajda@intel.com designates 192.55.52.115 as permitted sender) smtp.mailfrom=andrzej.hajda@intel.com; dmarc=pass (policy=none) header.from=intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1695633394; 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=/usun3KcI2m+RCMjSgo+RTjqedkzfPBnju74GP4bwA4=; b=0N4AdSdp/itLMQ/W8AmDNDO9cKpw/VtxCN2Kppb+OZTfAbn6qFXsPXekpmjZbVI8zNJXji Qytt/tWy6rxpLSuekifdsb7vHuCkRxugOks0orJQ+XY9EnF+OrAJ3xr3L+IeHqdWxvc9uz T41nOO0HuLIQ7ynTxoowK1ULLwAvD0M= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=GkOkjtuf; spf=pass (imf16.hostedemail.com: domain of andrzej.hajda@intel.com designates 192.55.52.115 as permitted sender) smtp.mailfrom=andrzej.hajda@intel.com; dmarc=pass (policy=none) header.from=intel.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1695633394; a=rsa-sha256; cv=none; b=CHPcBXbi3IcNQm5B9gb9XqgN5rjXGL1EKhBPntEitgq9E+HEzGEg4Oi9bqZIlLyByraASy zcUtvcroXFSs4Iiz8uuKqF5gXxeWMixeFddsh+58e2/REiseqOE9rz0ocHOg3RYMsGN152 ZMTo7eeaQs9enGc8Oc5JYGDE5b/DhdE= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1695633393; x=1727169393; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=4mYYGMtvhrZ5Go/HEV8QLtGBFT/aHlbOysA0rNPlmFE=; b=GkOkjtufIH57wd7BulKimsWbi1iafHEG5idCrsl4RbpyfAjFRJEeYDza 3jc5usqelbOojUczK1sulCy4PrPte5LBeMhUY7+R2X7G9CbEs7GQZGnJL gf3dMVCqy2Nab0iVsJf028+JOzZIda2zvSmBchKS1QhSqqznjnVFzadgg JimXfVOWrrWM22Zwiu9+MXYi82h8XGk5ae4qlwLotPYDBizUjIF1eeSEx sws3jLbLUz1NZ89HGjjcGPCCZL9KW0i9hAeUruEAYyjzpMo6SBWeI8kPI 3p145Lf88SUzGnV1XeUObSyxiszFTUWoNF0PzVOJ3vNIcyO7k4cyFl6BL A==; X-IronPort-AV: E=McAfee;i="6600,9927,10843"; a="381115721" X-IronPort-AV: E=Sophos;i="6.03,174,1694761200"; d="scan'208";a="381115721" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Sep 2023 02:16:29 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10843"; a="838515025" X-IronPort-AV: E=Sophos;i="6.03,174,1694761200"; d="scan'208";a="838515025" Received: from lab-ah.igk.intel.com (HELO lab-ah.corp.intel.com) ([10.102.138.202]) by fmsmga003-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Sep 2023 02:16:27 -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] debugobjects: stop accessing objects after releasing spinlock Date: Mon, 25 Sep 2023 11:16:00 +0200 Message-Id: <20230925091600.2941364-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 X-Rspamd-Queue-Id: 43D20180003 X-Rspam-User: X-Stat-Signature: z8rsws4nwe88c7rpgr44n7h8ogrmn5jz X-Rspamd-Server: rspam01 X-HE-Tag: 1695633393-971799 X-HE-Meta: U2FsdGVkX19tlrXRVa4Ti88zuU/ERBg8NWX6Zc2IoIgVrTHrnYaub+4t1U+/3kXtbZo9sm6ABxmdyA8VnwyUbo7smqjAVr5rDKPor7iKbtFSG+VKHLycXW6+9pKdzRN59G39KWMDZoQ/Mtuo6dzqJ3z+LuCbQbqKCZXZ59zubT03zhE9d10kTshLjxhlqZDfGwlgmJjjEuZVZYNYdYKDhJvWStMcCmq4OIt3NVzC0vBRQ3qyEPm+eh9Zza97aognfj/rBtbv9EicMy1Y1tpmXko4ffMkLTSxVufWtjdk77UAf9QkSNYtgEtPRa2TUMws13Sguq8tyPuvA0Om3APsT7Yjz/RLxjPRe8TvQSj9Q2PN+R3Ih+in8uG0gGCay8KhKUH86B97bZpuLqfjCBn4LhaBe8XhVlHNZldthjOgu0aHcWzUk4hNTSKDRyEOUjOK3STR8/JxiRvx/pqu/treczd7ZB/vW7rsgfsdk1M+3ZCEWvD0GL7oKG90buHXfxf6BNCq/SzHMY+NnJ1Tc4GGLUlEyADtmh+LQ33WRmAGHifXuRjt3A8qQzuHBw4DXkmF0Y+TziUd/K/Q4slbe9auhvAp7j13yHdj1u6pdynaq6haoXryvReRm4bOeSWBfW1VJIUTBqVv5jLRvDdNN9P6UIgiQqo6qOzXNDo7n/fpuBXGMySmidTidcTQplIZKsU/c9j43uCmvHvKKKiSPlmei5chhF7nzs9oxWfKkxzAmVSLZUcvuidt9Kc9tztNDaAeybPIL3dKFriq7MMgEGxe15mIJuehiwXTvyzNOSy93BxsZwy75jozVh3FnjT3SsB8LCoBKwvvmjubyj5NjmYrCslQWsbV9R9ARBRdLkvt51EATgx/PPCyOl6heVtXV+Aeuv2Ct+8QUin+S8FEicPi2dAjmwdJ3qBeHpOvRNARmnYML2RqJ7f2GNkUlSFHZqtwxCB4yLtYcUm5FB7S4ZE ef/UGd9e O7SlPCrtk3oCQEx1cKmSO4E3jTyNHWAL1666EfCeHnJq52xn1iad/VmCOjE5WBwXtokuYOCqLfZrSvFLCjjJM7xrshE0Iuh3ZVaLJK89FyD/+d//U8ymxiOSLeZr+15KOR25GWQZT9rMQ+d1XrYHJd4dbt+sD7uMRfR0UN2IN0obFLd7JNs4x4sQcgGXo6sp761No 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 --- lib/debugobjects.c | 208 +++++++++++++++++++++------------------------ 1 file changed, 97 insertions(+), 111 deletions(-) diff --git a/lib/debugobjects.c b/lib/debugobjects.c index a517256a270b71..d6f9af11bff0d9 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,46 @@ 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; + default: + } + + return -EINVAL; } EXPORT_SYMBOL_GPL(debug_object_activate); @@ -771,9 +760,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 +776,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 +809,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 +821,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 +833,22 @@ 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 +859,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 +871,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 +946,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 +960,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 +988,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 +1013,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);