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 209C2C433EF for ; Wed, 20 Jul 2022 08:21:26 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B80AA6B0072; Wed, 20 Jul 2022 04:21:25 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B31B46B0073; Wed, 20 Jul 2022 04:21:25 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9F8F56B0074; Wed, 20 Jul 2022 04:21:25 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 8FC6E6B0072 for ; Wed, 20 Jul 2022 04:21:25 -0400 (EDT) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 5690AA04EE for ; Wed, 20 Jul 2022 08:21:25 +0000 (UTC) X-FDA: 79706783730.04.6AD1C4B Received: from alexa-out-sd-01.qualcomm.com (alexa-out-sd-01.qualcomm.com [199.106.114.38]) by imf30.hostedemail.com (Postfix) with ESMTP id 618BB8009D for ; Wed, 20 Jul 2022 08:21:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; i=@quicinc.com; q=dns/txt; s=qcdkim; t=1658305284; x=1689841284; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=DyE1S6/M8cxx8TUOX92GSOdQu0Ro177cj6dL0LTcF6U=; b=VYsbSQNMyNxAtcjA8ReWy0CE4Jtm4U0/zBHNAViesOplNesiH4PbjNp7 UtILdAm8rryIF99yeWdH+BNMeuv/vr4FpyJOEygoO+hG9A+sv2MNRl6Y8 sCT1h4fhAL8VVEwKRH3pF1jdJ1uh7HxHamXVsXO9qc89qXuX+KvzZUgfy s=; Received: from unknown (HELO ironmsg-SD-alpha.qualcomm.com) ([10.53.140.30]) by alexa-out-sd-01.qualcomm.com with ESMTP; 20 Jul 2022 01:21:23 -0700 X-QCInternal: smtphost Received: from nasanex01c.na.qualcomm.com ([10.47.97.222]) by ironmsg-SD-alpha.qualcomm.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Jul 2022 01:21:22 -0700 Received: from nalasex01a.na.qualcomm.com (10.47.209.196) by nasanex01c.na.qualcomm.com (10.47.97.222) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.986.22; Wed, 20 Jul 2022 01:21:21 -0700 Received: from hu-pkondeti-hyd.qualcomm.com (10.80.80.8) by nalasex01a.na.qualcomm.com (10.47.209.196) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.986.22; Wed, 20 Jul 2022 01:21:16 -0700 Date: Wed, 20 Jul 2022 13:51:12 +0530 From: Pavan Kondeti To: Charan Teja Kalla CC: Michal Hocko , , , , , , , , , , , , , "iamjoonsoo.kim@lge.com" , Pavan Kondeti Subject: Re: [PATCH] mm: fix use-after free of page_ext after race with memory-offline Message-ID: <20220720082112.GA14437@hu-pkondeti-hyd.qualcomm.com> References: <1657810063-28938-1-git-send-email-quic_charante@quicinc.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) X-Originating-IP: [10.80.80.8] X-ClientProxiedBy: nasanex01a.na.qualcomm.com (10.52.223.231) To nalasex01a.na.qualcomm.com (10.47.209.196) ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=quicinc.com header.s=qcdkim header.b=VYsbSQNM; dmarc=pass (policy=none) header.from=quicinc.com; spf=pass (imf30.hostedemail.com: domain of quic_pkondeti@quicinc.com designates 199.106.114.38 as permitted sender) smtp.mailfrom=quic_pkondeti@quicinc.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1658305284; 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=DyE1S6/M8cxx8TUOX92GSOdQu0Ro177cj6dL0LTcF6U=; b=hkeRaqrFu2Y7dLCO729JrMXavPxk2uOFQD92GGgPFZ//S5iBeS7QNmtwL5K8xLp814OXT3 1cWF6mopfkxDoNR68xTlsrkEBOQfTKXd5UB5da33pCm79Gpcyx1JS0aom9ef5EaAcfWCsm /obguq4xIxUofj2vdj4VjV0pffZUAuY= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1658305284; a=rsa-sha256; cv=none; b=rFF8/tN5W3qAt4zlNQlKSZGbnqfrK6fH1VVMW32hi+7yuUgd23yHLVpcRCPxV+Muqy3QsZ 8GZrGiG4cJ1wjje4uJGdc+qj3+rF5TE0LQNHbRTkxHrYiv7sgBDcvRDXGjwx4N2WLkPue1 lZEX3op9pvsX1MZXOnkcy4omcHuUhL4= X-Stat-Signature: m9cup9h35zaqw7iri9outgwczjncin9n X-Rspamd-Queue-Id: 618BB8009D X-Rspamd-Server: rspam08 Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=quicinc.com header.s=qcdkim header.b=VYsbSQNM; dmarc=pass (policy=none) header.from=quicinc.com; spf=pass (imf30.hostedemail.com: domain of quic_pkondeti@quicinc.com designates 199.106.114.38 as permitted sender) smtp.mailfrom=quic_pkondeti@quicinc.com X-Rspam-User: X-HE-Tag: 1658305284-565214 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 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); } Thanks, Pavan