All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 5.4 regression fix] brcmfmac: Fix brcmf_cfg80211_get_channel returning uninitialized fields
@ 2019-10-03 20:08 Hans de Goede
  2019-10-04  9:29 ` Arend Van Spriel
  2019-10-04 10:34 ` Kalle Valo
  0 siblings, 2 replies; 5+ messages in thread
From: Hans de Goede @ 2019-10-03 20:08 UTC (permalink / raw)
  To: Arend van Spriel, Franky Lin, Hante Meuleman, Kalle Valo,
	Chi-Hsien Lin, Wright Feng
  Cc: Hans de Goede, linux-wireless, brcm80211-dev-list.pdl

With the new edmg support struct cfg80211_chan_def has been extended
with a number of new members. brcmf_cfg80211_get_channel() was not setting
(clearing) these causing the cfg80211_edmg_chandef_valid() check in
cfg80211_chandef_valid() to fail. Triggering a WARN_ON and, worse, causing
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 wiphy *wiphy,
 	}
 
 	freq = ieee80211_channel_to_frequency(ch.control_ch_num, band);
+	memset(chandef, 0, sizeof(*chandef));
 	chandef->chan = ieee80211_get_channel(wiphy, freq);
 	chandef->width = width;
 	chandef->center_freq1 = ieee80211_channel_to_frequency(ch.chnum, band);
-	chandef->center_freq2 = 0;
 
 	return 0;
 }
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 5.4 regression fix] brcmfmac: Fix brcmf_cfg80211_get_channel returning uninitialized fields
  2019-10-03 20:08 [PATCH 5.4 regression fix] brcmfmac: Fix brcmf_cfg80211_get_channel returning uninitialized fields Hans de Goede
@ 2019-10-04  9:29 ` Arend Van Spriel
  2019-10-04 10:34 ` Kalle Valo
  1 sibling, 0 replies; 5+ messages in thread
From: Arend Van Spriel @ 2019-10-04  9:29 UTC (permalink / raw)
  To: Hans de Goede, Franky Lin, Hante Meuleman, Kalle Valo,
	Chi-Hsien Lin, Wright Feng
  Cc: linux-wireless, brcm80211-dev-list.pdl

On 10/3/2019 10:08 PM, Hans de Goede wrote:
> With the new edmg support struct cfg80211_chan_def has been extended
> with a number of new members. brcmf_cfg80211_get_channel() was not setting
> (clearing) these causing the cfg80211_edmg_chandef_valid() check in
> cfg80211_chandef_valid() to fail. Triggering a WARN_ON and, worse, causing
> 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.

Guess this warrants a Fixes: tag and ...

Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> 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 wiphy *wiphy,
>   	}
>   
>   	freq = ieee80211_channel_to_frequency(ch.control_ch_num, band);
> +	memset(chandef, 0, sizeof(*chandef));
>   	chandef->chan = ieee80211_get_channel(wiphy, freq);
>   	chandef->width = width;
>   	chandef->center_freq1 = ieee80211_channel_to_frequency(ch.chnum, band);
> -	chandef->center_freq2 = 0;
>   
>   	return 0;
>   }
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 5.4 regression fix] brcmfmac: Fix brcmf_cfg80211_get_channel returning uninitialized fields
  2019-10-03 20:08 [PATCH 5.4 regression fix] brcmfmac: Fix brcmf_cfg80211_get_channel returning uninitialized fields Hans de Goede
  2019-10-04  9:29 ` Arend Van Spriel
@ 2019-10-04 10:34 ` Kalle Valo
  2019-10-04 10:37   ` Hans de Goede
  1 sibling, 1 reply; 5+ messages in thread
From: Kalle Valo @ 2019-10-04 10:34 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Arend van Spriel, Franky Lin, Hante Meuleman, Chi-Hsien Lin,
	Wright Feng, linux-wireless, brcm80211-dev-list.pdl

Hans de Goede <hdegoede@redhat.com> writes:

> With the new edmg support struct cfg80211_chan_def has been extended
> with a number of new members. brcmf_cfg80211_get_channel() was not setting
> (clearing) these causing the cfg80211_edmg_chandef_valid() check in
> cfg80211_chandef_valid() to fail. Triggering a WARN_ON and, worse, causing
> 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 wiphy *wiphy,
>  	}
>  
>  	freq = ieee80211_channel_to_frequency(ch.control_ch_num, band);
> +	memset(chandef, 0, sizeof(*chandef));
>  	chandef->chan = ieee80211_get_channel(wiphy, freq);
>  	chandef->width = width;
>  	chandef->center_freq1 = ieee80211_channel_to_frequency(ch.chnum, band);
> -	chandef->center_freq2 = 0;
>  
>  	return 0;
>  }

Is this a separate issue from the cfg80211 fix:

cfg80211: initialize on-stack chandefs

https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git/commit/?id=f43e5210c739fe76a4b0ed851559d6902f20ceb1

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 5.4 regression fix] brcmfmac: Fix brcmf_cfg80211_get_channel returning uninitialized fields
  2019-10-04 10:34 ` Kalle Valo
