[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <736584e7-571d-13f9-eb3e-34ce49e71e6c@huawei.com>
Date: Mon, 28 Nov 2022 22:01:38 +0800
From: "zhangxiaoxu (A)" <zhangxiaoxu5@...wei.com>
To: Kalle Valo <kvalo@...nel.org>
CC: <Ajay.Kathat@...rochip.com>, <linux-wireless@...r.kernel.org>,
<netdev@...r.kernel.org>, <davem@...emloft.net>,
<edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com>,
<Claudiu.Beznea@...rochip.com>
Subject: Re: [PATCH wireless] wifi: wilc1000: Fix UAF in wilc_netdev_cleanup()
when iterator the RCU list
On 2022/11/28 19:14, Kalle Valo wrote:
> "zhangxiaoxu (A)" <zhangxiaoxu5@...wei.com> writes:
>
>> On 2022/11/26 0:17, Ajay.Kathat@...rochip.com wrote:
>>>
>>> On 24/11/22 20:43, Zhang Xiaoxu wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>
>>>> There is a UAF read when remove the wilc1000_spi module:
>>>>
>>>> BUG: KASAN: use-after-free in wilc_netdev_cleanup.cold+0xc4/0xe0 [wilc1000]
>>>> Read of size 8 at addr ffff888116846900 by task rmmod/386
>>>>
>>>> CPU: 2 PID: 386 Comm: rmmod Tainted: G N 6.1.0-rc6+ #8
>>>> Call Trace:
>>>> dump_stack_lvl+0x68/0x85
>>>> print_report+0x16c/0x4a3
>>>> kasan_report+0x95/0x190
>>>> wilc_netdev_cleanup.cold+0xc4/0xe0
>>>> wilc_bus_remove+0x52/0x60
>>>> spi_remove+0x46/0x60
>>>> device_remove+0x73/0xc0
>>>> device_release_driver_internal+0x12d/0x210
>>>> driver_detach+0x84/0x100
>>>> bus_remove_driver+0x90/0x120
>>>> driver_unregister+0x4f/0x80
>>>> __x64_sys_delete_module+0x2fc/0x440
>>>> do_syscall_64+0x38/0x90
>>>> entry_SYSCALL_64_after_hwframe+0x63/0xcd
>>>>
>>>> Since set 'needs_free_netdev=true' when initialize the net device, the
>>>> net device will be freed when unregister, then use the freed 'vif' to
>>>> find the next will UAF read.
>>>
>>>
>>> Did you test this behaviour on the real device. I am seeing a kernel
>>> crash when the module is unloaded after the connection with an AP.
>>
>> Thanks Ajay, I have no real device, what kind of crash about your
>> scenario?
>
> If you don't have a real device to test on, please state that clearly in
> the commit log. For example, "Compile tested only" or something like
> that.
Thanks Kalle,
I found this problem with a bpf mock device, and test this patch use
the same way.
>
> We get way too much untested patches where there's no indication that
> they have had no testing. I'm really concerned about this trend, I'm
> even considering should I just start dropping these kind of untested
> cleanup patches?
>
Powered by blists - more mailing lists