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 8A571C43334 for ; Wed, 20 Jul 2022 09:10:24 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id F14CE6B0072; Wed, 20 Jul 2022 05:10:23 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id EC5166B0073; Wed, 20 Jul 2022 05:10:23 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DB3536B0074; Wed, 20 Jul 2022 05:10:23 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id CAE4A6B0072 for ; Wed, 20 Jul 2022 05:10:23 -0400 (EDT) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id A1AD51A0540 for ; Wed, 20 Jul 2022 09:10:23 +0000 (UTC) X-FDA: 79706907126.18.8316246 Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by imf01.hostedemail.com (Postfix) with ESMTP id 33E6C40076 for ; Wed, 20 Jul 2022 09:10:23 +0000 (UTC) Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id 9C73A37585; Wed, 20 Jul 2022 09:10:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1658308221; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=CAdq3+Dl59dOrtqGUadXbH90BiOSvY1QRdbnjHxitEY=; b=koq0O3YT0a8AFx9ViFLP55NVJSwPRrVN9orjcew1VcG7KtPIbpJQaHt331bBg7aQuI76bU cfezxUbxmK01wnYRU+2iJefDIrsaAMCCQoCtkhlPxPnz98nWB8RMki78YicWB9pg+DDG9w D9i8k5rGU7iX0TWgEj5v+2tYuYfUsSY= Received: from suse.cz (unknown [10.100.201.86]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id B92EB2C141; Wed, 20 Jul 2022 09:10:17 +0000 (UTC) Date: Wed, 20 Jul 2022 11:10:17 +0200 From: Michal Hocko To: Pavan Kondeti Cc: Charan Teja Kalla , akpm@linux-foundation.org, pasha.tatashin@soleen.com, sjpark@amazon.de, sieberf@amazon.com, shakeelb@google.com, dhowells@redhat.com, willy@infradead.org, vbabka@suse.cz, david@redhat.com, minchan@kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, "iamjoonsoo.kim@lge.com" Subject: Re: [PATCH] mm: fix use-after free of page_ext after race with memory-offline Message-ID: References: <1657810063-28938-1-git-send-email-quic_charante@quicinc.com> <20220720082112.GA14437@hu-pkondeti-hyd.qualcomm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220720082112.GA14437@hu-pkondeti-hyd.qualcomm.com> ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b=koq0O3YT; dmarc=pass (policy=quarantine) header.from=suse.com; spf=pass (imf01.hostedemail.com: domain of mhocko@suse.com designates 195.135.220.28 as permitted sender) smtp.mailfrom=mhocko@suse.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1658308223; 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:dkim-signature; bh=CAdq3+Dl59dOrtqGUadXbH90BiOSvY1QRdbnjHxitEY=; b=n71j7ejVb6sZQtciwiX6zSNApaeC5yRKWfXsAhImvNnhRgqiNc847YM0x0Uo6Lcz3K/zHE 8B4R9svkEIDc8hIEIK7d6GSi+euO5MxFd/J5MBz7SjJGUkZcWYlyKOU4sbDM8S7BIqG2W2 0WwT7fT/WOHXmzBx37gpOzedRzC1h+Y= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1658308223; a=rsa-sha256; cv=none; b=EebIcVlnfpindGmOxNfPJTuOwvLx4MAwft5bheI0S29AwOkQgbswR8n8O0l+dqtsjvArkD zIxH1gleDiFEQhVNNkyjrSE8kjeEiME8+AkHXZZ/kd1SKpPwd32CmhwpXXALIsZObMo2fJ l5RJsUM3M8hNFwi/OExe8xTDFybSTdE= X-Rspamd-Queue-Id: 33E6C40076 Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b=koq0O3YT; dmarc=pass (policy=quarantine) header.from=suse.com; spf=pass (imf01.hostedemail.com: domain of mhocko@suse.com designates 195.135.220.28 as permitted sender) smtp.mailfrom=mhocko@suse.com X-Rspam-User: X-Rspamd-Server: rspam03 X-Stat-Signature: i7rkrm1et8t4seksp9g7rdic69e7os14 X-HE-Tag: 1658308223-409775 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 Wed 20-07-22 13:51:12, Pavan Kondeti wrote: > Hi Charan, > > On Tue, Jul 19, 2022 at 08:42:42PM +0530, Charan Teja Kalla wrote: > > Thanks Michal!! > > > > On 7/18/2022 8:24 PM, Michal Hocko wrote: > > >>>> The above mentioned race is just one example __but the problem persists > > >>>> in the other paths too involving page_ext->flags access(eg: > > >>>> page_is_idle())__. Since offline waits till the last reference on the > > >>>> page goes down i.e. any path that took the refcount on the page can make > > >>>> the memory offline operation to wait. Eg: In the migrate_pages() > > >>>> operation, we do take the extra refcount on the pages that are under > > >>>> migration and then we do copy page_owner by accessing page_ext. For > > >>>> > > >>>> Fix those paths where offline races with page_ext access by maintaining > > >>>> synchronization with rcu lock. > > >>> Please be much more specific about the synchronization. How does RCU > > >>> actually synchronize the offlining and access? Higher level description > > >>> of all the actors would be very helpful not only for the review but also > > >>> for future readers. > > >> I will improve the commit message about this synchronization change > > >> using RCU's. > > > Thanks! The most imporant part is how the exclusion is actual achieved > > > because that is not really clear at first sight > > > > > > CPU1 CPU2 > > > lookup_page_ext(PageA) offlining > > > offline_page_ext > > > __free_page_ext(addrA) > > > get_entry(addrA) > > > ms->page_ext = NULL > > > synchronize_rcu() > > > free_page_ext > > > free_pages_exact (now addrA is unusable) > > > > > > rcu_read_lock() > > > entryA = get_entry(addrA) > > > base + page_ext_size * index # an address not invalidated by the freeing path > > > do_something(entryA) > > > rcu_read_unlock() > > > > > > CPU1 never checks ms->page_ext so it cannot bail out early when the > > > thing is torn down. Or maybe I am missing something. I am not familiar > > > with page_ext much. > > > > > > Thanks a lot for catching this Michal. You are correct that the proposed > > code from me is still racy. I Will correct this along with the proper > > commit message in the next version of this patch. > > > > Trying to understand your discussion with Michal. What part is still racy? We > do check for mem_section::page_ext and bail out early from lookup_page_ext(), > no? > > Also to make this scheme explicit, we can annotate page_ext member with __rcu > and use rcu_assign_pointer() on the writer side. > > struct page_ext *lookup_page_ext(const struct page *page) > { > unsigned long pfn = page_to_pfn(page); > struct mem_section *section = __pfn_to_section(pfn); > /* > * The sanity checks the page allocator does upon freeing a > * page can reach here before the page_ext arrays are > * allocated when feeding a range of pages to the allocator > * for the first time during bootup or memory hotplug. > */ > if (!section->page_ext) > return NULL; > return get_entry(section->page_ext, pfn); > } You are right. I was looking at the wrong implementation and misread ifdef vs. ifndef CONFIG_SPARSEMEM. My bad. Memory hotplug is not supported outside of CONFIG_SPARSEMEM so the scheme should really work. I would use READ_ONCE for ms->page_ext and WRITE_ONCE on the initialization side. -- Michal Hocko SUSE Labs