Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

net: sched: sch_htb: don't call qdisc_put() while holding tree lock

Recent changes that removed rtnl dependency from rules update path of tc
also made tcf_block_put() function sleeping. This function is called from
ops->destroy() of several Qdisc implementations, which in turn is called by
qdisc_put(). Some Qdiscs call qdisc_put() while holding sch tree spinlock,
which results sleeping-while-atomic BUG.

Steps to reproduce for htb:

tc qdisc add dev ens1f0 root handle 1: htb default 12
tc class add dev ens1f0 parent 1: classid 1:1 htb rate 100kbps ceil 100kbps
tc qdisc add dev ens1f0 parent 1:1 handle 40: sfq perturb 10
tc class add dev ens1f0 parent 1:1 classid 1:2 htb rate 100kbps ceil 100kbps

Resulting dmesg:

[ 4791.148551] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:909
[ 4791.151354] in_atomic(): 1, irqs_disabled(): 0, pid: 27273, name: tc
[ 4791.152805] INFO: lockdep is turned off.
[ 4791.153605] CPU: 19 PID: 27273 Comm: tc Tainted: G W 5.3.0-rc8+ #721
[ 4791.154336] Hardware name: Supermicro SYS-2028TP-DECR/X10DRT-P, BIOS 2.0b 03/30/2017
[ 4791.155075] Call Trace:
[ 4791.155803] dump_stack+0x85/0xc0
[ 4791.156529] ___might_sleep.cold+0xac/0xbc
[ 4791.157251] __mutex_lock+0x5b/0x960
[ 4791.157966] ? console_unlock+0x363/0x5d0
[ 4791.158676] ? tcf_chain0_head_change_cb_del.isra.0+0x1b/0xf0
[ 4791.159395] ? tcf_chain0_head_change_cb_del.isra.0+0x1b/0xf0
[ 4791.160103] tcf_chain0_head_change_cb_del.isra.0+0x1b/0xf0
[ 4791.160815] tcf_block_put_ext.part.0+0x21/0x50
[ 4791.161530] tcf_block_put+0x50/0x70
[ 4791.162233] sfq_destroy+0x15/0x50 [sch_sfq]
[ 4791.162936] qdisc_destroy+0x5f/0x160
[ 4791.163642] htb_change_class.cold+0x5df/0x69d [sch_htb]
[ 4791.164505] tc_ctl_tclass+0x19d/0x480
[ 4791.165360] rtnetlink_rcv_msg+0x170/0x4b0
[ 4791.166191] ? netlink_deliver_tap+0x95/0x400
[ 4791.166907] ? rtnl_dellink+0x2d0/0x2d0
[ 4791.167625] netlink_rcv_skb+0x49/0x110
[ 4791.168345] netlink_unicast+0x171/0x200
[ 4791.169058] netlink_sendmsg+0x224/0x3f0
[ 4791.169771] sock_sendmsg+0x5e/0x60
[ 4791.170475] ___sys_sendmsg+0x2ae/0x330
[ 4791.171183] ? ___sys_recvmsg+0x159/0x1f0
[ 4791.171894] ? do_wp_page+0x9c/0x790
[ 4791.172595] ? __handle_mm_fault+0xcd3/0x19e0
[ 4791.173309] __sys_sendmsg+0x59/0xa0
[ 4791.174024] do_syscall_64+0x5c/0xb0
[ 4791.174725] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ 4791.175435] RIP: 0033:0x7f0aa41497b8
[ 4791.176129] Code: 89 02 48 c7 c0 ff ff ff ff eb bb 0f 1f 80 00 00 00 00 f3 0f 1e fa 48 8d 05 65 8f 0c 00 8b 00 85 c0 75 17 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 58 c3 0f 1f 80 00 00 00 00 48 83 ec 28 89 5
4
[ 4791.177532] RSP: 002b:00007fff4e37d588 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
[ 4791.178243] RAX: ffffffffffffffda RBX: 000000005d8132f7 RCX: 00007f0aa41497b8
[ 4791.178947] RDX: 0000000000000000 RSI: 00007fff4e37d5f0 RDI: 0000000000000003
[ 4791.179662] RBP: 0000000000000000 R08: 0000000000000001 R09: 00000000020149a0
[ 4791.180382] R10: 0000000000404eda R11: 0000000000000246 R12: 0000000000000001
[ 4791.181100] R13: 000000000047f640 R14: 0000000000000000 R15: 0000000000000000

In htb_change_class() function save parent->leaf.q to local temporary
variable and put reference to it after sch tree lock is released in order
not to call potentially sleeping cls API in atomic section. This is safe to
do because Qdisc has already been reset by qdisc_purge_queue() inside sch
tree lock critical section.

Fixes: c266f64dbfa2 ("net: sched: protect block state with mutex")
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

authored by

Vlad Buslov and committed by
David S. Miller
4ce70b4a 05733434

+3 -1
+3 -1
net/sched/sch_htb.c
··· 1302 1302 struct htb_class *cl = (struct htb_class *)*arg, *parent; 1303 1303 struct nlattr *opt = tca[TCA_OPTIONS]; 1304 1304 struct nlattr *tb[TCA_HTB_MAX + 1]; 1305 + struct Qdisc *parent_qdisc = NULL; 1305 1306 struct tc_htb_opt *hopt; 1306 1307 u64 rate64, ceil64; 1307 1308 int warn = 0; ··· 1402 1401 if (parent && !parent->level) { 1403 1402 /* turn parent into inner node */ 1404 1403 qdisc_purge_queue(parent->leaf.q); 1405 - qdisc_put(parent->leaf.q); 1404 + parent_qdisc = parent->leaf.q; 1406 1405 if (parent->prio_activity) 1407 1406 htb_deactivate(q, parent); 1408 1407 ··· 1481 1480 cl->cbuffer = PSCHED_TICKS2NS(hopt->cbuffer); 1482 1481 1483 1482 sch_tree_unlock(sch); 1483 + qdisc_put(parent_qdisc); 1484 1484 1485 1485 if (warn) 1486 1486 pr_warn("HTB: quantum of class %X is %s. Consider r2q change.\n",