Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) client-ip=23.128.96.35;
MIME-Version: 1.0
References: <20231106141704.866455-1-zyytlz.wz@163.com> <87o7g7ueom.fsf@kernel.org>
 <CAJedcCytuGmvubqbSZgsU3Db=rg=xM+kSuLZn8BSvA18Yn+9Jw@mail.gmail.com>
 <18ba5520da0.279b.9b12b7fc0a3841636cfb5e919b41b954@broadcom.com>
 <CAJedcCxoL+L1QPaZty27k6kqR2JRjxPVY=BV5xn7BSPojbxe=A@mail.gmail.com> <fa0e7536-9b05-42fb-9fff-acd2ffad9af9@broadcom.com>
In-Reply-To: <fa0e7536-9b05-42fb-9fff-acd2ffad9af9@broadcom.com>
From:   Zheng Hacker <hackerzheng666@gmail.com>
Date:   Wed, 15 Nov 2023 23:00:33 +0800
Message-ID: <CAJedcCzj9SFbx-=xDymqJyV2fu0xjmz2RH4+gT+Gxsqubg35ZA@mail.gmail.com>
Subject: Re: [PATCH v5] wifi: brcmfmac: Fix use-after-free bug in brcmf_cfg80211_detach
To:     Arend van Spriel <arend.vanspriel@broadcom.com>
Cc:     Kalle Valo <kvalo@kernel.org>, Zheng Wang <zyytlz.wz@163.com>,
        aspriel@gmail.com, franky.lin@broadcom.com,
        hante.meuleman@broadcom.com, johannes.berg@intel.com,
        marcan@marcan.st, linus.walleij@linaro.org,
        jisoo.jang@yonsei.ac.kr, linuxlovemin@yonsei.ac.kr,
        wataru.gohda@cypress.com, linux-wireless@vger.kernel.org,
        brcm80211-dev-list.pdl@broadcom.com,
        SHA-cyfmac-dev-list@infineon.com, linux-kernel@vger.kernel.org,
        security@kernel.org, stable@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Arend van Spriel <arend.vanspriel@broadcom.com> =E4=BA=8E2023=E5=B9=B411=E6=
=9C=8813=E6=97=A5=E5=91=A8=E4=B8=80 17:18=E5=86=99=E9=81=93=EF=BC=9A
>
> On November 8, 2023 4:03:26 AM Zheng Hacker <hackerzheng666@gmail.com>
> wrote:
>
> > Arend Van Spriel <arend.vanspriel@broadcom.com> =E4=BA=8E2023=E5=B9=B41=
1=E6=9C=886=E6=97=A5=E5=91=A8=E4=B8=80 23:48=E5=86=99=E9=81=93=EF=BC=9A
> >>
> >> On November 6, 2023 3:44:53 PM Zheng Hacker <hackerzheng666@gmail.com>=
 wrote:
> >>
> >>> Thanks! I didn't test it for I don't have a device. Very appreciated
> >>> if anyone could help with that.
> >>
> >> I would volunteer, but it made me dig deep and not sure if there is a
> >> problem to solve here.
> >>
> >> brcmf_cfg80211_detach() calls wl_deinit_priv() -> brcmf_abort_scanning=
() ->
> >> brcmf_notify_escan_complete() which does delete the timer.
> >>
> >> What am I missing here?
> >
> > Thanks four your detailed review. I did see the code and not sure if
> > brcmf_notify_escan_complete
> > would be triggered for sure. So in the first version I want to delete
> > the pending timer ahead of time.
>
> Why requesting a CVE when you are not sure? Seems a bit hasty to put it
> mildly.

I'm sure the issue exists because there's only cancler of timer but not wok=
er.
As there's similar CVEs before like : https://github.com/V4bel/CVE-2022-412=
18,
I submit it as soon as I found it.

>
> > As I'm not very familiar with the logic here. I'm still not sure if we
> > should delete the timer_shutdown_sync.
> > Looking forward to your reply :)
>
> Reading the kerneldoc of timer_shutdown_sync() has the advantage that
> the timer can not be rearmed by another thread. However, that will only
> happen when a new scan is initiated in firmware, but the bus is already
> down so that can not happen. The only improvement (no bug fix!) I see
> here is to replace timer handling code in brcmf_notify_escan_complete():
>
> -       if (timer_pending(&cfg->escan_timeout))
> -               del_timer_sync(&cfg->escan_timeout);
> +       timer_delete_sync(&cfg->escan_timeout);
>

Very thanks for your reviews and suggestions! I thinks it's a good
idea. I'll make
another patch sooner or later.

Best regards,
Zheng

> Regards,
> Arend