MIME-Version: 1.0
References: <20230218075956.1563118-1-zyytlz.wz@163.com> <Y/U+w7aMc+BttZwl@google.com>
 <CAJedcCzmnZCR=XF+zKHiJ+8PNK88sXFDm5n=RnwcTnJfO0ihOw@mail.gmail.com>
 <Y/aHHSkUOsOsU+Kq@google.com> <CAJedcCykky7E_uyeU=Pj1HR0rcpUTF1tKJ-2UmmM33bweDg=yw@mail.gmail.com>
 <CAJedcCztEkE=EB2GmH=BpTvD=r_bwGXk3RYDM2FU=f_SvEaJHA@mail.gmail.com> <Y/kupIIGBHQ2rQIZ@google.com>
In-Reply-To: <Y/kupIIGBHQ2rQIZ@google.com>
From:   Zheng Hacker <hackerzheng666@gmail.com>
Date:   Thu, 2 Mar 2023 17:48:49 +0800
Message-ID: <CAJedcCzKi89D+CEwrwgwT_=eXYjt3dh-m10Q8bxE9ZRB04PK+Q@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=8825=
=E6=97=A5=E5=91=A8=E5=85=AD 05:39=E5=86=99=E9=81=93=EF=BC=9A
>
> 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 connectio=
n.
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 =3D mwifiex_init_fw(adapter);
  if (ret =3D=3D -1) {
    goto err_init_fw;
  } else if (!ret) {
    adapter->hw_status =3D MWIFIEX_HW_STATUS_READY;
    goto done;
  }
```

 it continues executing in _mwifiex_fw_dpc. Then in some unexpected situati=
on,,
it'll get into error path like mwifiex_init_channel_scan_gap return non-zer=
o
code if there is no more memory to use. It'll then get into err_init_chan_s=
can
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