Skip to content
  • Paolo Abeni's avatar
    net: sched: red: avoid hashing NULL child · 8ffa5f97
    Paolo Abeni authored
    [ Upstream commit 44a63b13
    
     ]
    
    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: default avatarHangbin Liu <liuhangbin@gmail.com>
    Fixes: 49b49971
    
     ("net: sched: make default fifo qdiscs appear in the dump")
    Signed-off-by: default avatarPaolo Abeni <pabeni@redhat.com>
    Acked-by: default avatarJiri Kosina <jkosina@suse.cz>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    8ffa5f97