netdev - Re: [PATCH net] net: sched: red: avoid hashing NULL child

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]

Message-ID: <nycvar.YFH.7.76.1805181622200.27054@cbobk.fhfr.pm>
Date:   Fri, 18 May 2018 16:22:45 +0200 (CEST)
From:   Jiri Kosina <jikos@...nel.org>
To:     Paolo Abeni <pabeni@...hat.com>
cc:     netdev@...r.kernel.org, Jamal Hadi Salim <jhs@...atatu.com>,
        Cong Wang <xiyou.wangcong@...il.com>,
        Jiri Pirko <jiri@...nulli.us>,
        "David S. Miller" <davem@...emloft.net>,
        Hangbin Liu <liuhangbin@...il.com>
Subject: Re: [PATCH net] net: sched: red: avoid hashing NULL child

On Fri, 18 May 2018, Paolo Abeni wrote:

> 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@...il.com>
> Fixes: 49b499718fa1 ("net: sched: make default fifo qdiscs appear in the dump")

Acked-by: Jiri Kosina <jkosina@...e.cz>

Thanks for fixing this Paolo.

-- 
Jiri Kosina
SUSE Labs