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

Message-ID: <CAJedcCzKi89D+CEwrwgwT_=eXYjt3dh-m10Q8bxE9ZRB04PK+Q@mail.gmail.com>
Date:   Thu, 2 Mar 2023 17:48:49 +0800
From:   Zheng Hacker <hackerzheng666@...il.com>
To:     Brian Norris <briannorris@...omium.org>
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

Brian Norris <briannorris@...omium.org> 于2023年2月25日周六 05:39写道：
>
> On Fri, Feb 24, 2023 at 02:17:59PM +0800, Zheng Hacker wrote:
> > This email is broken for the statement is too long, Here is the newest email.
>
> It still wraps a bit weird, but it's good enough I suppose.
>
> >               retn -EINPROGRESS in mwifiex_init_fw
> >               mwifiex_free_adapter when in error
>
> These two statements don't connect. _mwifiex_fw_dpc() only treats -1 as
> a true error; -EINPROGRESS is treated as success, such that we continue
> to wait for the command response. Now, we might hang here if that
> response doesn't come, but that's a different problem...
>

Hi Brain,

Sorry for my unclear description. As you say, they don't have any connection.
What I really want to express is after mwifiex_init_fw return -EINPROGRESS
to its invoker, which is _mwifiex_fw_dpc. It will pass the check of
return value,
as the following code.
```cpp
ret = mwifiex_init_fw(adapter);
  if (ret == -1) {
    goto err_init_fw;
  } else if (!ret) {
    adapter->hw_status = MWIFIEX_HW_STATUS_READY;
    goto done;
  }
```

 it continues executing in _mwifiex_fw_dpc. Then in some unexpected situation,,
it'll get into error path like mwifiex_init_channel_scan_gap return non-zero
code if there is no more memory to use. It'll then get into err_init_chan_scan
label and call mwifiex_free_adapte finally.  The other thread may USE it
afterwards.

```cpp
if (mwifiex_init_channel_scan_gap(adapter)) {
    mwifiex_dbg(adapter, ERROR,
          "could not init channel stats table\n");
    goto err_init_chan_scan;
  }
```
> I'm sure there are true bugs in here somewhere, but I've spent enough
> time reading your incorrect reports and don't plan to spend more. (If
> you're lucky, maybe you can pique my curiosity again, but don't count on
> it.)
>

BUT after reviewing the code carefully, I found this might not happen
due to some  exclusive condition. So yes, I also think there is still some
problem here but I'm kind of busy nowadays. I promise to attach a clearer
report next time.

So sorry to bother you so many times. And appreciate for your precious
time spending on this report.

Best regards,
Zheng