Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <b65dad8137855de027ce332c0f1b87837287a038.camel@sipsolutions.net>
Subject: Re: [PATCH 08/15] wifi: iwlwifi: mvm: Add NULL check before
 dereferencing the pointer
From:   Johannes Berg <johannes@sipsolutions.net>
To:     "Greenman, Gregory" <gregory.greenman@intel.com>
Cc:     "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
        "Sisodiya, Mukesh" <mukesh.sisodiya@intel.com>
Date:   Wed, 14 Jun 2023 14:32:11 +0200
In-Reply-To: <75ea42d01f6397f5b46842e6eccd3fced9b6e31a.camel@intel.com>
References: <20230612155116.168000-1-gregory.greenman@intel.com>
         <20230612184434.cf7a5ce82fb0.Id3c05d13eeee6638f0930f750e93fb928d5c9dee@changeid>
         <416fc1eba88e13c245fac4499ee0af2efbd5485a.camel@sipsolutions.net>
         <75ea42d01f6397f5b46842e6eccd3fced9b6e31a.camel@intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.48.3 (3.48.3-1.fc38) 
MIME-Version: 1.0
Precedence: bulk

On Wed, 2023-06-14 at 12:28 +0000, Greenman, Gregory wrote:
> On Wed, 2023-06-14 at 12:07 +0200, Johannes Berg wrote:
> > On Mon, 2023-06-12 at 18:51 +0300, gregory.greenman@intel.com=C2=A0wrot=
e:
> > > From: Mukesh Sisodiya <mukesh.sisodiya@intel.com>
> > >=20
> > > The p2p, bss and ap vif pointers are assigned based on the mode.
> > > All pointers will not have valid value at same time and can be
> > > NULL, based on configured mode. This can lead to NULL pointer
> > > access.
> >=20
> > This is not true.
> >=20
> > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0/* enable PM on bss i=
f bss stand alone */
> > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if (vifs->bss_active && !v=
ifs->p2p_active && !vifs->ap_active) {
> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if (bss_mvmvif && vifs->bs=
s_active && !vifs->p2p_active &&
> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 !vifs->=
ap_active) {
> > >=20
> >=20
> > The pointers can only be NULL iff *_active is false, however, it may be
> > false even if the pointer is non-NULL, so it's not exactly the same.
> >=20
> > Probably a static checker thing that didn't understand it?
> >=20
> > johannes
>=20
> Right, so the commit message could be rephrased like this:
> "While vif pointers are protected by the corresponding "*active" fields,
> static checkers can get confused sometimes. Add an explicit check."
>=20
> Do you want me to resend it with the fixed commit message?

Yes please. I also delegated this and the other one to you in patchwork
again.

johannes