MIME-Version: 1.0
References: <b519e746-ddfd-421f-d897-7620d229e4b2@gmail.com> <16ea722f-e08d-044f-216c-4ea745cc6344@broadcom.com>
In-Reply-To: <16ea722f-e08d-044f-216c-4ea745cc6344@broadcom.com>
From:   =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= <zajec5@gmail.com>
Date:   Fri, 15 Feb 2019 07:15:15 +0100
Message-ID: <CACna6rxR4t1MpniasnUfyiuBhc9tTkJRx=O7GrMnmVPOXCZ-dQ@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 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() calls.
> >
> > Explanation:
> > brcmf_detach() first frees all "ifp"s and then deletes flowrings. If an=
y
> > flowring has a skb it results in calling brcmf_txfinalize() which tries
> > 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 o=
rder?

--=20
Rafa=C5=82