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 629A6C05027 for ; Fri, 20 Jan 2023 19:18:11 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id AED6C6B0071; Fri, 20 Jan 2023 14:18:10 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id A9D456B0073; Fri, 20 Jan 2023 14:18:10 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 98C686B0074; Fri, 20 Jan 2023 14:18:10 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 89D906B0071 for ; Fri, 20 Jan 2023 14:18:10 -0500 (EST) Received: from smtpin01.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 568081C5BEE for ; Fri, 20 Jan 2023 19:18:10 +0000 (UTC) X-FDA: 80376137940.01.E355082 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by imf07.hostedemail.com (Postfix) with ESMTP id A1AE34001B for ; Fri, 20 Jan 2023 19:18:07 +0000 (UTC) Authentication-Results: imf07.hostedemail.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=arm.com (policy=none); spf=pass (imf07.hostedemail.com: domain of cmarinas@kernel.org designates 145.40.68.75 as permitted sender) smtp.mailfrom=cmarinas@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1674242287; 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-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Td0mi+U0BZCVxHQ/XBdhPODqu2Kj6BMWrM+UBczhFEs=; b=epDP8Sa62ER1H0CL7iLKLUx//6WFqlWn2QGQmDT+1C7TMZdC2Qzf3eMKcewcG23aj+TLJf WJ7bn1AIFw+iDNFuscknzodhjbOOY2/X2csQhWajX8PIl1aK/7igmcZjB4rDGIHr83/AsH unlz0vOgTrKaHcv5gXaCisrAgetDgI0= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=arm.com (policy=none); spf=pass (imf07.hostedemail.com: domain of cmarinas@kernel.org designates 145.40.68.75 as permitted sender) smtp.mailfrom=cmarinas@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1674242287; a=rsa-sha256; cv=none; b=FHGw9yKj1sEFgZ+NFOzVxkHm0Clw5Fp193UhTxU71Lw7bE+8jVEjmKB2UxvDWSPulO2be6 R6G1zRq2g21WyZqrnRbp+QCqQsTQ55ARbIgHt6CEF09xRp23HGnnIutPA01nbAHc+Msc9Y kkZ4+y2XTq1s+uHZXsglfoz70xemm9g= Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id D8EF8B82846; Fri, 20 Jan 2023 19:18:05 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 567A2C433EF; Fri, 20 Jan 2023 19:18:03 +0000 (UTC) Date: Fri, 20 Jan 2023 19:18:00 +0000 From: Catalin Marinas To: Waiman Long Cc: Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Muchun Song Subject: Re: [RESEND PATCH v2 2/2] mm/kmemleak: Fix UAF bug in kmemleak_scan() Message-ID: References: <20230119040111.350923-1-longman@redhat.com> <20230119040111.350923-3-longman@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230119040111.350923-3-longman@redhat.com> X-Rspamd-Queue-Id: A1AE34001B X-Stat-Signature: nzws1hxua7zw9rcox9gzcgr11h388et8 X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1674242287-745145 X-HE-Meta: U2FsdGVkX1/M7ZwCQDBn0StLZDP3dXV1KcGbey6OlVi03YmFEAdrw1yQRJGLeNUSwfbCuLLGnFKImY61WL4YRd0dfBlm8JQ9XHZ025cL3kBq7NLKeyAy12ryOnOkfHE0bvAszfmWcCHaCSg/MbuphuX+fN/fCRDI2+Gm3OUIFIZgffexFTBNnXUmfmT4DWZ2oN3Fxkzzw4m5E5apt3aur4pbZUXQ16g89LZiBMYZlTAwGzWzQEP+MlEL9STNInM0sMvHem7yKBrhGVS8ERSkgAzq/hEIYVrPkTyHCXVjHEU4UDbBlfPcNey8A+Qe7fHzKD7J5rKqzfWF+Y0gzb3iDqSt2EmWRL6UjGLiI05YbYwhmEzfRNYIh74n5MV5aA5o01F4bTOQlC0IPVdmZYi4pJdA+dVBjWr2KheXfRlHjVlFcUbtcKHCtzeVFJIgD9jvOmviLOeN0A8bxEE+CMmUELQq44hh6Ye+H5BoLikdO/sTpoCbJtXUfdg8GEZGSW+K7DxEiM7cqIGjxGs1kCkubyi15VcUqeKOFeMWddkUypomXMgh52ROsbhPDiX85N3C7VRcUMepk0LrWWON2xGMvKnG9OZ4bOWGgWPG/EYUDb1NsvkYO9uYLsXVfVv+8Oby5iN/zm93sVQdHdIBDYrqL6ETtOC6Byf5H5TsDocaFYyGrelPYWemumFJ9am8Br/IrMlw8znImvDHVmU6XPu3/fEvyoTJslkk7DW74UAzT+WrFab8KYOiMDypWtfTkDIJGmWy92shr8z8hrLD/AMX9z4FXZ0hqtlaOTuFV1WCOX1vX1jqfI6irVjag2i6cOkPoOBxUHLb8pei5N7+JQUMY2JZN0ZwPd39KTzNiZ8ct2TmtjaqxdhNeFet29im41vNSbtcT6rSH+bUUpeW55/9BmU8ueQD/pj87I0SGNgn3kiXDlrW+dXcXVSOBE3e/5nmaHq4PADViGQetjefg6g QRD5Ntqm HK4c6Lo0b8JgseyxE21aT6hhXfnfZae8GJ3ehpsLdY+dtBTQBJeJWYw5sYX1f9IrYZBuZEfTGLLOFm2AqfelZelxMHJ+/GJHMr+6gdugy5DqvbhMyVgI4nIDrEqyJyuURFZR3O/buA/b3gQU6DJmRkh4xGIHhrpHq4NBLkhTihTH8FQYuNEpLbUIhjSxBDg7nM7QLkfXW5NwaraBMGXYvW3a79xOCOSWalD1HcytkViebtdYU++wmA34FZCx+D8wSoxgUUSmalzWZ07Bd1fuJTyCXJb3XpcqKUrUQq1Yb9xl7URbVk9+Imax5tXFixRzooRDn4ct5uhVkqbY4dnkiOM3MTnwf0MzFRfJHV5dO/a7/aHLOpjRfSl+mZVb55tgibkUPQie+0LOfUkp79D75tRhVdmZhp07RwpCwnzahpoUsHoYp0T4shEpefo/h+VtjQ8aTZqTAASe9WwNBmVbK4TjCHTTIDN6owqgJ 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: Hi Waiman, Thanks for your effort on trying to fix this. On Wed, Jan 18, 2023 at 11:01:11PM -0500, Waiman Long wrote: > @@ -567,7 +574,9 @@ static void __remove_object(struct kmemleak_object *object) > rb_erase(&object->rb_node, object->flags & OBJECT_PHYS ? > &object_phys_tree_root : > &object_tree_root); > - list_del_rcu(&object->object_list); > + if (!(object->del_state & DELSTATE_NO_DELETE)) > + list_del_rcu(&object->object_list); > + object->del_state |= DELSTATE_REMOVED; > } So IIUC, this prevents the current object being scanned from being removed from the list during the kmemleak_cond_resched() call. > /* > @@ -633,6 +642,7 @@ static void __create_object(unsigned long ptr, size_t size, > object->count = 0; /* white color initially */ > object->jiffies = jiffies; > object->checksum = 0; > + object->del_state = 0; > > /* task information */ > if (in_hardirq()) { > @@ -1470,9 +1480,22 @@ static void kmemleak_cond_resched(struct kmemleak_object *object) > if (!get_object(object)) > return; /* Try next object */ > > + raw_spin_lock_irq(&kmemleak_lock); > + if (object->del_state & DELSTATE_REMOVED) > + goto unlock_put; /* Object removed */ > + object->del_state |= DELSTATE_NO_DELETE; > + raw_spin_unlock_irq(&kmemleak_lock); > + > rcu_read_unlock(); > cond_resched(); > rcu_read_lock(); > + > + raw_spin_lock_irq(&kmemleak_lock); > + if (object->del_state & DELSTATE_REMOVED) > + list_del_rcu(&object->object_list); > + object->del_state &= ~DELSTATE_NO_DELETE; > +unlock_put: > + raw_spin_unlock_irq(&kmemleak_lock); > put_object(object); > } I'm not sure this was the only problem. We do have the problem that the current object may be removed from the list, solved above, but another scenario I had in mind is the next object being released during this brief resched period. The RCU relies on object->next->next being valid but, with a brief rcu_read_unlock(), the object->next could be freed, reallocated, so object->next->next invalid. -- Catalin