Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753313AbdDKSap (ORCPT ); Tue, 11 Apr 2017 14:30:45 -0400 Received: from mx2.suse.de ([195.135.220.15]:44718 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753176AbdDKSal (ORCPT ); Tue, 11 Apr 2017 14:30:41 -0400 Date: Tue, 11 Apr 2017 20:30:36 +0200 From: Michal Hocko To: Christoph Lameter Cc: Kees Cook , Andrew Morton , Pekka Enberg , David Rientjes , Joonsoo Kim , Linux-MM , LKML Subject: Re: [PATCH] mm: Add additional consistency check Message-ID: <20170411183035.GD21171@dhcp22.suse.cz> References: <20170404194220.GT15132@dhcp22.suse.cz> <20170404201334.GV15132@dhcp22.suse.cz> <20170411134618.GN6729@dhcp22.suse.cz> <20170411141956.GP6729@dhcp22.suse.cz> <20170411164134.GA21171@dhcp22.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1718 Lines: 40 On Tue 11-04-17 13:03:01, Cristopher Lameter wrote: > On Tue, 11 Apr 2017, Michal Hocko wrote: > > > > > > > There is a flag SLAB_DEBUG_OBJECTS that is available for this check. > > > > Which is way too late, at least for the kfree path. page->slab_cache > > on anything else than PageSlab is just a garbage. And my understanding > > of the patch objective is to stop those from happening. > > We are looking here at SLAB. SLUB code can legitimately have a compound > page there because large allocations fallback to the page allocator. > > Garbage would be attempting to free a page that has !PageSLAB set but also > is no compound page. That condition is already checked in kfree() with a > BUG_ON() and that BUG_ON has been there for a long time. Are you talking about SLAB or SLUB here? The only BUG_ON(PageSlab(page)) in SLAB I can see is in kmem_freepages and that is way too late because we already rely on cachep which is not trustworthy. Or am I missing some other place you have in mind? > Certainly we can > make SLAB consistent if there is no check there already. Slab just > attempts a free on that object which will fail too. > > So we are already handling that condition. Why change things? Add a BUG_ON > if you want to make SLAB consistent. I hate to repeat myself but let me do it for the last time in this thread. BUG_ON for something that is recoverable is completely inappropriate. And I consider kfree with a bogus pointer something that we can easily recover from. There are other cases where the internal state of the allocator is compromised to the point where continuing is not possible and BUGing there is acceptable but kfree(garbage) is not that case. -- Michal Hocko SUSE Labs