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 52B90C43334 for ; Wed, 20 Jul 2022 11:13:31 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 9EF226B0073; Wed, 20 Jul 2022 07:13:30 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 99DBF6B0074; Wed, 20 Jul 2022 07:13:30 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 88CD16B0075; Wed, 20 Jul 2022 07:13:30 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 79F036B0073 for ; Wed, 20 Jul 2022 07:13:30 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 3E677C0462 for ; Wed, 20 Jul 2022 11:13:30 +0000 (UTC) X-FDA: 79707217380.22.E5E19DB Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by imf03.hostedemail.com (Postfix) with ESMTP id A38652005A for ; Wed, 20 Jul 2022 11:13:28 +0000 (UTC) Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id 2CB70209BB; Wed, 20 Jul 2022 11:13:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1658315607; 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=uVS1Ma60eQeIoz+rEvVQL37s9TZNdcW1qvb1bYiop7w=; b=qikOJz1OrtvsQzT3S3IVsO8DkmFUlUOrUn3irfxkGSwS7PfCfrkkQbLJjkDWnSgql/Y+C1 efg7qFQ6/qRrUnLiU0MW+BvNpgOB0kfZU0uYYmka2+hQUcrplKGVB3EtOpHj43EkwCdfL4 JQ8EDTuagJSf1+aUvOD0wzNG7nXYKYs= 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 775AB2C141; Wed, 20 Jul 2022 11:13:23 +0000 (UTC) Date: Wed, 20 Jul 2022 13:13:19 +0200 From: Michal Hocko To: Charan Teja Kalla Cc: Pavan Kondeti , 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: ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1658315608; a=rsa-sha256; cv=none; b=bjSpK9ktZvYew3Vw1LanxLTeh7sAR9sJ/ZsQpUK5oILe1fHIv2vblVKycGQBqJClwn5TNp +ddUx01ohlIsJ8IF+OsiR6pcBm77DvYgB7medsj+PiBwfwZfcGToJN8v9o+esjEb+IguW3 zJHiWK7B6TqxxOtFiqqsOPbWHN58P2Y= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b=qikOJz1O; spf=pass (imf03.hostedemail.com: domain of mhocko@suse.com designates 195.135.220.29 as permitted sender) smtp.mailfrom=mhocko@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1658315608; 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=uVS1Ma60eQeIoz+rEvVQL37s9TZNdcW1qvb1bYiop7w=; b=YImQQqZMPoyHS1RfBPLF94Wom2OT2rN4gm8R1NObPLAyOh+l54YM+mPDCDKFX2tI12+qEO 0SGzKaT6ZnlxWCupumu6DBdvVrauDH+IOPkNwRBjM016G+WVh1u5PVqJ9kQlt5mf8vUUiK Zw/jwKYvyFlbXHbVienp3A3qeo3DQ4Q= X-Stat-Signature: frj1y9fc1eqn1g8s8pkrhya1gcxogw7g X-Rspamd-Queue-Id: A38652005A Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b=qikOJz1O; spf=pass (imf03.hostedemail.com: domain of mhocko@suse.com designates 195.135.220.29 as permitted sender) smtp.mailfrom=mhocko@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com X-Rspam-User: X-Rspamd-Server: rspam10 X-HE-Tag: 1658315608-636397 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 16:13:19, Charan Teja Kalla wrote: > Thanks Michal & Pavan, > > On 7/20/2022 2:40 PM, Michal Hocko wrote: > >>>> 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. > > Annotating with __rcu requires all the read and writes to ms->page_ext > to be under rcu_[access|assign]_pointer which is a big patch. I think > READ_ONCE and WRITE_ONCE, mentioned by Michal, below should does the job. > > >> > >> 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. > > > > There is still a small race window b/n ms->page_ext setting NULL and its > access even under CONFIG_SPARSEMEM. In the above mentioned example: > > CPU1 CPU2 > rcu_read_lock() > lookup_page_ext(PageA): offlining > offline_page_ext > __free_page_ext(addrA) > get_entry(addrA) > if (!section->page_ext) > turns to be false. > ms->page_ext = NULL > > addrA = get_entry(base=section->page_ext): > base + page_ext_size * index; > **Since base is NULL here, caller > can still do the dereference on > the invalid pointer address.** only if the value is re-fetched. Not likely but definitely better to have it covered. That is why I was suggesting READ_ONCE/WRITE_ONCE for this iperation. > > synchronize_rcu() > free_page_ext > free_pages_exact (now ) > > > > 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. > > Yes, I should be using the READ_ONCE() and WRITE_ONCE() here. yes. -- Michal Hocko SUSE Labs