MIME-Version: 1.0
References: <b519e746-ddfd-421f-d897-7620d229e4b2@gmail.com>
 <16ea722f-e08d-044f-216c-4ea745cc6344@broadcom.com> <CACna6rxR4t1MpniasnUfyiuBhc9tTkJRx=O7GrMnmVPOXCZ-dQ@mail.gmail.com>
In-Reply-To: <CACna6rxR4t1MpniasnUfyiuBhc9tTkJRx=O7GrMnmVPOXCZ-dQ@mail.gmail.com>
From:   =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= <zajec5@gmail.com>
Date:   Thu, 18 Apr 2019 13:55:39 +0200
Message-ID: <CACna6rwF7rai_PMY3VtYXD=uBAT+jHsr8zrWP28vKjUx2S7Uyg@mail.gmail.com>
Subject: Re: brcmfmac: NULL pointer dereference during brcmf_detach() after
 firmware crash
To:     Arend Van Spriel <arend.vanspriel@broadcom.com>
Cc:     "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
        "open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER" 
        <brcm80211-dev-list.pdl@broadcom.com>,
        "open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER
        <brcm80211-dev-list.pdl@broadcom.com>," 
        <brcm80211-dev-list@cypress.com>, Aaron Blair <aaron@aaronpb.me>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On Fri, 15 Feb 2019 at 07:15, Rafa=C5=82 Mi=C5=82ecki <zajec5@gmail.com> wr=
ote:
> On Thu, 14 Feb 2019 at 23:37, Arend Van Spriel
> <arend.vanspriel@broadcom.com> wrote:
> > On 2/14/2019 11:30 PM, Rafa=C5=82 Mi=C5=82ecki wrote:
> > > I've just found a well reproducible brcmfmac crash (NULL pointer
> > > dereference).
> > >
> > > Steps:
> > > 1. Wait for or trigger a FullMAC firmware crash
> > > 2. Wait for some skb to get queued on a flowring
> > > 3. Call rmmod brcmfmac
> > >
> > > Problem:
> > > There is a NULL pointer dereference in one of the brcmf_detach() call=
s.
> > >
> > > Explanation:
> > > brcmf_detach() first frees all "ifp"s and then deletes flowrings. If =
any
> > > flowring has a skb it results in calling brcmf_txfinalize() which tri=
es
> > > to access "ifp" (struct brcmf_if) which is a NULL.
> >
> > Hi Rafa=C5=82,
> >
> > Thanks for diving in. That was my suspicion. Does it mean you are
> > working on a patch or shall I take care of it.
>
> It would be nice to have someone more experienced with detaching &
> rings look at it. Is adding a simple
> if (ifp)
> enough? Or should that code get redesigned? Should we e.g. reorder detach=
 order?

Hi Arend, would you find a moment to look at that crash, please?

--=20
Rafa=C5=82