netdev - Re: [PATCH] mwifiex: Fix use-after-free bug due to race condition between main thread thread and timer thread

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: <Y/aHHSkUOsOsU+Kq@google.com>
Date:   Wed, 22 Feb 2023 13:20:29 -0800
From:   Brian Norris <briannorris@...omium.org>
To:     Zheng Hacker <hackerzheng666@...il.com>
Cc:     Zheng Wang <zyytlz.wz@....com>, ganapathi017@...il.com,
        alex000young@...il.com, amitkarwar@...il.com,
        sharvari.harisangam@....com, huxinming820@...il.com,
        kvalo@...nel.org, davem@...emloft.net, edumazet@...gle.com,
        kuba@...nel.org, pabeni@...hat.com, linux-wireless@...r.kernel.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mwifiex: Fix use-after-free bug due to race condition
 between main thread thread and timer thread

On Wed, Feb 22, 2023 at 12:17:21PM +0800, Zheng Hacker wrote:
> Could you please provide some advice about the fix?

This entire driver's locking patterns (or lack
thereof) need rewritten. This driver was probably written by someone
that doesn't really understand concurrent programming. It really only
works because the bulk of normal operation is sequentialized into the
main loop (mwifiex_main_process()). Any time you get outside that,
you're likely to find bugs.

But now that I've looked a little further, I'm not confident you pointed
out a real bug. How does mwifiex_sdio_card_reset_work() get past
mwifiex_shutdown_sw() -> wait_for_completion(adapter->fw_done) ? That
should ensure that _mwifiex_fw_dpc() is finished, and so we can't hit
the race you point out.

Note to self: ignore most "static analysis" reports of race conditions,
unless they have thorough analysis or a runtime reproduction.

Brian