Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753604AbdGJJT6 (ORCPT ); Mon, 10 Jul 2017 05:19:58 -0400 Received: from mail-vk0-f54.google.com ([209.85.213.54]:34788 "EHLO mail-vk0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752387AbdGJJT5 (ORCPT ); Mon, 10 Jul 2017 05:19:57 -0400 MIME-Version: 1.0 In-Reply-To: References: <20170707083408.40410-1-glider@google.com> <20170707132351.4f10cd778fc5eb58e9cc5513@linux-foundation.org> From: Alexander Potapenko Date: Mon, 10 Jul 2017 11:19:55 +0200 Message-ID: Subject: Re: [PATCH] slub: make sure struct kmem_cache_node is initialized before publication To: Christoph Lameter Cc: Andrew Morton , 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 v6A9LrY4004939 Content-Length: 1630 Lines: 55 On Sat, Jul 8, 2017 at 1:18 AM, 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. > >> Inviting a use-after-free? I guess not, as there should be no way >> to look up these items at this stage. > > Right. > >> 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? I've asked Andrew about READ_ONCE privately. My concern is as follows. Since unfreeze_partials() sees uninitialized value of n->list_lock, I was suspecting there's a data race between unfreeze_partials() and init_kmem_cache_nodes(). If so, reads and writes to s->node[node] must be acquire/release atomics (not actually READ_ONCE/WRITE_ONCE, but smp_load_acquire/smp_store_release). -- 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