Re: [PATCH V2] mm: fix use-after free of page_ext after race with memory-offline

From: Michal Hocko <mhocko@suse.com>
To: Charan Teja Kalla <quic_charante@quicinc.com>
Cc: akpm@linux-foundation.org, david@redhat.com,
	quic_pkondeti@quicinc.com, pasha.tatashin@soleen.com,
	sjpark@amazon.de, sieberf@amazon.com, shakeelb@google.com,
	dhowells@redhat.com, willy@infradead.org, minchan@kernel.org,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [PATCH V2] mm: fix use-after free of page_ext after race with memory-offline
Date: Mon, 1 Aug 2022 15:08:17 +0200	[thread overview]
Message-ID: <YufQQQyUmn4y+CYX@dhcp22.suse.cz> (raw)
In-Reply-To: <54e4ce7d-7cbd-480c-28ba-cba684341b37@quicinc.com>

On Mon 01-08-22 18:31:45, Charan Teja Kalla wrote:
[...]
> >> The roll back operation in the online_page_ext(), where we free the
> >> allocated page_ext's, will not have the PAGE_EXT_INVALID flag thus
> >> WARN() may not work here. no?
> > Wouldn't ms->page_ext be NULL in that case?
> I don't think that ms->page_ext would be NULL here.
> online_page_ext():
>   (a) for (pfn = start; !fail && pfn < end; pfn += PAGES_PER_SECTION)
>      fail = init_section_page_ext():
> 	   ms->page_ext = (void *)base - page_ext_size * pfn;
> 
>   //If fail = -ERROR in the middle, roll back operation.
>   (b) for (pfn = start; pfn < end; pfn += PAGES_PER_SECTION)
>        __free_page_ext();
> 
>    Here (b) can be called on the sections without PAGE_EXT_INVALID with
> ms->page_ext != NULL.
> 
You are right. My sloppy code reading. A tiny comment would be nice.
Because this shouldn't really happen for normal calls.

-- 
Michal Hocko
SUSE Labs