MIME-Version: 1.0
References: <20230218075956.1563118-1-zyytlz.wz@163.com> <Y/U+w7aMc+BttZwl@google.com>
In-Reply-To: <Y/U+w7aMc+BttZwl@google.com>
From:   Zheng Hacker <hackerzheng666@gmail.com>
Date:   Wed, 22 Feb 2023 12:17:21 +0800
Message-ID: <CAJedcCzmnZCR=XF+zKHiJ+8PNK88sXFDm5n=RnwcTnJfO0ihOw@mail.gmail.com>
Subject: Re: [PATCH] mwifiex: Fix use-after-free bug due to race condition
 between main thread thread and timer thread
To:     Brian Norris <briannorris@chromium.org>
Cc:     Zheng Wang <zyytlz.wz@163.com>, ganapathi017@gmail.com,
        alex000young@gmail.com, amitkarwar@gmail.com,
        sharvari.harisangam@nxp.com, huxinming820@gmail.com,
        kvalo@kernel.org, davem@davemloft.net, edumazet@google.com,
        kuba@kernel.org, pabeni@redhat.com, linux-wireless@vger.kernel.org,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Brian Norris <briannorris@chromium.org> =E4=BA=8E2023=E5=B9=B42=E6=9C=8822=
=E6=97=A5=E5=91=A8=E4=B8=89 11:31=E5=86=99=E9=81=93=EF=BC=9A
>
> On Sat, Feb 18, 2023 at 03:59:56PM +0800, Zheng Wang wrote:
> > Note that, this bug is found by static analysis, it could be wrong. We
> > could discuss that before writing the fix.
>
> Yeah, please don't accept this patch. It deserves an "RFC" in the title
> at best. Sure, it's an identified race condition, but the cure here
> (deleting all possible recovery from firmware crashes) is worse than the
> disease.
>

Hello Brain,

Thanks for your reply. I do need add "RFC" in the title. Sorry about that.
The invoking chain is as descriped in the original mail.

There is some place which may confuse you. In
mwifiex_exec_next_cmd function, the adapter is got from
cmd_node->priv->adapter. You might think if this is the same adapter
in outer function. In mwifiex_register function, it inits the
priv_arrary with
adapter->priv[i]->adapter =3D adapter,
Then use mwifiex_init_lock_list to init the list. When it fetch adapter lik=
e
calling mwifiex_cfg80211_get_tx_power, it travers the array to find the
target priv. So all the adapter paramter is pointed to the same adapter.
The UAF of it is vulnerable.

In summary, after the firmware is downloaded, it will start the timer funct=
ion,
which is known as mwifiex_cmd_timeout_func. The if_ops.card_reset
function pointer is assigned with mwifiex_sdio_card_reset, which will
schedule_work the card->work. It finally pass the check becauese
card->work_flags has MWIFIEX_IFACE_WORK_CARD_RESET.

Finnaly, in _mwifiex_fw_dp, if init is failed, it will call mwifiex_free_ad=
apter
and free the adapter.


> There's no real attempt at analyzing the race or providing solutions, so
> there's not much to discuss yet.

Yes, I did't figure out a good plan to fix. As I say, it free the adapter i=
n the
error handling path. Could you please provide some advice about the fix?

Best regards,
Zheng