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 0C239C00140 for ; Wed, 24 Aug 2022 07:04:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8517F940008; Wed, 24 Aug 2022 03:04:19 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7D9F36B0074; Wed, 24 Aug 2022 03:04:19 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6A196940008; Wed, 24 Aug 2022 03:04:19 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 58EDC6B0073 for ; Wed, 24 Aug 2022 03:04:19 -0400 (EDT) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 66DD8140C46 for ; Wed, 24 Aug 2022 07:04:18 +0000 (UTC) X-FDA: 79833597396.25.1831555 Received: from out2.migadu.com (out2.migadu.com [188.165.223.204]) by imf11.hostedemail.com (Postfix) with ESMTP id E6C2740011 for ; Wed, 24 Aug 2022 07:04:17 +0000 (UTC) Content-Type: text/plain; charset=utf-8 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1661324655; 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: in-reply-to:in-reply-to:references:references; bh=6tvwkfCP3dZ+ZrJ0eNLvzLAz7UpWICuxq3NpSBeqntU=; b=JunrwnnLdI6piFW4nALnDaW6RyDeknYztEVEEq1kYXEcDlQDHTBFWTvP8zNva/nFBj7Jxq QepF3Va2IjydiNXNph8l3Is8C6a5p7We4mMSMhRFhSpyNugZV2Ruqu96o5gAM6ppHqVPEU o4s67SknytA+VlP/1MMoBlMJW2LD4U4= MIME-Version: 1.0 Subject: Re: [PATCH] mm: fix pgdat->kswap accessed concurrently X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Muchun Song In-Reply-To: <16cf184f-95ec-a763-0606-7423db8dcb0f@huawei.com> Date: Wed, 24 Aug 2022 15:03:50 +0800 Cc: Andrew Morton , Linux MM , linux-kernel@vger.kernel.org Content-Transfer-Encoding: quoted-printable Message-Id: <2B0D6AB9-736E-40EB-996F-C0B6200EE749@linux.dev> References: <20220820032506.126860-1-wangkefeng.wang@huawei.com> <1E87F09C-4904-49E2-B45C-C408DD5F6F62@linux.dev> <20220820135955.1520aa480fe04ab31d4fce1f@linux-foundation.org> <16cf184f-95ec-a763-0606-7423db8dcb0f@huawei.com> To: Kefeng Wang X-Migadu-Flow: FLOW_OUT X-Migadu-Auth-User: linux.dev ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=JunrwnnL; spf=pass (imf11.hostedemail.com: domain of muchun.song@linux.dev designates 188.165.223.204 as permitted sender) smtp.mailfrom=muchun.song@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1661324658; a=rsa-sha256; cv=none; b=b0Kjvy1ZCzlMjkBUtJr0SQTmJ02csXki8wXCelVUw72IY62ZfSPnWvkhcVMd04XnlL8qtA YaYGHaPzjzGPFcDY19nFf/USgHrjB/z3kaT+kQ1S+FPlYU5aMpXdWZRBhi1IYxpy3P7Kcf GATK5b1EnDlzJvKBfiQ9GqLL8ipI3Uw= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1661324658; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=6tvwkfCP3dZ+ZrJ0eNLvzLAz7UpWICuxq3NpSBeqntU=; b=oCQIUckeoa4HYYYRmyDfKn/OBn6BZBl1UAf6WTiCTgVxBATdP/GJjIpiPXuoi9jBhYaLr5 MtdeOmGGtqGXSxULMVQ6sZgSMHIIEjaUQWGm+rj9bbM3FLkoI5T4Vfm7dURmDcjNYkWcnv LzsyCGgSK018qYXnTOO2SpAO/AWDu3g= Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=JunrwnnL; spf=pass (imf11.hostedemail.com: domain of muchun.song@linux.dev designates 188.165.223.204 as permitted sender) smtp.mailfrom=muchun.song@linux.dev; dmarc=pass (policy=none) header.from=linux.dev X-Rspam-User: X-Rspamd-Queue-Id: E6C2740011 X-Rspamd-Server: rspam05 X-Stat-Signature: riy4xootzsejr3qjxwqqbqsxds7q73xx X-HE-Tag: 1661324657-668181 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: > On Aug 23, 2022, at 22:47, Kefeng Wang = wrote: >=20 >=20 > On 2022/8/23 9:07, Kefeng Wang wrote: >>=20 >> On 2022/8/21 4:59, Andrew Morton wrote: >>> On Sat, 20 Aug 2022 15:33:04 +0800 Muchun Song = wrote: >>>=20 >>>>=20 >>>>> + if (IS_ERR(t)) { >>>>> /* failure at boot is fatal */ >>>>> BUG_ON(system_state < SYSTEM_RUNNING); >>>>> pr_err("Failed to start kswapd on node %d\n", nid); >>>>> - pgdat->kswapd =3D NULL; >>>>> + WRITE_ONCE(pgdat->kswapd, NULL); >>>>> + } else { >>>>> + WRITE_ONCE(pgdat->kswapd, t); >>>>> } >>>>> } >>>> IIUC, the race is like the followings: >>>>=20 >>>> CPU 0: CPU 1: >>>>=20 >>>> kswapd_run() >>>> pgdat->kswapd =3D kthread_run() >>>> if (IS_ERR(pgdat->kswapd)) >>>> kswapd_is_running >>>> // load pgdat->kswapd and it is NOT NULL. >>>> pgdat->kswapd =3D NULL >>>> task_is_running(pgdat->kswapd); // NULL = pointer dereference >>>>=20 >>> But don't we still have a bug? Sure, kswapd_is_running() will no >>> longer deref a null pointer. But it now runs kswapd_is_running() >>> against a task which has exited - a use-after-free? >=20 > The UAF is caused by race between kswapd_stop() and kcompactd(), = right? >=20 > so kcompactd() should be stop before kswapd_stop() to avoid the above = UAF. >=20 > $ git diff > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c > index fad6d1f2262a..2fd45ccbce45 100644 > --- a/mm/memory_hotplug.c > +++ b/mm/memory_hotplug.c > @@ -1940,8 +1940,8 @@ int __ref offline_pages(unsigned long start_pfn, = unsigned long nr_pages, >=20 > node_states_clear_node(node, &arg); > if (arg.status_change_nid >=3D 0) { > - kswapd_stop(node); > kcompactd_stop(node); > + kswapd_stop(node); > } >=20 > writeback_set_ratelimit(); The changes make sense to me. Again: Reviewed-by: Muchun Song Thanks. >=20 >> we could add get/put_task_struct() to avoid the UAF=EF=BC=8C will = update=EF=BC=8C thanks. >=20 > sorry, the task refcount won't fix anything. >=20 >=20 >> .