Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752720AbdGLOLb (ORCPT ); Wed, 12 Jul 2017 10:11:31 -0400 Received: from mail-ua0-f179.google.com ([209.85.217.179]:32978 "EHLO mail-ua0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751898AbdGLOLa (ORCPT ); Wed, 12 Jul 2017 10:11:30 -0400 MIME-Version: 1.0 In-Reply-To: <20170710133238.2afcda57ea28e020ca03c4f0@linux-foundation.org> References: <20170707083408.40410-1-glider@google.com> <20170707132351.4f10cd778fc5eb58e9cc5513@linux-foundation.org> <20170710133238.2afcda57ea28e020ca03c4f0@linux-foundation.org> From: Alexander Potapenko Date: Wed, 12 Jul 2017 16:11:28 +0200 Message-ID: Subject: Re: [PATCH] slub: make sure struct kmem_cache_node is initialized before publication To: Andrew Morton Cc: Christoph Lameter , Dmitriy Vyukov , Kostya Serebryany , LKML , Linux Memory Management List , Pekka Enberg , David Rientjes , Joonsoo Kim Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by nfs id v6CEBhGD002803 Content-Length: 1966 Lines: 62 Hi everyone, On Mon, Jul 10, 2017 at 10:32 PM, Andrew Morton wrote: > On Fri, 7 Jul 2017 18:18:31 -0500 (CDT) Christoph Lameter wrote: > >> On Fri, 7 Jul 2017, Andrew Morton wrote: >> >> > On Fri, 7 Jul 2017 10:34:08 +0200 Alexander Potapenko wrote: >> > >> > > --- a/mm/slub.c >> > > +++ b/mm/slub.c >> > > @@ -3389,8 +3389,8 @@ static int init_kmem_cache_nodes(struct kmem_cache *s) >> > > return 0; >> > > } >> > > >> > > - s->node[node] = n; >> > > init_kmem_cache_node(n); >> > > + s->node[node] = n; >> > > } >> > > return 1; >> > > } >> > >> > If this matters then I have bad feelings about free_kmem_cache_nodes(): >> >> At creation time the kmem_cache structure is private and no one can run a >> free operation. I've double-checked the code path and this turned out to be a false positive caused by KMSAN not instrumenting the contents of mm/slub.c (i.e. the initialization of the spinlock remained unnoticed). Christoph is indeed right that kmem_cache_structure is private, so a race is not possible here. I am sorry for the false alarm. >> > Inviting a use-after-free? I guess not, as there should be no way >> > to look up these items at this stage. >> >> Right. > > Still. It looks bad, and other sites do these things in the other order. If the maintainers agree the initialization order needs to be fixed, we'll need to remove the (irrelevant) KMSAN report from the patch description. >> > Could the slab maintainers please take a look at these and also have a >> > think about Alexander's READ_ONCE/WRITE_ONCE question? >> >> Was I cced on these? > > It's all on linux-mm. -- Alexander Potapenko Software Engineer Google Germany GmbH Erika-Mann-Straße, 33 80636 München Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg