linux-kernel - Re: [PATCH 1/1] slob: Only use list functions when safe to do so

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: <20190402190538.GA5084@eros.localdomain>
Date:   Wed, 3 Apr 2019 06:05:38 +1100
From:   "Tobin C. Harding" <me@...in.cc>
To:     Andrew Morton <akpm@...ux-foundation.org>
Cc:     "Tobin C. Harding" <tobin@...nel.org>, LKP <lkp@...org>,
        Roman Gushchin <guro@...com>, Christoph Lameter <cl@...ux.com>,
        Pekka Enberg <penberg@...nel.org>,
        David Rientjes <rientjes@...gle.com>,
        Joonsoo Kim <iamjoonsoo.kim@....com>,
        Matthew Wilcox <willy@...radead.org>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, kernel test robot <lkp@...el.com>
Subject: Re: [PATCH 1/1] slob: Only use list functions when safe to do so

On Mon, Apr 01, 2019 at 09:41:28PM -0700, Andrew Morton wrote:
> On Tue,  2 Apr 2019 14:29:57 +1100 "Tobin C. Harding" <tobin@...nel.org> wrote:
> 
> > Currently we call (indirectly) list_del() then we manually try to combat
> > the fact that the list may be in an undefined state by getting 'prev'
> > and 'next' pointers in a somewhat contrived manner.  It is hard to
> > verify that this works for all initial states of the list.  Clearly the
> > author (me) got it wrong the first time because the 0day kernel testing
> > robot managed to crash the kernel thanks to this code.
> > 
> > All this is done in order to do an optimisation aimed at preventing
> > fragmentation at the start of a slab.  We can just skip this
> > optimisation any time the list is put into an undefined state since this
> > only occurs when an allocation completely fills the slab and in this
> > case the optimisation is unnecessary since we have not fragmented the slab
> > by this allocation.
> > 
> > Change the page pointer passed to slob_alloc_page() to be a double
> > pointer so that we can set it to NULL to indicate that the page was
> > removed from the list.  Skip the optimisation if the page was removed.
> > 
> > Found thanks to the kernel test robot, email subject:
> > 
> > 	340d3d6178 ("mm/slob.c: respect list_head abstraction layer"):  kernel BUG at lib/list_debug.c:31!
> > 
> 
> It's regrettable that this fixes
> slob-respect-list_head-abstraction-layer.patch but doesn't apply to
> that patch - slob-use-slab_list-instead-of-lru.patch gets in the way. 
> So we end up with a patch series which introduces a bug and later
> fixes it.

Yes I thought that also.  Do you rebase the mm tree?  Did you apply this
right after slob-use-slab_list-instead-of-lru or to the current tip?  If
it is applied to the tip does this effect the ability to later bisect in
between these two commits (if the need arises for some unrelated reason)?

> I guess we can live with that but if the need comes to respin this
> series, please do simply fix
> slob-respect-list_head-abstraction-layer.patch so we get a clean
> series.

If its not too much work for you to apply the new series I'll do another
version just to get this right.

	Tobin.