From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-14.4 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6CB34C5DF60 for ; Thu, 7 Nov 2019 11:15:19 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 198F821882 for ; Thu, 7 Nov 2019 11:15:18 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="gI9Ei2/E" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 198F821882 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 995816B000A; Thu, 7 Nov 2019 06:15:18 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 945A96B000C; Thu, 7 Nov 2019 06:15:18 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8355B6B000D; Thu, 7 Nov 2019 06:15:18 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0215.hostedemail.com [216.40.44.215]) by kanga.kvack.org (Postfix) with ESMTP id 6CC186B000A for ; Thu, 7 Nov 2019 06:15:18 -0500 (EST) Received: from smtpin22.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with SMTP id 1D43B5010 for ; Thu, 7 Nov 2019 11:15:18 +0000 (UTC) X-FDA: 76129225116.22.price01_2e36c38e33447 X-HE-Tag: price01_2e36c38e33447 X-Filterd-Recvd-Size: 9001 Received: from mail-wr1-f67.google.com (mail-wr1-f67.google.com [209.85.221.67]) by imf09.hostedemail.com (Postfix) with ESMTP for ; Thu, 7 Nov 2019 11:15:17 +0000 (UTC) Received: by mail-wr1-f67.google.com with SMTP id h3so2492229wrx.12 for ; Thu, 07 Nov 2019 03:15:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=lkhKlP+nRmdiHSeBByl2uzBydDfCxHRYE22JZBDwszc=; b=gI9Ei2/E5vg3K70OQbtvyLQFKNCmpvb02EzBVRHawvo509ZV+i6q/NuFzM512blCCw w8FivJLQ5+sD4Ra7JGG0QBRBuIXkqRGvlu7nUHq/1/r0Ctb95RbMva/tTYnOfc2m4+bA DGor7b9uEfdoemUrrH8DPT4lzzaIekGREGHXI7zuCcfSW7Yf7GtXVVDw8TQBIgM0ro6J RD6hFqkl1hT65pOS0RUCZXqYFWjtDyk0FEHuEpeYtglM2Br8ox9jY1KV86BGABMINWhb 0PtfO7a2lKsxYBooRqTroJiMScICvc06lhMJBwprgyF889dZsiXge/vY4Xf2sSaiFHey z5mg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=lkhKlP+nRmdiHSeBByl2uzBydDfCxHRYE22JZBDwszc=; b=nSRx2Y4eZlTH7Ht3BWMd534izot2WG7nmO7gJsPKFXsvv296ik7LleXa0pYiGPUfFS ZHH3mqwTANXABr0mcL85fJtId1p9PP0rJCbawBFbdzAJMASCfWt79r270h3LPn06hrgW Qs97RVZLAPTCyDrynXNg0IuerOD0pGfu+HlyDa02Ns7mC+8chbCpeFgfnQRjQkY0Ndyu 89Ou//Su344aR/pz2We28QHLLqZoP5JKb2Nqeul6g1+18Q9buWCk2JK7DKpsLOFxfO00 O9jRb5DCcdEYpiFggoJgoEzVoa+NqMMp1ig15xerI25KR+07cMZQEVwhymbs0acwGeZy f9Ng== X-Gm-Message-State: APjAAAUAqbOZwGSX4r2BwUfySZuLBTtqfYtADdceLuqGnWu3WS0+fi9N JzgUyxLnLEGRogggmzs8M1o8FTsWQernzRcjkWrf+Q== X-Google-Smtp-Source: APXvYqz7YI76LF+j2pFIef8kTkDy0Is83JgCQNNutw3kIaS4vQMomuB1a03nTsKljJfzzgJey6bPXaX4wzbgLLbrR2g= X-Received: by 2002:a5d:4ecd:: with SMTP id s13mr2419196wrv.216.1573125315730; Thu, 07 Nov 2019 03:15:15 -0800 (PST) MIME-Version: 1.0 References: <20191106222208.26815-1-labbott@redhat.com> In-Reply-To: <20191106222208.26815-1-labbott@redhat.com> From: Alexander Potapenko Date: Thu, 7 Nov 2019 12:15:04 +0100 Message-ID: Subject: Re: [PATCH] mm: slub: Really fix slab walking for init_on_free To: Laura Abbott Cc: Andrew Morton , Networking , Linux Memory Management List , LKML , "David S. Miller" , Kees Cook , clipos@ssi.gouv.fr, Vlastimil Babka , Thibaut Sautereau Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Wed, Nov 6, 2019 at 11:22 PM Laura Abbott wrote: > > Commit 1b7e816fc80e ("mm: slub: Fix slab walking for init_on_free") > fixed one problem with the slab walking but missed a key detail: > When walking the list, the head and tail pointers need to be updated > since we end up reversing the list as a result. Without doing this, > bulk free is broken. One way this is exposed is a NULL pointer with > slub_debug=3DF: Thanks for the fix! Is it possible to reproduce the problem on a small test case that could be added to lib/test_meminit.c? > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D > BUG skbuff_head_cache (Tainted: G T): Object already free > -------------------------------------------------------------------------= ---- > > INFO: Slab 0x000000000d2d2f8f objects=3D16 used=3D3 fp=3D0x00000000643090= 71 flags=3D0x3fff00000000201 > BUG: kernel NULL pointer dereference, address: 0000000000000000 > PGD 0 P4D 0 > Oops: 0000 [#1] PREEMPT SMP PTI > CPU: 0 PID: 0 Comm: swapper/0 Tainted: G B T 5.3.8 #1 > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015 > RIP: 0010:print_trailer+0x70/0x1d5 > Code: 28 4d 8b 4d 00 4d 8b 45 20 81 e2 ff 7f 00 00 e8 86 ce ef ff 8b 4b 2= 0 48 89 ea 48 89 ee 4c 29 e2 48 c7 c7 90 6f d4 89 48 01 e9 <48> 33 09 48 33= 8b 70 01 00 00 e8 61 ce ef ff f6 43 09 04 74 35 8b > RSP: 0018:ffffbf7680003d58 EFLAGS: 00010046 > RAX: 000000000000005d RBX: ffffa3d2bb08e540 RCX: 0000000000000000 > RDX: 00005c2d8fdc2000 RSI: 0000000000000000 RDI: ffffffff89d46f90 > RBP: 0000000000000000 R08: 0000000000000242 R09: 000000000000006c > R10: 0000000000000000 R11: 0000000000000030 R12: ffffa3d27023e000 > R13: fffff11080c08f80 R14: ffffa3d2bb047a80 R15: 0000000000000002 > FS: 0000000000000000(0000) GS:ffffa3d2be400000(0000) knlGS:0000000000000= 000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 0000000000000000 CR3: 000000007a6c4000 CR4: 00000000000006f0 > Call Trace: > > free_debug_processing.cold.37+0xc9/0x149 > ? __kfree_skb_flush+0x30/0x40 > ? __kfree_skb_flush+0x30/0x40 > __slab_free+0x22a/0x3d0 > ? tcp_wfree+0x2a/0x140 > ? __sock_wfree+0x1b/0x30 > kmem_cache_free_bulk+0x415/0x420 > ? __kfree_skb_flush+0x30/0x40 > __kfree_skb_flush+0x30/0x40 > net_rx_action+0x2dd/0x480 > __do_softirq+0xf0/0x246 > irq_exit+0x93/0xb0 > do_IRQ+0xa0/0x110 > common_interrupt+0xf/0xf > > > Given we're now almost identical to the existing debugging > code which correctly walks the list, combine with that. > > Link: https://lkml.kernel.org/r/20191104170303.GA50361@gandi.net > Reported-by: Thibaut Sautereau > Fixes: 1b7e816fc80e ("mm: slub: Fix slab walking for init_on_free") > Signed-off-by: Laura Abbott Acked-by: Alexander Potapenko > --- > mm/slub.c | 39 +++++++++------------------------------ > 1 file changed, 9 insertions(+), 30 deletions(-) > > diff --git a/mm/slub.c b/mm/slub.c > index dac41cf0b94a..d2445dd1c7ed 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -1432,12 +1432,15 @@ static inline bool slab_free_freelist_hook(struct= kmem_cache *s, > void *old_tail =3D *tail ? *tail : *head; > int rsize; > > - if (slab_want_init_on_free(s)) { > - void *p =3D NULL; > + /* Head and tail of the reconstructed freelist */ > + *head =3D NULL; > + *tail =3D NULL; > > - do { > - object =3D next; > - next =3D get_freepointer(s, object); > + do { > + object =3D next; > + next =3D get_freepointer(s, object); > + > + if (slab_want_init_on_free(s)) { > /* > * Clear the object and the metadata, but don't t= ouch > * the redzone. > @@ -1447,29 +1450,8 @@ static inline bool slab_free_freelist_hook(struct = kmem_cache *s, > : 0; > memset((char *)object + s->inuse, 0, > s->size - s->inuse - rsize); > - set_freepointer(s, object, p); > - p =3D object; > - } while (object !=3D old_tail); > - } > - > -/* > - * Compiler cannot detect this function can be removed if slab_free_hook= () > - * evaluates to nothing. Thus, catch all relevant config debug options = here. > - */ > -#if defined(CONFIG_LOCKDEP) || \ > - defined(CONFIG_DEBUG_KMEMLEAK) || \ > - defined(CONFIG_DEBUG_OBJECTS_FREE) || \ > - defined(CONFIG_KASAN) > > - next =3D *head; > - > - /* Head and tail of the reconstructed freelist */ > - *head =3D NULL; > - *tail =3D NULL; > - > - do { > - object =3D next; > - next =3D get_freepointer(s, object); > + } > /* If object's reuse doesn't have to be delayed */ > if (!slab_free_hook(s, object)) { > /* Move object to the new freelist */ > @@ -1484,9 +1466,6 @@ static inline bool slab_free_freelist_hook(struct k= mem_cache *s, > *tail =3D NULL; > > return *head !=3D NULL; > -#else > - return true; > -#endif > } > > static void *setup_object(struct kmem_cache *s, struct page *page, > -- > 2.21.0 > --=20 Alexander Potapenko Software Engineer Google Germany GmbH Erika-Mann-Stra=C3=9Fe, 33 80636 M=C3=BCnchen Gesch=C3=A4ftsf=C3=BChrer: Paul Manicle, Halimah DeLaine Prado Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg