linux-kernel - Re: [PATCH 2] mm/kvmalloc: do not call kmalloc for size > KMALLOC_MAX

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20181101165519.GM23921@dhcp22.suse.cz>
Date:   Thu, 1 Nov 2018 17:55:19 +0100
From:   Michal Hocko <mhocko@...nel.org>
To:     Konstantin Khlebnikov <khlebnikov@...dex-team.ru>
Cc:     linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2] mm/kvmalloc: do not call kmalloc for size >
 KMALLOC_MAX_SIZE

On Thu 01-11-18 19:42:48, Konstantin Khlebnikov wrote:
> On 01.11.2018 15:55, Michal Hocko wrote:
> > On Thu 01-11-18 13:48:17, Konstantin Khlebnikov wrote:
> > > 
> > > 
> > > On 01.11.2018 13:24, Michal Hocko wrote:
> > > > On Thu 01-11-18 13:09:16, Konstantin Khlebnikov wrote:
> > > > > Allocations over KMALLOC_MAX_SIZE could be served only by vmalloc.
> > > > 
> > > > I would go on and say that allocations with sizes too large can actually
> > > > trigger a warning (once you have posted in the previous version outside
> > > > of the changelog area) because that might be interesting to people -
> > > > there are deployments to panic on warning and then a warning is much
> > > > more important.
> > > 
> > > It seems that warning isn't completely valid.
> > > 
> > > 
> > > __alloc_pages_slowpath() handles this more gracefully:
> > > 
> > > 	/*
> > > 	 * In the slowpath, we sanity check order to avoid ever trying to
> > > 	 * reclaim >= MAX_ORDER areas which will never succeed. Callers may
> > > 	 * be using allocators in order of preference for an area that is
> > > 	 * too large.
> > > 	 */
> > > 	if (order >= MAX_ORDER) {
> > > 		WARN_ON_ONCE(!(gfp_mask & __GFP_NOWARN));
> > > 		return NULL;
> > > 	}
> > > 
> > > 
> > > Fast path is ready for order >= MAX_ORDER
> > > 
> > > 
> > > Problem is in node_reclaim() which is called earlier than __alloc_pages_slowpath()
> > > from surprising place - get_page_from_freelist()
> > > 
> > > 
> > > Probably node_reclaim() simply needs something like this:
> > > 
> > > 	if (order >= MAX_ORDER)
> > > 		return NODE_RECLAIM_NOSCAN;
> > 
> > Maybe but the point is that triggering this warning is possible. Even if
> > the warning is bogus it doesn't really make much sense to even try
> > kmalloc if the size is not supported by the allocator.
> > 
> 
> But __GFP_NOWARN allocation (like in this case) should just fail silently
> without warnings regardless of reason because caller can deal with that.

__GFP_NOWARN is not about no warning to be triggered from the allocation
context. It is more about not complaining about the allocation failure.
I do not think we want to check the gfp mask in all possible paths
triggered from the allocator/reclaim.

I have just looked at the original warning you have hit and it came from
88d6ac40c1c6 ("mm/vmstat: fix divide error at __fragmentation_index"). I
would argue that the warning is a bit of an over-reaction. Regardless of
the gfp_mask.
-- 
Michal Hocko
SUSE Labs