Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965163AbdCWTK6 (ORCPT ); Thu, 23 Mar 2017 15:10:58 -0400 Received: from mail-wm0-f49.google.com ([74.125.82.49]:37817 "EHLO mail-wm0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755637AbdCWTK4 (ORCPT ); Thu, 23 Mar 2017 15:10:56 -0400 MIME-Version: 1.0 In-Reply-To: References: From: Eric Dumazet Date: Thu, 23 Mar 2017 12:10:53 -0700 Message-ID: Subject: Re: net/sched: GPF in qdisc_hash_add To: Dmitry Vyukov Cc: Cong Wang , Jamal Hadi Salim , David Miller , netdev , LKML , syzkaller 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-Length: 3390 Lines: 85 On Thu, Mar 23, 2017 at 12:06 PM, Dmitry Vyukov wrote: > > On Thu, Mar 23, 2017 at 8:00 PM, Cong Wang wrote: > > On Thu, Mar 23, 2017 at 9:06 AM, Dmitry Vyukov wrote: > >> kasan: CONFIG_KASAN_INLINE enabled > >> kasan: GPF could be caused by NULL-ptr deref or user memory access > >> general protection fault: 0000 [#1] SMP KASAN > >> Dumping ftrace buffer: > >> (ftrace buffer empty) > >> Modules linked in: > >> CPU: 2 PID: 12732 Comm: syz-executor6 Not tainted 4.11.0-rc3+ #365 > >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 > >> task: ffff880062b7a2c0 task.stack: ffff880033480000 > >> RIP: 0010:qdisc_hash_add.part.19+0xb6/0x3c0 net/sched/sch_api.c:280 > >> RSP: 0018:ffff880033487820 EFLAGS: 00010202 > >> RAX: dffffc0000000000 RBX: ffffffff85357e00 RCX: ffffc90002b24000 > >> RDX: 000000000000007a RSI: ffffffff835a523a RDI: 00000000000003d0 > >> RBP: ffff8800334878b8 R08: fffffbfff0a6afeb R09: fffffbfff0a6afeb > >> R10: 0000000000000001 R11: fffffbfff0a6afea R12: ffffffff85357e48 > >> R13: 1ffff10006690f06 R14: ffff880033487890 R15: 0000000000000000 > >> FS: 00007f68665d0700(0000) GS:ffff88006e200000(0000) knlGS:0000000000000000 > >> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > >> CR2: 00000000004c2d44 CR3: 000000003c6f8000 CR4: 00000000000026e0 > >> Call Trace: > >> qdisc_hash_add+0x76/0x90 net/sched/sch_api.c:279 > >> attach_default_qdiscs net/sched/sch_generic.c:798 [inline] > >> dev_activate+0x6ca/0x920 net/sched/sch_generic.c:829 > >> __dev_open+0x25b/0x360 net/core/dev.c:1348 > >> __dev_change_flags+0x159/0x3d0 net/core/dev.c:6460 > >> dev_change_flags+0x88/0x140 net/core/dev.c:6525 > >> dev_ifsioc+0x51f/0x9b0 net/core/dev_ioctl.c:254 > >> dev_ioctl+0x1fe/0x1030 net/core/dev_ioctl.c:532 > >> sock_do_ioctl+0x94/0xb0 net/socket.c:902 > >> sock_ioctl+0x2c2/0x440 net/socket.c:993 > >> vfs_ioctl fs/ioctl.c:45 [inline] > >> do_vfs_ioctl+0x1af/0x16d0 fs/ioctl.c:685 > >> SYSC_ioctl fs/ioctl.c:700 [inline] > >> SyS_ioctl+0x8f/0xc0 fs/ioctl.c:691 > >> entry_SYSCALL_64_fastpath+0x1f/0xc2 > > > > The interesting part is why the NULL dereference is in > > qdisc_hash_add(), since we have a check before calling > > it: > > > > #ifdef CONFIG_NET_SCHED > > if (dev->qdisc) > > qdisc_hash_add(dev->qdisc); > > #endif > > > > > > When attach_one_default_qdisc() fails, we should trigger > > the NULL pointer dereference bug at: > > > > atomic_inc(&dev->qdisc->refcnt); > > I think qdisc is not NULL, it's something _in_ qdisc that is NULL. The > crash happens here: > > struct Qdisc *root = qdisc_dev(q)->qdisc; > > so it's probably device. Looks like this bug came with commit 59cc1f61f09c ("net: sched: convert qdisc linked list to hashtable") I would simply guard qdisc_hash_add() (Against &noop_qdisc) diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c index bcf49cd2278670197f2a7e9d4e9a62ae8d117468..2bb34b51cffdb05434a488b9f45c344d57868253 100644 --- a/net/sched/sch_api.c +++ b/net/sched/sch_api.c @@ -276,6 +276,8 @@ static struct Qdisc *qdisc_match_from_root(struct Qdisc *root, u32 handle) void qdisc_hash_add(struct Qdisc *q) { + if (q == &noop_qdisc) + return; if ((q->parent != TC_H_ROOT) && !(q->flags & TCQ_F_INGRESS)) { struct Qdisc *root = qdisc_dev(q)->qdisc;