Received-SPF: pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH 5.4 regression fix] brcmfmac: Fix
 brcmf_cfg80211_get_channel returning uninitialized fields
To:     Kalle Valo <kvalo@codeaurora.org>
Cc:     Arend van Spriel <arend.vanspriel@broadcom.com>,
        Franky Lin <franky.lin@broadcom.com>,
        Hante Meuleman <hante.meuleman@broadcom.com>,
        Chi-Hsien Lin <chi-hsien.lin@cypress.com>,
        Wright Feng <wright.feng@cypress.com>,
        linux-wireless@vger.kernel.org, brcm80211-dev-list.pdl@broadcom.com
References: <20191003200821.819594-1-hdegoede@redhat.com>
 <87r23ssssh.fsf@codeaurora.org>
From:   Hans de Goede <hdegoede@redhat.com>
Message-ID: <0d86c109-78d3-3858-ae56-e4cdcb056fe4@redhat.com>
Date:   Fri, 4 Oct 2019 12:37:22 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.8.0
MIME-Version: 1.0
In-Reply-To: <87r23ssssh.fsf@codeaurora.org>
Content-Language: en-US
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: quoted-printable
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

Hi,

On 10/4/19 12:34 PM, Kalle Valo wrote:
> Hans de Goede <hdegoede@redhat.com> writes:
>=20
>> With the new edmg support struct cfg80211_chan_def has been extended
>> with a number of new members. brcmf_cfg80211_get_channel() was not setti=
ng
>> (clearing) these causing the cfg80211_edmg_chandef_valid() check in
>> cfg80211_chandef_valid() to fail. Triggering a WARN_ON and, worse, causi=
ng
>> brcmfmac based wifi cards to not work.
>>
>> This commit fixes this by clearing the entire passed struct to 0 before
>> setting the members used by the brcmfmac code. This solution also makes
>> sure that this problem will not repeat itself in the future if further
>> members are added to the struct.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c=
 b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> index e3ebb7abbdae..480c05f66ebd 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> @@ -5041,10 +5041,10 @@ static int brcmf_cfg80211_get_channel(struct wip=
hy *wiphy,
>>   =09}
>>  =20
>>   =09freq =3D ieee80211_channel_to_frequency(ch.control_ch_num, band);
>> +=09memset(chandef, 0, sizeof(*chandef));
>>   =09chandef->chan =3D ieee80211_get_channel(wiphy, freq);
>>   =09chandef->width =3D width;
>>   =09chandef->center_freq1 =3D ieee80211_channel_to_frequency(ch.chnum, =
band);
>> -=09chandef->center_freq2 =3D 0;
>>  =20
>>   =09return 0;
>>   }
>=20
> Is this a separate issue from the cfg80211 fix:
>=20
> cfg80211: initialize on-stack chandefs
>=20
> https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git/commit=
/?id=3Df43e5210c739fe76a4b0ed851559d6902f20ceb1

No it is the same issue, I was actually wondering if I should do a generic =
fix
instead... I'm glad someone has already fixed this with a generic fix. Assu=
ming the
generic fix is going upstream soon, then this (my) patch can be dropped.

Regards,

Hans