Re: [PATCH resend] mm/page_alloc: fix comment is __get_free_pages

[Michal Hocko]

On Fri 01-12-17 08:24:14, Michal Hocko wrote:
> On Thu 30-11-17 13:17:06, Andrew Morton wrote:
> > On Thu, 30 Nov 2017 07:53:35 +0100 Michal Hocko <mhocko@xxxxxxxxxx> wrote:
> > 
> > > > mm...  So we have a caller which hopes to be getting highmem pages but
> > > > isn't.  Caller then proceeds to pointlessly kmap the page and wonders
> > > > why it isn't getting as much memory as it would like on 32-bit systems,
> > > > etc.
> > > 
> > > How he can kmap the page when he gets a _virtual_ address?
> > 
> > doh.
> > 
> > > > I do think we should help ferret out such bogosity.  A WARN_ON_ONCE
> > > > would suffice.
> > > 
> > > This function has always been about lowmem pages. I seriously doubt we
> > > have anybody confused and asking for a highmem page in the kernel. I
> > > haven't checked that but it would already blow up as VM_BUG_ON tends to
> > > be enabled on many setups.
> > 
> > OK.  But silently accepting __GFP_HIGHMEM is a bit weird - callers
> > shouldn't be doing that in the first place.
> 
> Yes, they shouldn't be.
> 
> > I wonder what happens if we just remove the WARN_ON and pass any
> > __GFP_HIGHMEM straight through.  The caller gets a weird address from
> > page_to_virt(highmem page) and usually goes splat?  Good enough
> > treatment for something which never happens anyway?
> 
> page_address will return NULL so they will blow up and leak the freshly
> allocated memory.

let me be more specific. They will blow up and leak if the returned
address is not checked. If it is then we just leak. None of that sounds
good to me.

-- 
Michal Hocko
SUSE Labs