lkml.org 
[lkml]   [2018]   [May]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.16 005/161] net: sched: red: avoid hashing NULL child
    Date
    4.16-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Paolo Abeni <pabeni@redhat.com>

    [ Upstream commit 44a63b137f7b6e4c7bd6c9cc21615941cb36509d ]

    Hangbin reported an Oops triggered by the syzkaller qdisc rules:

    kasan: GPF could be caused by NULL-ptr deref or user memory access
    general protection fault: 0000 [#1] SMP KASAN PTI
    Modules linked in: sch_red
    CPU: 0 PID: 28699 Comm: syz-executor5 Not tainted 4.17.0-rc4.kcov #1
    Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
    RIP: 0010:qdisc_hash_add+0x26/0xa0
    RSP: 0018:ffff8800589cf470 EFLAGS: 00010203
    RAX: dffffc0000000000 RBX: 0000000000000000 RCX: ffffffff824ad971
    RDX: 0000000000000007 RSI: ffffc9000ce9f000 RDI: 000000000000003c
    RBP: 0000000000000001 R08: ffffed000b139ea2 R09: ffff8800589cf4f0
    R10: ffff8800589cf50f R11: ffffed000b139ea2 R12: ffff880054019fc0
    R13: ffff880054019fb4 R14: ffff88005c0af600 R15: ffff880054019fb0
    FS: 00007fa6edcb1700(0000) GS:ffff88005ce00000(0000) knlGS:0000000000000000
    CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    CR2: 0000000020000740 CR3: 000000000fc16000 CR4: 00000000000006f0
    DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
    DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
    Call Trace:
    red_change+0x2d2/0xed0 [sch_red]
    qdisc_create+0x57e/0xef0
    tc_modify_qdisc+0x47f/0x14e0
    rtnetlink_rcv_msg+0x6a8/0x920
    netlink_rcv_skb+0x2a2/0x3c0
    netlink_unicast+0x511/0x740
    netlink_sendmsg+0x825/0xc30
    sock_sendmsg+0xc5/0x100
    ___sys_sendmsg+0x778/0x8e0
    __sys_sendmsg+0xf5/0x1b0
    do_syscall_64+0xbd/0x3b0
    entry_SYSCALL_64_after_hwframe+0x44/0xa9
    RIP: 0033:0x450869
    RSP: 002b:00007fa6edcb0c48 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
    RAX: ffffffffffffffda RBX: 00007fa6edcb16b4 RCX: 0000000000450869
    RDX: 0000000000000000 RSI: 00000000200000c0 RDI: 0000000000000013
    RBP: 000000000072bea0 R08: 0000000000000000 R09: 0000000000000000
    R10: 0000000000000000 R11: 0000000000000246 R12: 00000000ffffffff
    R13: 0000000000008778 R14: 0000000000702838 R15: 00007fa6edcb1700
    Code: e9 0b fe ff ff 0f 1f 44 00 00 55 53 48 89 fb 89 f5 e8 3f 07 f3 fe 48 8d 7b 3c 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <0f> b6 14 02 48 89 f8 83 e0 07 83 c0 03 38 d0 7c 04 84 d2 75 51
    RIP: qdisc_hash_add+0x26/0xa0 RSP: ffff8800589cf470

    When a red qdisc is updated with a 0 limit, the child qdisc is left
    unmodified, no additional scheduler is created in red_change(),
    the 'child' local variable is rightfully NULL and must not add it
    to the hash table.

    This change addresses the above issue moving qdisc_hash_add() right
    after the child qdisc creation. It additionally removes unneeded checks
    for noop_qdisc.

    Reported-by: Hangbin Liu <liuhangbin@gmail.com>
    Fixes: 49b499718fa1 ("net: sched: make default fifo qdiscs appear in the dump")
    Signed-off-by: Paolo Abeni <pabeni@redhat.com>
    Acked-by: Jiri Kosina <jkosina@suse.cz>
    Signed-off-by: David S. Miller <davem@davemloft.net>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    net/sched/sch_red.c | 5 +++--
    net/sched/sch_tbf.c | 5 +++--
    2 files changed, 6 insertions(+), 4 deletions(-)

    --- a/net/sched/sch_red.c
    +++ b/net/sched/sch_red.c
    @@ -222,10 +222,11 @@ static int red_change(struct Qdisc *sch,
    extack);
    if (IS_ERR(child))
    return PTR_ERR(child);
    - }

    - if (child != &noop_qdisc)
    + /* child is fifo, no need to check for noop_qdisc */
    qdisc_hash_add(child, true);
    + }
    +
    sch_tree_lock(sch);
    q->flags = ctl->flags;
    q->limit = ctl->limit;
    --- a/net/sched/sch_tbf.c
    +++ b/net/sched/sch_tbf.c
    @@ -383,6 +383,9 @@ static int tbf_change(struct Qdisc *sch,
    err = PTR_ERR(child);
    goto done;
    }
    +
    + /* child is fifo, no need to check for noop_qdisc */
    + qdisc_hash_add(child, true);
    }

    sch_tree_lock(sch);
    @@ -391,8 +394,6 @@ static int tbf_change(struct Qdisc *sch,
    q->qdisc->qstats.backlog);
    qdisc_destroy(q->qdisc);
    q->qdisc = child;
    - if (child != &noop_qdisc)
    - qdisc_hash_add(child, true);
    }
    q->limit = qopt->limit;
    if (tb[TCA_TBF_PBURST])

    \
     
     \ /
      Last update: 2018-05-24 12:38    [W:4.137 / U:0.112 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site