• Eric Dumazet's avatar
    net_sched: fix qdisc_tree_decrease_qlen() races · 4eaf3b84
    Eric Dumazet authored
    qdisc_tree_decrease_qlen() suffers from two problems on multiqueue
    devices.
    
    One problem is that it updates sch->q.qlen and sch->qstats.drops
    on the mq/mqprio root qdisc, while it should not : Daniele
    reported underflows errors :
    [  681.774821] PAX: sch->q.qlen: 0 n: 1
    [  681.774825] PAX: size overflow detected in function qdisc_tree_decrease_qlen net/sched/sch_api.c:769 cicus.693_49 min, count: 72, decl: qlen; num: 0; context: sk_buff_head;
    [  681.774954] CPU: 2 PID: 19 Comm: ksoftirqd/2 Tainted: G           O    4.2.6.201511282239-1-grsec #1
    [  681.774955] Hardware name: ASUSTeK COMPUTER INC. X302LJ/X302LJ, BIOS X302LJ.202 03/05/2015
    [  681.774956]  ffffffffa9a04863 0000000000000000 0000000000000000 ffffffffa990ff7c
    [  681.774959]  ffffc90000d3bc38 ffffffffa95d2810 0000000000000007 ffffffffa991002b
    [  681.774960]  ffffc90000d3bc68 ffffffffa91a44f4 0000000000000001 0000000000000001
    [  681.774962] Call Trace:
    [  681.774967]  [<ffffffffa95d2810>] dump_stack+0x4c/0x7f
    [  681.774970]  [<ffffffffa91a44f4>] report_size_overflow+0x34/0x50
    [  681.774972]  [<ffffffffa94d17e2>] qdisc_tree_decrease_qlen+0x152/0x160
    [  681.774976]  [<ffffffffc02694b1>] fq_codel_dequeue+0x7b1/0x820 [sch_fq_codel]
    [  681.774978]  [<ffffffffc02680a0>] ? qdisc_peek_dequeued+0xa0/0xa0 [sch_fq_codel]
    [  681.774980]  [<ffffffffa94cd92d>] __qdisc_run+0x4d/0x1d0
    [  681.774983]  [<ffffffffa949b2b2>] net_tx_action+0xc2/0x160
    [  681.774985]  [<ffffffffa90664c1>] __do_softirq+0xf1/0x200
    [  681.774987]  [<ffffffffa90665ee>] run_ksoftirqd+0x1e/0x30
    [  681.774989]  [<ffffffffa90896b0>] smpboot_thread_fn+0x150/0x260
    [  681.774991]  [<ffffffffa9089560>] ? sort_range+0x40/0x40
    [  681.774992]  [<ffffffffa9085fe4>] kthread+0xe4/0x100
    [  681.774994]  [<ffffffffa9085f00>] ? kthread_worker_fn+0x170/0x170
    [  681.774995]  [<ffffffffa95d8d1e>] ret_from_fork+0x3e/0x70
    
    mq/mqprio have their own ways to report qlen/drops by folding stats on
    all their queues, with appropriate locking.
    
    A second problem is that qdisc_tree_decrease_qlen() calls qdisc_lookup()
    without proper locking : concurrent qdisc updates could corrupt the list
    that qdisc_match_from_root() parses to find a qdisc given its handle.
    
    Fix first problem adding a TCQ_F_NOPARENT qdisc flag that
    qdisc_tree_decrease_qlen() can use to abort its tree traversal,
    as soon as it meets a mq/mqprio qdisc children.
    
    Second problem can be fixed by RCU protection.
    Qdisc are already freed after RCU grace period, so qdisc_list_add() and
    qdisc_list_del() simply have to use appropriate rcu list variants.
    
    A future patch will add a per struct netdev_queue list anchor, so that
    qdisc_tree_decrease_qlen() can have more efficient lookups.
    Reported-by: default avatarDaniele Fucini <dfucini@gmail.com>
    Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
    Cc: Cong Wang <cwang@twopensource.com>
    Cc: Jamal Hadi Salim <jhs@mojatatu.com>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    4eaf3b84
sch_api.c 45.2 KB