netdev - Re: [PATCH net] sch_htb: fix crash on init failure

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 29 Aug 2017 11:20:53 -0700
From:   Cong Wang <xiyou.wangcong@...il.com>
To:     Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
Cc:     Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Eric Dumazet <edumazet@...gle.com>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Jiri Pirko <jiri@...nulli.us>,
        Roopa Prabhu <roopa@...ulusnetworks.com>
Subject: Re: [PATCH net] sch_htb: fix crash on init failure

On Tue, Aug 29, 2017 at 10:58 AM, Nikolay Aleksandrov
<nikolay@...ulusnetworks.com> wrote:
> 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: 87b60cfacf9f ("net_sched: fix error recovery at qdisc creation")
> Signed-off-by: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
> ---
> Always calling qdisc destroy on init failure in the default qdisc handling
> was added in commit 0fbbeb1ba43b. I'm not sure if I should include that
> one as fixes tag.
>
> There're more fixes to come, some are much easier to trigger without
> modifications to tc.

Acked-by: Cong Wang <xiyou.wangcong@...il.com>