@ 2019-10-04 10:37   ` Hans de Goede
  2019-10-04 11:19     ` Kalle Valo
  0 siblings, 1 reply; 5+ messages in thread
From: Hans de Goede @ 2019-10-04 10:37 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Arend van Spriel, Franky Lin, Hante Meuleman, Chi-Hsien Lin,
	Wright Feng, linux-wireless, brcm80211-dev-list.pdl

Hi,

On 10/4/19 12:34 PM, Kalle Valo wrote:
> Hans de Goede <hdegoede@redhat.com> writes:
> 
>> With the new edmg support struct cfg80211_chan_def has been extended
>> with a number of new members. brcmf_cfg80211_get_channel() was not setting
>> (clearing) these causing the cfg80211_edmg_chandef_valid() check in
>> cfg80211_chandef_valid() to fail. Triggering a WARN_ON and, worse, causing
>> 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 wiphy *wiphy,
>>   	}
>>   
>>   	freq = ieee80211_channel_to_frequency(ch.control_ch_num, band);
>> +	memset(chandef, 0, sizeof(*chandef));
>>   	chandef->chan = ieee80211_get_channel(wiphy, freq);
>>   	chandef->width = width;
>>   	chandef->center_freq1 = ieee80211_channel_to_frequency(ch.chnum, band);
>> -	chandef->center_freq2 = 0;
>>   
>>   	return 0;
>>   }
> 
> Is this a separate issue from the cfg80211 fix:
> 
> cfg80211: initialize on-stack chandefs
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git/commit/?id=f43e5210c739fe76a4b0ed851559d6902f20ceb1

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. Assuming the
generic fix is going upstream soon, then this (my) patch can be dropped.

Regards,

Hans


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 5.4 regression fix] brcmfmac: Fix brcmf_cfg80211_get_channel returning uninitialized fields
  2019-10-04 10:37   ` Hans de Goede
@ 2019-10-04 11:19     ` Kalle Valo
  0 siblings, 0 replies; 5+ messages in thread
From: Kalle Valo @ 2019-10-04 11:19 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Arend van Spriel, Franky Lin, Hante Meuleman, Chi-Hsien Lin,
	Wright Feng, linux-wireless, brcm80211-dev-list.pdl

Hans de Goede <hdegoede@redhat.com> writes:

> Hi,
>
> On 10/4/19 12:34 PM, Kalle Valo wrote:
>> Hans de Goede <hdegoede@redhat.com> writes:
>>
>>> With the new edmg support struct cfg80211_chan_def has been extended
>>> with a number of new members. brcmf_cfg80211_get_channel() was not setting
>>> (clearing) these causing the cfg80211_edmg_chandef_valid() check in
>>> cfg80211_chandef_valid() to fail. Triggering a WARN_ON and, worse, causing
>>> 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 wiphy *wiphy,
>>>   	}
>>>     	freq = ieee80211_channel_to_frequency(ch.control_ch_num,
>>> band);
>>> +	memset(chandef, 0, sizeof(*chandef));
>>>   	chandef->chan = ieee80211_get_channel(wiphy, freq);
>>>   	chandef->width = width;
>>>   	chandef->center_freq1 = ieee80211_channel_to_frequency(ch.chnum, band);
>>> -	chandef->center_freq2 = 0;
>>>     	return 0;
>>>   }
>>
>> Is this a separate issue from the cfg80211 fix:
>>
>> cfg80211: initialize on-stack chandefs
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git/commit/?id=f43e5210c739fe76a4b0ed851559d6902f20ceb1
>
> 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. Assuming the
> generic fix is going upstream soon

It's in net tree right now so it should go to Linus' tree soon.

> then this (my) patch can be dropped.

Ok, I'll drop it.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-10-04 11:19 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-03 20:08 [PATCH 5.4 regression fix] brcmfmac: Fix brcmf_cfg80211_get_channel returning uninitialized fields Hans de Goede
2019-10-04  9:29 ` Arend Van Spriel
2019-10-04 10:34 ` Kalle Valo
2019-10-04 10:37   ` Hans de Goede
2019-10-04 11:19     ` Kalle Valo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.