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] [day] [month] [year] [list]

Message-ID: <d8c1daa0-32d4-7c95-1ebe-037396309e86@huawei.com>
Date:   Tue, 18 Oct 2022 14:09:43 +0800
From:   shaozhengchao <shaozhengchao@...wei.com>
To:     Eric Dumazet <edumazet@...gle.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 2022/10/18 12:02, Eric Dumazet wrote:
> 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.
Hi Eric:
	Thank you for your review. I will revert it in V2.

Zhengchao Shao