Skip to content
  • Nikolay Aleksandrov's avatar
    sch_htb: fix crash on init failure · 951104e4
    Nikolay Aleksandrov authored
    commit 88c2ace6 upstream.
    
    The commit below added a call to the ->destroy() callback for all qdiscs
    which failed in their ->init(), but some were not prepared for such
    change and can't handle partially initialized qdisc. HTB is one of them
    and if any error occurs before the qdisc watchdog timer and qdisc work are
    initialized then we can hit either a null ptr deref (timer->base) when
    canceling in ->destroy or lockdep error info about trying to register
    a non-static key and a stack dump. So to fix these two move the watchdog
    timer and workqueue init before anything that can err out.
    To reproduce userspace needs to send broken htb qdisc create request,
    tested with a modified tc (q_htb.c).
    
    Trace log:
    [ 2710.897602] BUG: unable to handle kernel NULL pointer dereference at (null)
    [ 2710.897977] IP: hrtimer_active+0x17/0x8a
    [ 2710.898174] PGD 58fab067
    [ 2710.898175] P4D 58fab067
    [ 2710.898353] PUD 586c0067
    [ 2710.898531] PMD 0
    [ 2710.898710]
    [ 2710.899045] Oops: 0000 [#1] SMP
    [ 2710.899232] Modules linked in:
    [ 2710.899419] CPU: 1 PID: 950 Comm: tc Not tainted 4.13.0-rc6+ #54
    [ 2710.899646] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.7.5-20140531_083030-gandalf 04/01/2014
    [ 2710.900035] task: ffff880059ed2700 task.stack: ffff88005ad4c000
    [ 2710.900262] RIP: 0010:hrtimer_active+0x17/0x8a
    [ 2710.900467] RSP: 0018:ffff88005ad4f960 EFLAGS: 00010246
    [ 2710.900684] RAX: 0000000000000000 RBX: ffff88003701e298 RCX: 0000000000000000
    [ 2710.900933] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff88003701e298
    [ 2710.901177] RBP: ffff88005ad4f980 R08: 0000000000000001 R09: 0000000000000001
    [ 2710.901419] R10: ffff88005ad4f800 R11: 0000000000000400 R12: 0000000000000000
    [ 2710.901663] R13: ffff88003701e298 R14: ffffffff822a4540 R15: ffff88005ad4fac0
    [ 2710.901907] FS:  00007f2f5e90f740(0000) GS:ffff88005d880000(0000) knlGS:0000000000000000
    [ 2710.902277] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    [ 2710.902500] CR2: 0000000000000000 CR3: 0000000058ca3000 CR4: 00000000000406e0
    [ 2710.902744] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
    [ 2710.902977] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
    [ 2710.903180] Call Trace:
    [ 2710.903332]  hrtimer_try_to_cancel+0x1a/0x93
    [ 2710.903504]  hrtimer_cancel+0x15/0x20
    [ 2710.903667]  qdisc_watchdog_cancel+0x12/0x14
    [ 2710.903866]  htb_destroy+0x2e/0xf7
    [ 2710.904097]  qdisc_create+0x377/0x3fd
    [ 2710.904330]  tc_modify_qdisc+0x4d2/0x4fd
    [ 2710.904511]  rtnetlink_rcv_msg+0x188/0x197
    [ 2710.904682]  ? rcu_read_unlock+0x3e/0x5f
    [ 2710.904849]  ? rtnl_newlink+0x729/0x729
    [ 2710.905017]  netlink_rcv_skb+0x6c/0xce
    [ 2710.905183]  rtnetlink_rcv+0x23/0x2a
    [ 2710.905345]  netlink_unicast+0x103/0x181
    [ 2710.905511]  netlink_sendmsg+0x326/0x337
    [ 2710.905679]  sock_sendmsg_nosec+0x14/0x3f
    [ 2710.905847]  sock_sendmsg+0x29/0x2e
    [ 2710.906010]  ___sys_sendmsg+0x209/0x28b
    [ 2710.906176]  ? do_raw_spin_unlock+0xcd/0xf8
    [ 2710.906346]  ? _raw_spin_unlock+0x27/0x31
    [ 2710.906514]  ? __handle_mm_fault+0x651/0xdb1
    [ 2710.906685]  ? check_chain_key+0xb0/0xfd
    [ 2710.906855]  __sys_sendmsg+0x45/0x63
    [ 2710.907018]  ? __sys_sendmsg+0x45/0x63
    [ 2710.907185]  SyS_sendmsg+0x19/0x1b
    [ 2710.907344]  entry_SYSCALL_64_fastpath+0x23/0xc2
    
    Note that probably this bug goes further back because the default qdisc
    handling always calls ->destroy on init failure too.
    
    Fixes: 87b60cfa ("net_sched: fix error recovery at qdisc creation")
    Fixes: 0fbbeb1b
    
     ("[PKT_SCHED]: Fix missing qdisc_destroy() in qdisc_create_dflt()")
    Signed-off-by: default avatarNikolay Aleksandrov <nikolay@cumulusnetworks.com>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    Signed-off-by: default avatarAmit Pundir <amit.pundir@linaro.org>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    951104e4