2021-12-08 08:43:05

by Loic Poulain

[permalink] [raw]
Subject: [PATCH] brcmfmac: Fix incorrect type assignments for keep-alive

cpu_to_le16 has been used for all keep-alive fields, but 'perdio_msec'
is a 32-bit field and 'keep_alive_id' a 8-bit one. Fix that.

Fixes: 7a6cfe28ae3e ("brcmfmac: Configure keep-alive packet on suspend")
Reported-by: kernel test robot <[email protected]>
Signed-off-by: Loic Poulain <[email protected]>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 1679361..ba52318 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -3908,9 +3908,9 @@ static int brcmf_keepalive_start(struct brcmf_if *ifp, unsigned int interval)

/* Configure Null function/data keepalive */
kalive.version = cpu_to_le16(1);
- kalive.period_msec = cpu_to_le16(interval * MSEC_PER_SEC);
+ kalive.period_msec = cpu_to_le32(interval * MSEC_PER_SEC);
kalive.len_bytes = cpu_to_le16(0);
- kalive.keep_alive_id = cpu_to_le16(0);
+ kalive.keep_alive_id = 0;

ret = brcmf_fil_iovar_data_set(ifp, "mkeep_alive", &kalive, sizeof(kalive));
if (ret)
--
2.7.4



2021-12-08 13:37:34

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: Fix incorrect type assignments for keep-alive

On 12/8/2021 9:55 AM, Loic Poulain wrote:
> cpu_to_le16 has been used for all keep-alive fields, but 'perdio_msec'
> is a 32-bit field and 'keep_alive_id' a 8-bit one. Fix that.
>
> Fixes: 7a6cfe28ae3e ("brcmfmac: Configure keep-alive packet on suspend")
> Reported-by: kernel test robot <[email protected]>
> Signed-off-by: Loic Poulain <[email protected]>
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index 1679361..ba52318 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -3908,9 +3908,9 @@ static int brcmf_keepalive_start(struct brcmf_if *ifp, unsigned int interval)

missed it in original patch, but prefer u32 typed.

>
> /* Configure Null function/data keepalive */
> kalive.version = cpu_to_le16(1);
> - kalive.period_msec = cpu_to_le16(interval * MSEC_PER_SEC);
> + kalive.period_msec = cpu_to_le32(interval * MSEC_PER_SEC);

should we check whether multiplication (interval * MSEC_PER_SEC) does
not overflow 32 bits? The only caller now uses 30 as interval so that is
fine.

Regards,
Arend


Attachments:
smime.p7s (4.12 kB)
S/MIME Cryptographic Signature

2021-12-13 18:57:23

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: Fix incorrect type assignments for keep-alive

Loic Poulain <[email protected]> wrote:

> cpu_to_le16 has been used for all keep-alive fields, but 'perdio_msec'
> is a 32-bit field and 'keep_alive_id' a 8-bit one. Fix that.
>
> Fixes: 7a6cfe28ae3e ("brcmfmac: Configure keep-alive packet on suspend")
> Reported-by: kernel test robot <[email protected]>
> Signed-off-by: Loic Poulain <[email protected]>

Patch applied to wireless-drivers-next.git, thanks.

78da5cca6ece brcmfmac: Fix incorrect type assignments for keep-alive

--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


2021-12-13 19:04:40

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: Fix incorrect type assignments for keep-alive

Kalle Valo <[email protected]> writes:

> Loic Poulain <[email protected]> wrote:
>
>> cpu_to_le16 has been used for all keep-alive fields, but 'perdio_msec'
>> is a 32-bit field and 'keep_alive_id' a 8-bit one. Fix that.
>>
>> Fixes: 7a6cfe28ae3e ("brcmfmac: Configure keep-alive packet on suspend")
>> Reported-by: kernel test robot <[email protected]>
>> Signed-off-by: Loic Poulain <[email protected]>
>
> Patch applied to wireless-drivers-next.git, thanks.
>
> 78da5cca6ece brcmfmac: Fix incorrect type assignments for keep-alive

Actually the fixes tag was incorrect, I fixed it but the commit id changed:

3db30b790289 brcmfmac: Fix incorrect type assignments for keep-alive

The error was:

In commit

78da5cca6ece ("brcmfmac: Fix incorrect type assignments for keep-alive")

Fixes tag

Fixes: 7a6cfe28ae3e ("brcmfmac: Configure keep-alive packet on suspend")

has these problem(s):

- Subject does not match target commit subject
Just use
git log -1 --format='Fixes: %h ("%s")'

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches