netdev - Re: [PATCH net 2/3] net: sched: fq_codel: fix null pointer access issue when fq_codel

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: <CANn89iJubvtbdpgKXhP8CMcWEn8Ws80sLeu=F4RMTAEKWePoyg@mail.gmail.com>
Date:   Mon, 17 Oct 2022 21:02:26 -0700
From:   Eric Dumazet <edumazet@...gle.com>
To:     Zhengchao Shao <shaozhengchao@...wei.com>
Cc:     cake@...ts.bufferbloat.net, netdev@...r.kernel.org, toke@...e.dk,
        jhs@...atatu.com, xiyou.wangcong@...il.com, jiri@...nulli.us,
        davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com,
        dave.taht@...il.com, weiyongjun1@...wei.com, yuehaibing@...wei.com
Subject: Re: [PATCH net 2/3] net: sched: fq_codel: fix null pointer access
 issue when fq_codel_init() fails

On Mon, Oct 17, 2022 at 8:39 PM Zhengchao Shao <shaozhengchao@...wei.com> wrote:
>
> When the default qdisc is fq_codel, if the qdisc of dev_queue fails to be
> inited during mqprio_init(), fq_codel_reset() is invoked to clear
> resources. In this case, the flow is NULL, and it will cause gpf issue.
>
> The process is as follows:
> qdisc_create_dflt()
>         fq_codel_init()
>                 ...
>                 q->flows_cnt = 1024;
>                 ...
>                 q->flows = kvcalloc(...)      --->failed, q->flows is NULL
>                 ...
>         ...
>         qdisc_put()
>                 ...
>                 fq_codel_reset()
>                         ...
>                         flow = q->flows + i   --->q->flows is NULL
>
> The following is the Call Trace information:
> general protection fault, probably for non-canonical address
> 0xdffffc0000000001: 0000 [#1] PREEMPT SMP KASAN
> KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
> RIP: 0010:fq_codel_reset+0x14d/0x350
> Call Trace:
> <TASK>
> qdisc_reset+0xed/0x6f0
> qdisc_destroy+0x82/0x4c0
> qdisc_put+0x9e/0xb0
> qdisc_create_dflt+0x2c3/0x4a0
> mqprio_init+0xa71/0x1760
> qdisc_create+0x3eb/0x1000
> tc_modify_qdisc+0x408/0x1720
> rtnetlink_rcv_msg+0x38e/0xac0
> netlink_rcv_skb+0x12d/0x3a0
> netlink_unicast+0x4a2/0x740
> netlink_sendmsg+0x826/0xcc0
> sock_sendmsg+0xc5/0x100
> ____sys_sendmsg+0x583/0x690
> ___sys_sendmsg+0xe8/0x160
> __sys_sendmsg+0xbf/0x160
> do_syscall_64+0x35/0x80
> entry_SYSCALL_64_after_hwframe+0x46/0xb0
> RIP: 0033:0x7fd272b22d04
> </TASK>
>
> Fixes: 494f5063b86c ("net: sched: fq_codel: remove redundant resource cleanup in fq_codel_init()")
> Signed-off-by: Zhengchao Shao <shaozhengchao@...wei.com>
> ---

I vote for a revert, previous code was much cleaner.