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 9A623ECAAD1 for ; Sat, 27 Aug 2022 22:08:59 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DFC45940007; Sat, 27 Aug 2022 18:08:58 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id DAB486B0074; Sat, 27 Aug 2022 18:08:58 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C7352940007; Sat, 27 Aug 2022 18:08:58 -0400 (EDT) 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 B8C0F6B0073 for ; Sat, 27 Aug 2022 18:08:58 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 7750AA08B3 for ; Sat, 27 Aug 2022 22:08:58 +0000 (UTC) X-FDA: 79846763556.28.FCA7476 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf28.hostedemail.com (Postfix) with ESMTP id 13A2DC0023 for ; Sat, 27 Aug 2022 22:08:57 +0000 (UTC) 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 dfw.source.kernel.org (Postfix) with ESMTPS id 33F966010E; Sat, 27 Aug 2022 22:08:57 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 5C1AAC433D6; Sat, 27 Aug 2022 22:08:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linux-foundation.org; s=korg; t=1661638136; bh=EetUzY2Orm6TubMy5lXXo7jyaxG/w2IE8VsBq71JB/I=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=TH/oR8PL1TUQCxixiMDVlk+JuwZxSGKlID6RD90O92rMgxwUoKhLctUJiaaCQsPl2 86mIkQgzxCLOAB8OV+t6Z3CksQ/cU3jMcgv/BDgoSLA/Y3EsZyVYYGdwHhpIKGmY+t hwrbCEgeZ+CXVjwo2+PKT+6YwRptid7A0AT5ZtEw= Date: Sat, 27 Aug 2022 15:08:55 -0700 From: Andrew Morton To: Kefeng Wang Cc: , , David Hildenbrand , Muchun Song Subject: Re: [PATCH v2] mm: fix null-ptr-deref in kswapd_is_running() Message-Id: <20220827150855.b8885a5c4f09df712d619760@linux-foundation.org> In-Reply-To: <20220827111959.186838-1-wangkefeng.wang@huawei.com> References: <20220827111959.186838-1-wangkefeng.wang@huawei.com> X-Mailer: Sylpheed 3.7.0 (GTK+ 2.24.33; x86_64-redhat-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=korg header.b="TH/oR8PL"; dmarc=none; spf=pass (imf28.hostedemail.com: domain of akpm@linux-foundation.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=akpm@linux-foundation.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1661638138; a=rsa-sha256; cv=none; b=vQ28FLYFCPTZH1yCpFpxg+zKx4Qih/VsGf81m7rAUE067WWgHoK2FH31pVHes48vxLJsH6 qDlSHL4qBWHPY4oeis8CdaCMYjA/s77j97GLxfXGQF++93dL83xDXLtryaCiFYujf7EmqB V6EHDGbX8+Fc5gcdj9BoQURAKh7ctKU= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1661638138; 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=WlojmorT6wcd3oYX33Rsw363Oplp2wCqPrA+ZkNvZoA=; b=X22edr0uli6zdo3GaACvWmJLrCKK3a1MWQW1+P6/hu6J9wZrcILGtHTfZ1v79redoeGeyf HiY2+5IOnHr5k6K98pgoUC3OUzHuTuaehD4eX3f0oKWqeBeyIw40ATJqGIP75VcnI+vC4A 92msv654Qr+VCt3rLEvkfyUTJdHJgSg= X-Rspam-User: X-Rspamd-Queue-Id: 13A2DC0023 Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=korg header.b="TH/oR8PL"; dmarc=none; spf=pass (imf28.hostedemail.com: domain of akpm@linux-foundation.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=akpm@linux-foundation.org X-Stat-Signature: xodjcrbbf5hoy3fsor7zqrxknk1j1bxc X-Rspamd-Server: rspam03 X-HE-Tag: 1661638137-166030 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 Sat, 27 Aug 2022 19:19:59 +0800 Kefeng Wang wrote: > The kswapd_run/stop() will set pgdat->kswapd to NULL, which > could race with kswapd_is_running() in kcompactd(), > > kswapd_run/stop() kcompactd() > kswapd_is_running() > pgdat->kswapd // error or nomal ptr > verify pgdat->kswapd > // load non-NULL > pgdat->kswapd > pgdat->kswapd = NULL > task_is_running(pgdat->kswapd) > // Null pointer derefence > > The KASAN report the null-ptr-deref shown below, > > vmscan: Failed to start kswapd on node 0 > ... > BUG: KASAN: null-ptr-deref in kcompactd+0x440/0x504 > Read of size 8 at addr 0000000000000024 by task kcompactd0/37 > > CPU: 0 PID: 37 Comm: kcompactd0 Kdump: loaded Tainted: G OE 5.10.60 #1 > Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015 > Call trace: > dump_backtrace+0x0/0x394 > show_stack+0x34/0x4c > dump_stack+0x158/0x1e4 > __kasan_report+0x138/0x140 > kasan_report+0x44/0xdc > __asan_load8+0x94/0xd0 > kcompactd+0x440/0x504 > kthread+0x1a4/0x1f0 > ret_from_fork+0x10/0x18 > > For now, kswapd/kcompactd_run() and kswapd/kcompactd_stop() protected > by mem_hotplug_begin/done(), but without kcompactd(). It is no need to > involve memory hotplug lock in kcompactd(), so let's add new mutex to > protect pgdat->kswapd accessed concurrently, also because kcompactd task > will check the state of kswapd task, it's better to call kcompactd_stop() > before kswapd_stop() to reduce lock conflicts. > Looks right to me. I think the below will make the code a little more maintainable? --- a/include/linux/memory_hotplug.h~mm-fix-null-ptr-deref-in-kswapd_is_running-fix +++ a/include/linux/memory_hotplug.h @@ -215,6 +215,7 @@ void put_online_mems(void); void mem_hotplug_begin(void); void mem_hotplug_done(void); +/* See kswapd_is_running() */ static inline void pgdat_kswapd_lock(pg_data_t *pgdat) { mutex_lock(&pgdat->kswapd_lock); --- a/mm/compaction.c~mm-fix-null-ptr-deref-in-kswapd_is_running-fix +++ a/mm/compaction.c @@ -1980,6 +1980,12 @@ static inline bool is_via_compact_memory return order == -1; } +/* + * Determine whether kswapd is (or recently was!) running on this node. + * + * pgdat_kswapd_lock() pins pgdat->kswapd, so a concurrent kswapd_stop() can't + * zero it. + */ static bool kswapd_is_running(pg_data_t *pgdat) { bool running; _