linux-kernel - Re: [PATCH] qdisc: fix NULL pointer dereference in perf_trace_qdisc

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]

Date: Sat, 22 Jun 2024 00:06:56 +0900
From: Yunseong Kim <yskelg@...il.com>
To: Pedro Tammela <pctammela@...atatu.com>,
 Steven Rostedt <rostedt@...dmis.org>, Masami Hiramatsu
 <mhiramat@...nel.org>, Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
 Takashi Iwai <tiwai@...e.de>, "David S. Miller" <davem@...emloft.net>,
 Thomas Hellström <thomas.hellstrom@...ux.intel.com>,
 "Rafael J. Wysocki" <rafael@...nel.org>, Jamal Hadi Salim
 <jhs@...atatu.com>, Cong Wang <xiyou.wangcong@...il.com>,
 Jiri Pirko <jiri@...nulli.us>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>
Cc: Taehee Yoo <ap420073@...il.com>, Austin Kim <austindh.kim@...il.com>,
 shjy180909@...il.com, linux-kernel@...r.kernel.org,
 linux-trace-kernel@...r.kernel.org, netdev@...r.kernel.org,
 pbuk5246@...il.com
Subject: Re: [PATCH] qdisc: fix NULL pointer dereference in
 perf_trace_qdisc_reset()

Hi Pedro,

On 6/21/24 11:24 오후, Pedro Tammela wrote:
> On 21/06/2024 08:45, yskelg@...il.com wrote:
>> From: Yunseong Kim <yskelg@...il.com>
>>
>> In the TRACE_EVENT(qdisc_reset) NULL dereference occurred from
>>
>>   qdisc->dev_queue->dev <NULL> ->name
>>
>> This situation simulated from bunch of veths and Bluetooth
>> dis/reconnection.
>>
>> During qdisc initialization, qdisc was being set to noop_queue.
>> In veth_init_queue, the initial tx_num was reduced back to one,
>> causing the qdisc reset to be called with noop, which led to the
>> kernel panic.
>>
>> I think this will happen on the kernel version.
>>   Linux kernel version ≥ v6.7.10, ≥ v6.8 ≥ v6.9 and 6.10
> 
> You should tag your patch for the net tree
Thank you for the code review, I will tag the next patch for the net tree.

>> This occurred from 51270d573a8d. I think this patch is absolutely
>> necessary. Previously, It was showing not intended string value of name.
> Add a 'Fixes:' tag with this commit

I will added 'Fixes: 51270d573a8d' Tag on patch v2 message.

>> I can attach a sys-execprog's executing program, kernel dump and dmesg
>> if someone need it, but I'm not sure how to safely attach large vmcore
>> with vmlinux.
> 
> The syzkaller program + C reproducer is usually enough, please make it
> visible somewhere

I got it, I have a converted C syz program. So, I've attached the GitHub
gist link and C source code in this mail.

 https://gist.github.com/yskelg/cc64562873ce249cdd0d5a358b77d740

>> Signed-off-by: Yunseong Kim <yskelg@...il.com>, Yeoreum Yun
>> <yeoreum.yun@....com>
> 
> Should be two SoB tags

Oh, It's the first time we've sent together, I made a mistake.. Sorry.
Thank you Pedro for the advice!

>> ---
>>   include/trace/events/qdisc.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/trace/events/qdisc.h b/include/trace/events/qdisc.h
>> index f1b5e816e7e5..170b51fbe47a 100644
>> --- a/include/trace/events/qdisc.h
>> +++ b/include/trace/events/qdisc.h
>> @@ -81,7 +81,7 @@ TRACE_EVENT(qdisc_reset,
>>       TP_ARGS(q),
>>         TP_STRUCT__entry(
>> -        __string(    dev,        qdisc_dev(q)->name    )
>> +        __string(dev, qdisc_dev(q) ? qdisc_dev(q)->name : "noop_queue")
>>           __string(    kind,        q->ops->id        )
>>           __field(    u32,        parent            )
>>           __field(    u32,        handle            )
> 

Warm Regards,
Yunseong Kim
View attachment "qdisc-null-ptr-deref.c" of type "text/plain" (9962 bytes)