* Re: [PATCH 3/7] brcmsmac: switch source files to using SPDX license identifier
From: Kalle Valo @ 2019-05-23 9:16 UTC (permalink / raw)
To: Arend van Spriel
Cc: linux-wireless, Arend van Spriel, Thomas Gleixner,
Greg Kroah-Hartman
In-Reply-To: <1558008251-13692-4-git-send-email-arend.vanspriel@broadcom.com>
Arend van Spriel <arend.vanspriel@broadcom.com> wrote:
> With ISC license text in place under the LICENSES folder switch
> to using the SPDX license identifier to refer to the ISC license.
>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
> Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
> Reviewed-by: Franky Lin <franky.lin@broadcom.com>
> Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Dropped per discussion
Patch set to Changes Requested.
--
https://patchwork.kernel.org/patch/10946471/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH 1/7] brcm80211: switch common header files to using SPDX license identifier
From: Kalle Valo @ 2019-05-23 9:17 UTC (permalink / raw)
To: Arend Van Spriel; +Cc: linux-wireless
In-Reply-To: <f4d582e8-3d65-959f-78bd-88b7c47131a5@broadcom.com>
Arend Van Spriel <arend.vanspriel@broadcom.com> writes:
> On 5/20/2019 6:54 PM, Kalle Valo wrote:
>> Arend Van Spriel <arend.vanspriel@broadcom.com> writes:
>>
>>> On 5/16/2019 10:57 PM, Arend Van Spriel wrote:
>>>> On 5/16/2019 2:04 PM, Arend van Spriel wrote:
>>>>> With ISC license text in place under the LICENSES folder switch
>>>>> to using the SPDX license identifier to refer to the ISC license.
>>>>
>>>> Hi Kalle,
>>>>
>>>> Given the feedback on checkpatch (or spdxcheck) failures let me
>>>> respin this series.
>>>
>>> Actually let's *NOT* respin and leave this series as is and ignore the
>>> warning for the header files as Thomas wrote in his response: " So we
>>> can fixup the documentation and allow // style for headers as well.".
>>
>> What about patch 3, should I drop that patch? Wasn't the conclusion that
>> you need separately change led.c?
>
> Okay. Let's do that.
Dropped it now.
> Or do you want me to resend the whole series without patch 3?
If the series applies without patch 3 (and I assume it does) no need to
resend the whole series.
--
Kalle Valo
^ permalink raw reply
* [PATCH] iw: add HE support to station dump call
From: John Crispin @ 2019-05-23 10:50 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, John Crispin, Shashidhar Lakkavalli
Signed-off-by: Shashidhar Lakkavalli <slakkavalli@datto.com>
Signed-off-by: John Crispin <john@phrozen.org>
---
station.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/station.c b/station.c
index 0454f87..aaad079 100644
--- a/station.c
+++ b/station.c
@@ -243,6 +243,18 @@ void parse_bitrate(struct nlattr *bitrate_attr, char *buf, int buflen)
if (rinfo[NL80211_RATE_INFO_VHT_NSS])
pos += snprintf(pos, buflen - (pos - buf),
" VHT-NSS %d", nla_get_u8(rinfo[NL80211_RATE_INFO_VHT_NSS]));
+ if (rinfo[NL80211_RATE_INFO_HE_MCS])
+ pos += snprintf(pos, buflen - (pos - buf),
+ " HE-MCS %d", nla_get_u8(rinfo[NL80211_RATE_INFO_HE_MCS]));
+ if (rinfo[NL80211_RATE_INFO_HE_NSS])
+ pos += snprintf(pos, buflen - (pos - buf),
+ " HE-NSS %d", nla_get_u8(rinfo[NL80211_RATE_INFO_HE_NSS]));
+ if (rinfo[NL80211_RATE_INFO_HE_GI])
+ pos += snprintf(pos, buflen - (pos - buf),
+ " HE-GI %d", nla_get_u8(rinfo[NL80211_RATE_INFO_HE_GI]));
+ if (rinfo[NL80211_RATE_INFO_HE_DCM])
+ pos += snprintf(pos, buflen - (pos - buf),
+ " HE-DCM %d", nla_get_u8(rinfo[NL80211_RATE_INFO_HE_DCM]));
}
static char *get_chain_signal(struct nlattr *attr_list)
--
2.20.1
^ permalink raw reply related
* Re: [PATCH 1/7] brcm80211: switch common header files to using SPDX license identifier
From: Arend Van Spriel @ 2019-05-23 10:53 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless
In-Reply-To: <871s0pedih.fsf@kamboji.qca.qualcomm.com>
On May 23, 2019 11:17:22 AM Kalle Valo <kvalo@codeaurora.org> wrote:
> Arend Van Spriel <arend.vanspriel@broadcom.com> writes:
>> Or do you want me to resend the whole series without patch 3?
>
> If the series applies without patch 3 (and I assume it does) no need to
> resend the whole series.
Thanks, Kalle
I assume the same.
Regards,
Arend
^ permalink raw reply
* Re: [PATCH 00/16] wilc1000: move out of staging
From: Ajay.Kathat @ 2019-05-23 13:32 UTC (permalink / raw)
To: kvalo
Cc: linux-wireless, johannes, gregkh, Adham.Abozaeid,
Venkateswara.Kaja, Nicolas.Ferre, Claudiu.Beznea
In-Reply-To: <871s30pevc.fsf@codeaurora.org>
Hi Kalle,
On 3/21/2019 6:24 PM, Kalle Valo wrote:
>
> <Ajay.Kathat@microchip.com> writes:
>
>> Hi Kalle/Johannes,
>>
>> On 2/9/2019 12:42 PM, Ajay Kathat - I15481 wrote:
>>> From: Ajay Singh <ajay.kathat@microchip.com>
>>>
>>> This patch series is to review and move wilc1000 driver out of staging.
>>> Implemented the initial received review comments[1] and submitting the
>>> driver again. During this cleanup deleted around 3.3k lines of code.
>>>
>>> Below are the major items fixed in recent cleanup:
>>> - remove use of shadow buffer to keep scan result.
>>> - remove internal messaging flow to handle cfg80211_ops.
>>> - make use of cfg80211 provided API.
>>> - use structure for packing firmware commands.
>>> - make use of kernel provided API and macros.
>>> - remove unnecessary logs messages.
>>>
>>> Pending action item:
>>> - dynamically add/remove p2p interface.
>>>
>>> This item will take some time, we are planning to take it up after
>>> mainline.
>>>
>>> We hope it can be move out staging in v5.1.
>>> Please review and confirm if it is good to move out.
>>
>> Do you have any update for this series. Please provide your inputs for
>> next step.
>
> rtw88 is taking priority in the new drivers "queue" so I doubt I can
> take a look at this in the next few weeks.
>
Is there any update for wilc1000 driver review. Please let me know your
inputs.
Regards,
Ajay
^ permalink raw reply
* Re: [PATCH 00/16] wilc1000: move out of staging
From: Kalle Valo @ 2019-05-23 14:13 UTC (permalink / raw)
To: Ajay.Kathat
Cc: linux-wireless, johannes, gregkh, Adham.Abozaeid,
Venkateswara.Kaja, Nicolas.Ferre, Claudiu.Beznea
In-Reply-To: <2ba91fc2-af3a-b235-5cb0-2ba06444ea30@microchip.com>
<Ajay.Kathat@microchip.com> writes:
> Hi Kalle,
>
> On 3/21/2019 6:24 PM, Kalle Valo wrote:
>>
>> <Ajay.Kathat@microchip.com> writes:
>>
>>> Hi Kalle/Johannes,
>>>
>>> On 2/9/2019 12:42 PM, Ajay Kathat - I15481 wrote:
>>>> From: Ajay Singh <ajay.kathat@microchip.com>
>>>>
>>>> This patch series is to review and move wilc1000 driver out of staging.
>>>> Implemented the initial received review comments[1] and submitting the
>>>> driver again. During this cleanup deleted around 3.3k lines of code.
>>>>
>>>> Below are the major items fixed in recent cleanup:
>>>> - remove use of shadow buffer to keep scan result.
>>>> - remove internal messaging flow to handle cfg80211_ops.
>>>> - make use of cfg80211 provided API.
>>>> - use structure for packing firmware commands.
>>>> - make use of kernel provided API and macros.
>>>> - remove unnecessary logs messages.
>>>>
>>>> Pending action item:
>>>> - dynamically add/remove p2p interface.
>>>>
>>>> This item will take some time, we are planning to take it up after
>>>> mainline.
>>>>
>>>> We hope it can be move out staging in v5.1.
>>>> Please review and confirm if it is good to move out.
>>>
>>> Do you have any update for this series. Please provide your inputs for
>>> next step.
>>
>> rtw88 is taking priority in the new drivers "queue" so I doubt I can
>> take a look at this in the next few weeks.
>>
>
> Is there any update for wilc1000 driver review. Please let me know your
> inputs.
So is the driver in good shape now? I really do not want to use a lot of
time reviewing it just to find out that there's a lot of work to do. For
example, has someone else reviewed it?
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH v2] sdio: Fix a memory leaking bug in wl1271_probe()
From: Kalle Valo @ 2019-05-23 15:00 UTC (permalink / raw)
To: Gen Zhang; +Cc: eyalreizer, netdev, linux-kernel, linux-wireless
In-Reply-To: <20190523144425.GA26766@zhanggen-UX430UQ>
+ linux-wireless
Gen Zhang <blackgod016574@gmail.com> writes:
> In wl1271_probe(), 'glue->core' is allocated by platform_device_alloc(),
> when this allocation fails, ENOMEM is returned. However, 'pdev_data'
> and 'glue' are allocated by devm_kzalloc() before 'glue->core'. When
> platform_device_alloc() returns NULL, we should also free 'pdev_data'
> and 'glue' before wl1271_probe() ends to prevent leaking memory.
>
> Similarly, we should free 'pdev_data' when 'glue' is NULL. And we
> should free 'pdev_data' and 'glue' when 'ret' is error.
>
> Further, we shoulf free 'glue->dev', 'pdev_data' and 'glue' when this
> function normally ends to prevent memory leaking.
>
> Signed-off-by: Gen Zhang <blackgod016574@gmail.com>
> ---
> diff --git a/drivers/net/wireless/ti/wlcore/sdio.c b/drivers/net/wireless/ti/wlcore/sdio.c
> index 4d4b0770..232ce5f 100644
> --- a/drivers/net/wireless/ti/wlcore/sdio.c
> +++ b/drivers/net/wireless/ti/wlcore/sdio.c
You need to CC linux-wireless, otherwise patchwork won't see the patch
and it will not be applied. Also you should use prefix "wlcore:".
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#commit_title_is_wrong
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH] iw: print HE capabilities
From: Sven Eckelmann @ 2019-05-23 15:22 UTC (permalink / raw)
To: John Crispin; +Cc: Johannes Berg, linux-wireless, Shashidhar Lakkavalli
In-Reply-To: <20190520105416.27185-1-john@phrozen.org>
[-- Attachment #1: Type: text/plain, Size: 568 bytes --]
On Monday, 20 May 2019 12:54:16 CEST John Crispin wrote:
> + char *iftypes[NUM_NL80211_IFTYPES] = {
> + "Unspec", "Adhoc", "Station", "AP", "AP/VLAN", "WDS", "Monitor"
> + "Mesh", "P2P/Client", "P2P/Go", "P2P/Device", "OCB", "NAN",
> + };
Noticed during tests with hwsim that this list is slightly incorrect - because
the "," is missing after "Monitor". Thus you will now have an entry
"MonitorMesh" instead of two entries "Monitor" + "Mesh". So everything after
NL80211_IFTYPE_MESH_POINT is off by one.
Kind regards,
Sven
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH v2 5.2] rsi: Properly initialize data in rsi_sdio_ta_reset
From: Nathan Chancellor @ 2019-05-23 15:30 UTC (permalink / raw)
To: Amitkumar Karwar, Siva Rebbagondla, Kalle Valo
Cc: linux-wireless, netdev, linux-kernel, Arnd Bergmann,
Nick Desaulniers, clang-built-linux, Nathan Chancellor
In-Reply-To: <20190502151548.11143-1-natechancellor@gmail.com>
When building with -Wuninitialized, Clang warns:
drivers/net/wireless/rsi/rsi_91x_sdio.c:940:43: warning: variable 'data'
is uninitialized when used here [-Wuninitialized]
put_unaligned_le32(TA_HOLD_THREAD_VALUE, data);
^~~~
drivers/net/wireless/rsi/rsi_91x_sdio.c:930:10: note: initialize the
variable 'data' to silence this warning
u8 *data;
^
= NULL
1 warning generated.
Using Clang's suggestion of initializing data to NULL wouldn't work out
because data will be dereferenced by put_unaligned_le32. Use kzalloc to
properly initialize data, which matches a couple of other places in this
driver.
Fixes: e5a1ecc97e5f ("rsi: add firmware loading for 9116 device")
Link: https://github.com/ClangBuiltLinux/linux/issues/464
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
v1 -> v2:
* Use RSI_9116_REG_SIZE instead of sizeof(u32) for kzalloc thanks to
review from Arnd.
drivers/net/wireless/rsi/rsi_91x_sdio.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/net/wireless/rsi/rsi_91x_sdio.c b/drivers/net/wireless/rsi/rsi_91x_sdio.c
index f9c67ed473d1..b42cd50b837e 100644
--- a/drivers/net/wireless/rsi/rsi_91x_sdio.c
+++ b/drivers/net/wireless/rsi/rsi_91x_sdio.c
@@ -929,11 +929,15 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
u32 addr;
u8 *data;
+ data = kzalloc(RSI_9116_REG_SIZE, GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
status = rsi_sdio_master_access_msword(adapter, TA_BASE_ADDR);
if (status < 0) {
rsi_dbg(ERR_ZONE,
"Unable to set ms word to common reg\n");
- return status;
+ goto err;
}
rsi_dbg(INIT_ZONE, "%s: Bring TA out of reset\n", __func__);
@@ -944,7 +948,7 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
RSI_9116_REG_SIZE);
if (status < 0) {
rsi_dbg(ERR_ZONE, "Unable to hold TA threads\n");
- return status;
+ goto err;
}
put_unaligned_le32(TA_SOFT_RST_CLR, data);
@@ -954,7 +958,7 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
RSI_9116_REG_SIZE);
if (status < 0) {
rsi_dbg(ERR_ZONE, "Unable to get TA out of reset\n");
- return status;
+ goto err;
}
put_unaligned_le32(TA_PC_ZERO, data);
@@ -964,7 +968,8 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
RSI_9116_REG_SIZE);
if (status < 0) {
rsi_dbg(ERR_ZONE, "Unable to Reset TA PC value\n");
- return -EINVAL;
+ status = -EINVAL;
+ goto err;
}
put_unaligned_le32(TA_RELEASE_THREAD_VALUE, data);
@@ -974,17 +979,19 @@ static int rsi_sdio_ta_reset(struct rsi_hw *adapter)
RSI_9116_REG_SIZE);
if (status < 0) {
rsi_dbg(ERR_ZONE, "Unable to release TA threads\n");
- return status;
+ goto err;
}
status = rsi_sdio_master_access_msword(adapter, MISC_CFG_BASE_ADDR);
if (status < 0) {
rsi_dbg(ERR_ZONE, "Unable to set ms word to common reg\n");
- return status;
+ goto err;
}
rsi_dbg(INIT_ZONE, "***** TA Reset done *****\n");
- return 0;
+err:
+ kfree(data);
+ return status;
}
static struct rsi_host_intf_ops sdio_host_intf_ops = {
--
2.22.0.rc1
^ permalink raw reply related
* Re: [PATCH] ath10k: add missing error handling
From: Brian Norris @ 2019-05-23 16:42 UTC (permalink / raw)
To: Claire Chang; +Cc: Kalle Valo, ath10k, linux-wireless, Wen Gong
In-Reply-To: <20190523071534.254611-1-tientzu@chromium.org>
On Thu, May 23, 2019 at 12:15 AM Claire Chang <tientzu@chromium.org> wrote:
> --- a/drivers/net/wireless/ath/ath10k/sdio.c
> +++ b/drivers/net/wireless/ath/ath10k/sdio.c
> @@ -607,6 +607,10 @@ static int ath10k_sdio_mbox_rx_alloc(struct ath10k *ar,
> full_len,
> last_in_bundle,
> last_in_bundle);
> + if (ret) {
IIUC, you have basically the same failure case a few lines up, where
ath10k_sdio_mbox_alloc_pkt_bundle() may fail. Do the same there?
This (including the error label to which it's jumping) looks fine to me though:
Reviewed-by: Brian Norris <briannorris@chromium.org>
> + ath10k_warn(ar, "alloc_rx_pkt error %d\n", ret);
> + goto err;
> + }
> }
>
> ar_sdio->n_rx_pkts = i;
^ permalink raw reply
* Re: [PATCH 00/16] wilc1000: move out of staging
From: Ajay.Kathat @ 2019-05-23 16:45 UTC (permalink / raw)
To: kvalo
Cc: linux-wireless, johannes, gregkh, Adham.Abozaeid,
Venkateswara.Kaja, Nicolas.Ferre, Claudiu.Beznea
In-Reply-To: <87sgt5cl8q.fsf@kamboji.qca.qualcomm.com>
Hi Kalle,
On 5/23/2019 7:43 PM, Kalle Valo wrote:
> <Ajay.Kathat@microchip.com> writes:
>
>> Hi Kalle,
>>
>> On 3/21/2019 6:24 PM, Kalle Valo wrote:
>>>
>>> <Ajay.Kathat@microchip.com> writes:
>>>
>>>> Hi Kalle/Johannes,
>>>>
>>>> On 2/9/2019 12:42 PM, Ajay Kathat - I15481 wrote:
>>>>> From: Ajay Singh <ajay.kathat@microchip.com>
>>>>>
>>>>> This patch series is to review and move wilc1000 driver out of staging.
>>>>> Implemented the initial received review comments[1] and submitting the
>>>>> driver again. During this cleanup deleted around 3.3k lines of code.
>>>>>
>>>>> Below are the major items fixed in recent cleanup:
>>>>> - remove use of shadow buffer to keep scan result.
>>>>> - remove internal messaging flow to handle cfg80211_ops.
>>>>> - make use of cfg80211 provided API.
>>>>> - use structure for packing firmware commands.
>>>>> - make use of kernel provided API and macros.
>>>>> - remove unnecessary logs messages.
>>>>>
>>>>> Pending action item:
>>>>> - dynamically add/remove p2p interface.
>>>>>
>>>>> This item will take some time, we are planning to take it up after
>>>>> mainline.
>>>>>
>>>>> We hope it can be move out staging in v5.1.
>>>>> Please review and confirm if it is good to move out.
>>>>
>>>> Do you have any update for this series. Please provide your inputs for
>>>> next step.
>>>
>>> rtw88 is taking priority in the new drivers "queue" so I doubt I can
>>> take a look at this in the next few weeks.
>>>
>>
>> Is there any update for wilc1000 driver review. Please let me know your
>> inputs.
>
> So is the driver in good shape now? I really do not want to use a lot of
> time reviewing it just to find out that there's a lot of work to do. For
> example, has someone else reviewed it?
>
The first series for complete driver review was submitted earlier [1].
It was reviewed by Johannes and he looked into driver integration with
the cfg80211 stack part. We worked on the review comments and submitted
the changes to staging.
We need further review to identify if there is any blocker to move to
mainline.
[1]. https://www.spinics.net/lists/linux-wireless/msg177878.html
Regards,
Ajay
^ permalink raw reply
* Re: [PATCH v2 2/2] qtnfmac: add support for Topaz chipsets
From: Igor Mitsyanko @ 2019-05-23 19:26 UTC (permalink / raw)
To: Jonas Gorski, Sergey Matyukevich
Cc: linux-wireless@vger.kernel.org, Andrey Shevchenko
In-Reply-To: <CAOiHx=nBWr4GNh61WV+SAY-++Z6es-HX3_pd70DB_N33bVK1tw@mail.gmail.com>
>
> A bit late of a review/question, but how does one obtain one of these
> files? There's nothing in linux-firmware, and I see only one aborted
> attempt for adding fmac_qsr10g.img from 2016, but none for the others.
> Searching for these filenames also didn't reveal any external
> locations.
>
>
> Regards
> Jonas
>
Hi Jonas, we're working towards a second attempt to get those accepted
to linux-firmware (fmac_qsr1000.img binary first) admittedly it takes us
a long time to do that. The main obstacle for us as developers is that
the binary contains 3-d party GPL code so we have to work with other
departments to satisfy all submission requirements (provide sources,
proper licensing/attribution etc). From a failed fmac_qsr10g.img attempt
it was clear that simply providing an contact email is not enough.
We're planning a second attempt to submit firmware binary patch
relatively soon, for now we expect device will boot from flash.
^ permalink raw reply
* Re: [PATCH] ath10k: add missing error handling
From: Brian Norris @ 2019-05-23 23:18 UTC (permalink / raw)
To: Claire Chang; +Cc: Kalle Valo, ath10k, linux-wireless, Wen Gong, Erik Stromdahl
In-Reply-To: <CA+ASDXMaKpMWnLnKxeft-8eKfpM6qGDsmEzvh290JCCjeRRtxQ@mail.gmail.com>
On Thu, May 23, 2019 at 9:42 AM Brian Norris <briannorris@chromium.org> wrote:
> IIUC, you have basically the same failure case a few lines up, where
> ath10k_sdio_mbox_alloc_pkt_bundle() may fail. Do the same there?
Oh, I see Erik Stromdahl already got that one:
ath10k: sdio: add missing error check
https://patchwork.kernel.org/patch/10906009/
^ permalink raw reply
* [PATCH v2] wlcore: sdio: Fix a memory leaking bug in wl1271_probe()
From: Gen Zhang @ 2019-05-24 2:43 UTC (permalink / raw)
To: Kalle Valo; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <87d0k9b4hm.fsf@kamboji.qca.qualcomm.com>
In wl1271_probe(), 'glue->core' is allocated by platform_device_alloc(),
when this allocation fails, ENOMEM is returned. However, 'pdev_data'
and 'glue' are allocated by devm_kzalloc() before 'glue->core'. When
platform_device_alloc() returns NULL, we should also free 'pdev_data'
and 'glue' before wl1271_probe() ends to prevent leaking memory.
Similarly, we should free 'pdev_data' when 'glue' is NULL. And we
should free 'pdev_data' and 'glue' when 'ret' is error.
Further, we shoulf free 'glue->dev', 'pdev_data' and 'glue' when this
function normally ends to prevent memory leaking.
Signed-off-by: Gen Zhang <blackgod016574@gmail.com>
---
diff --git a/drivers/net/wireless/ti/wlcore/sdio.c b/drivers/net/wireless/ti/wlcore/sdio.c
index 4d4b0770..9110891 100644
--- a/drivers/net/wireless/ti/wlcore/sdio.c
+++ b/drivers/net/wireless/ti/wlcore/sdio.c
@@ -298,8 +298,10 @@ static int wl1271_probe(struct sdio_func *func,
pdev_data->if_ops = &sdio_ops;
glue = devm_kzalloc(&func->dev, sizeof(*glue), GFP_KERNEL);
- if (!glue)
- return -ENOMEM;
+ if (!glue) {
+ ret = -ENOMEM;
+ goto out_free1;
+ }
glue->dev = &func->dev;
@@ -311,7 +313,7 @@ static int wl1271_probe(struct sdio_func *func,
ret = wlcore_probe_of(&func->dev, &irq, &wakeirq, pdev_data);
if (ret)
- goto out;
+ goto out_free2;
/* if sdio can keep power while host is suspended, enable wow */
mmcflags = sdio_get_host_pm_caps(func);
@@ -340,7 +342,7 @@ static int wl1271_probe(struct sdio_func *func,
if (!glue->core) {
dev_err(glue->dev, "can't allocate platform_device");
ret = -ENOMEM;
- goto out;
+ goto out_free2;
}
glue->core->dev.parent = &func->dev;
@@ -380,12 +382,17 @@ static int wl1271_probe(struct sdio_func *func,
dev_err(glue->dev, "can't add platform device\n");
goto out_dev_put;
}
- return 0;
+ ret = 0;
out_dev_put:
platform_device_put(glue->core);
-out:
+out_free2:
+ devm_kfree(&func->dev, glue);
+
+out_free1:
+ devm_kfree(&func->dev, pdev_data);
+
return ret;
}
---
^ permalink raw reply related
* [PATCH] wlcore: spi: Fix a memory leaking bug in wl1271_probe()
From: Gen Zhang @ 2019-05-24 3:02 UTC (permalink / raw)
To: kvalo, davem; +Cc: linux-wireless, netdev, linux-kernel
In wl1271_probe(), 'glue->core' is allocated by platform_device_alloc(),
when this allocation fails, ENOMEM is returned. However, 'pdev_data'
and 'glue' are allocated by devm_kzalloc() before 'glue->core'. When
platform_device_alloc() returns NULL, we should also free 'pdev_data'
and 'glue' before wl1271_probe() ends to prevent leaking memory.
Similarly, we shoulf free 'pdev_data' when 'glue' is NULL. And we should
free 'pdev_data' and 'glue' when 'glue->reg' is error and when 'ret' is
error.
Further, we should free 'glue->core', 'pdev_data' and 'glue' when this
function normally ends to prevent leaking memory.
Signed-off-by: Gen Zhang <blackgod016574@gmail.com>
---
diff --git a/drivers/net/wireless/ti/wlcore/spi.c b/drivers/net/wireless/ti/wlcore/spi.c
index 62ce54a..ea0ec26 100644
--- a/drivers/net/wireless/ti/wlcore/spi.c
+++ b/drivers/net/wireless/ti/wlcore/spi.c
@@ -480,7 +480,7 @@ static int wl1271_probe(struct spi_device *spi)
struct wl12xx_spi_glue *glue;
struct wlcore_platdev_data *pdev_data;
struct resource res[1];
- int ret;
+ int ret = -ENOMEM;
pdev_data = devm_kzalloc(&spi->dev, sizeof(*pdev_data), GFP_KERNEL);
if (!pdev_data)
@@ -491,7 +491,8 @@ static int wl1271_probe(struct spi_device *spi)
glue = devm_kzalloc(&spi->dev, sizeof(*glue), GFP_KERNEL);
if (!glue) {
dev_err(&spi->dev, "can't allocate glue\n");
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto out_free1;
}
glue->dev = &spi->dev;
@@ -503,31 +504,35 @@ static int wl1271_probe(struct spi_device *spi)
spi->bits_per_word = 32;
glue->reg = devm_regulator_get(&spi->dev, "vwlan");
- if (PTR_ERR(glue->reg) == -EPROBE_DEFER)
- return -EPROBE_DEFER;
+ if (PTR_ERR(glue->reg) == -EPROBE_DEFER) {
+ ret = -EPROBE_DEFER;
+ goto out_free2;
+ }
if (IS_ERR(glue->reg)) {
dev_err(glue->dev, "can't get regulator\n");
- return PTR_ERR(glue->reg);
+ ret = PTR_ERR(glue->reg);
+ goto out_free2;
}
ret = wlcore_probe_of(spi, glue, pdev_data);
if (ret) {
dev_err(glue->dev,
"can't get device tree parameters (%d)\n", ret);
- return ret;
+ goto out_free2;
}
ret = spi_setup(spi);
if (ret < 0) {
dev_err(glue->dev, "spi_setup failed\n");
- return ret;
+ goto out_free2;
}
glue->core = platform_device_alloc(pdev_data->family->name,
PLATFORM_DEVID_AUTO);
if (!glue->core) {
dev_err(glue->dev, "can't allocate platform_device\n");
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto out_free2;
}
glue->core->dev.parent = &spi->dev;
@@ -557,10 +562,14 @@ static int wl1271_probe(struct spi_device *spi)
goto out_dev_put;
}
- return 0;
+ ret = 0;
out_dev_put:
platform_device_put(glue->core);
+out_free2:
+ devm_kfree(&func->dev, glue);
+out_free1:
+ devm_kfree(&func->dev, pdev_data);
return ret;
}
---
^ permalink raw reply related
* [PATCH v2] ath10k: fix fw crash by moving chip reset after napi disabled
From: Miaoqing Pan @ 2019-05-24 3:16 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, Miaoqing Pan
On SMP platform, when continuously running wifi up/down, the napi
poll can be scheduled during chip reset, which will call
ath10k_pci_has_fw_crashed() to check the fw status. But in the reset
period, the value from FW_INDICATOR_ADDRESS register will return
0xdeadbeef, which also be treated as fw crash. Fix the issue by
moving chip reset after napi disabled.
ath10k_pci 0000:01:00.0: firmware crashed! (guid 73b30611-5b1e-4bdd-90b4-64c81eb947b6)
ath10k_pci 0000:01:00.0: qca9984/qca9994 hw1.0 target 0x01000000 chip_id 0x00000000 sub 168c:cafe
ath10k_pci 0000:01:00.0: htt-ver 2.2 wmi-op 6 htt-op 4 cal otp max-sta 512 raw 0 hwcrypto 1
ath10k_pci 0000:01:00.0: failed to get memcpy hi address for firmware address 4: -16
ath10k_pci 0000:01:00.0: failed to read firmware dump area: -16
ath10k_pci 0000:01:00.0: Copy Engine register dump:
ath10k_pci 0000:01:00.0: [00]: 0x0004a000 0 0 0 0
ath10k_pci 0000:01:00.0: [01]: 0x0004a400 0 0 0 0
ath10k_pci 0000:01:00.0: [02]: 0x0004a800 0 0 0 0
ath10k_pci 0000:01:00.0: [03]: 0x0004ac00 0 0 0 0
ath10k_pci 0000:01:00.0: [04]: 0x0004b000 0 0 0 0
ath10k_pci 0000:01:00.0: [05]: 0x0004b400 0 0 0 0
ath10k_pci 0000:01:00.0: [06]: 0x0004b800 0 0 0 0
ath10k_pci 0000:01:00.0: [07]: 0x0004bc00 1 0 1 0
ath10k_pci 0000:01:00.0: [08]: 0x0004c000 0 0 0 0
ath10k_pci 0000:01:00.0: [09]: 0x0004c400 0 0 0 0
ath10k_pci 0000:01:00.0: [10]: 0x0004c800 0 0 0 0
ath10k_pci 0000:01:00.0: [11]: 0x0004cc00 0 0 0 0
Tested HW: QCA9984,QCA9887,WCN3990
Signed-off-by: Miaoqing Pan <miaoqing@codeaurora.org>
---
Changes since v1:
- update commit message
---
drivers/net/wireless/ath/ath10k/pci.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 2bd6cba..80bcb2e 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -2059,6 +2059,11 @@ static void ath10k_pci_hif_stop(struct ath10k *ar)
ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot hif stop\n");
+ ath10k_pci_irq_disable(ar);
+ ath10k_pci_irq_sync(ar);
+ napi_synchronize(&ar->napi);
+ napi_disable(&ar->napi);
+
/* Most likely the device has HTT Rx ring configured. The only way to
* prevent the device from accessing (and possible corrupting) host
* memory is to reset the chip now.
@@ -2072,10 +2077,6 @@ static void ath10k_pci_hif_stop(struct ath10k *ar)
*/
ath10k_pci_safe_chip_reset(ar);
- ath10k_pci_irq_disable(ar);
- ath10k_pci_irq_sync(ar);
- napi_synchronize(&ar->napi);
- napi_disable(&ar->napi);
ath10k_pci_flush(ar);
spin_lock_irqsave(&ar_pci->ps_lock, flags);
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] mac80211: mesh: fix RCU warning
From: Johannes Berg @ 2019-05-24 8:29 UTC (permalink / raw)
To: Thomas Pedersen, linux-wireless; +Cc: peter.oh
In-Reply-To: <1557958906-1432-1-git-send-email-thomas@eero.com>
On Wed, 2019-05-15 at 15:21 -0700, Thomas Pedersen wrote:
> ifmsh->csa was being dereferenced without the RCU read
> lock held.
> +++ b/net/mac80211/mesh.c
> @@ -1220,10 +1220,12 @@ int ieee80211_mesh_finish_csa(struct ieee80211_sub_if_data *sdata)
> ifmsh->chsw_ttl = 0;
>
> /* Remove the CSA and MCSP elements from the beacon */
> + rcu_read_lock();
> tmp_csa_settings = rcu_dereference(ifmsh->csa);
> RCU_INIT_POINTER(ifmsh->csa, NULL);
> if (tmp_csa_settings)
> kfree_rcu(tmp_csa_settings, rcu_head);
> + rcu_read_unlock();
This seems wrong to me.
Really this code is the *writer* side, so you should do something like
this:
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 766e5e5bab8a..d578147ad7e8 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -1220,7 +1220,8 @@ int ieee80211_mesh_finish_csa(struct
ieee80211_sub_if_data *sdata)
ifmsh->chsw_ttl = 0;
/* Remove the CSA and MCSP elements from the beacon */
- tmp_csa_settings = rcu_dereference(ifmsh->csa);
+ tmp_csa_settings = rcu_dereference_protected(ifmsh->csa,
+ lockdep_is_held(&sdata->wdev.mtx));
RCU_INIT_POINTER(ifmsh->csa, NULL);
if (tmp_csa_settings)
kfree_rcu(tmp_csa_settings, rcu_head);
@@ -1242,6 +1243,8 @@ int ieee80211_mesh_csa_beacon(struct
ieee80211_sub_if_data *sdata,
struct mesh_csa_settings *tmp_csa_settings;
int ret = 0;
+ lockdep_assert_held(&sdata->wdev.mtx);
+
tmp_csa_settings = kmalloc(sizeof(*tmp_csa_settings),
GFP_ATOMIC);
if (!tmp_csa_settings)
Can you test that and send a proper patch?
johannes
^ permalink raw reply related
* Re: [PATCH v2] wlcore: sdio: Fix a memory leaking bug in wl1271_probe()
From: Jon Hunter @ 2019-05-24 9:28 UTC (permalink / raw)
To: Gen Zhang, Kalle Valo; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20190524024307.GA5639@zhanggen-UX430UQ>
On 24/05/2019 03:43, Gen Zhang wrote:
> In wl1271_probe(), 'glue->core' is allocated by platform_device_alloc(),
> when this allocation fails, ENOMEM is returned. However, 'pdev_data'
> and 'glue' are allocated by devm_kzalloc() before 'glue->core'. When
> platform_device_alloc() returns NULL, we should also free 'pdev_data'
> and 'glue' before wl1271_probe() ends to prevent leaking memory.
No, devm_kzalloc() automatically frees memory on failure.
> Similarly, we should free 'pdev_data' when 'glue' is NULL. And we
> should free 'pdev_data' and 'glue' when 'ret' is error.
>
> Further, we shoulf free 'glue->dev', 'pdev_data' and 'glue' when this
> function normally ends to prevent memory leaking.
Why?
> Signed-off-by: Gen Zhang <blackgod016574@gmail.com>
> ---
> diff --git a/drivers/net/wireless/ti/wlcore/sdio.c b/drivers/net/wireless/ti/wlcore/sdio.c
> index 4d4b0770..9110891 100644
> --- a/drivers/net/wireless/ti/wlcore/sdio.c
> +++ b/drivers/net/wireless/ti/wlcore/sdio.c
> @@ -298,8 +298,10 @@ static int wl1271_probe(struct sdio_func *func,
> pdev_data->if_ops = &sdio_ops;
>
> glue = devm_kzalloc(&func->dev, sizeof(*glue), GFP_KERNEL);
> - if (!glue)
> - return -ENOMEM;
> + if (!glue) {
> + ret = -ENOMEM;
> + goto out_free1;
> + }
>
> glue->dev = &func->dev;
>
> @@ -311,7 +313,7 @@ static int wl1271_probe(struct sdio_func *func,
>
> ret = wlcore_probe_of(&func->dev, &irq, &wakeirq, pdev_data);
> if (ret)
> - goto out;
> + goto out_free2;
>
> /* if sdio can keep power while host is suspended, enable wow */
> mmcflags = sdio_get_host_pm_caps(func);
> @@ -340,7 +342,7 @@ static int wl1271_probe(struct sdio_func *func,
> if (!glue->core) {
> dev_err(glue->dev, "can't allocate platform_device");
> ret = -ENOMEM;
> - goto out;
> + goto out_free2;
> }
>
> glue->core->dev.parent = &func->dev;
> @@ -380,12 +382,17 @@ static int wl1271_probe(struct sdio_func *func,
> dev_err(glue->dev, "can't add platform device\n");
> goto out_dev_put;
> }
> - return 0;
> + ret = 0;
This is completely wrong.
> out_dev_put:
> platform_device_put(glue->core);
>
> -out:
> +out_free2:
> + devm_kfree(&func->dev, glue);
> +
> +out_free1:
> + devm_kfree(&func->dev, pdev_data);
> +
> return ret;
> }
>
> ---
Jon
--
nvpublic
^ permalink raw reply
* Re: [RFC 0/8] nl80211: add 6GHz band support
From: Johannes Berg @ 2019-05-24 11:56 UTC (permalink / raw)
To: Arend van Spriel; +Cc: linux-wireless
In-Reply-To: <1558353645-18119-1-git-send-email-arend.vanspriel@broadcom.com>
Hi Arend,
On Mon, 2019-05-20 at 14:00 +0200, Arend van Spriel wrote:
> In 802.11ax D4.0 a new band has been proposed. This series contains
> changes to cfg80211 for supporting this band. With 2GHz and 5GHz there
> was no overlap in channel number. However, this new band has channel
> numbers with a range from 1 up to 253.
At the wireless workshop in Prague, we looked at this and sort of
decided that it'd be better to put all the 6 GHz channels into the 5 GHz
"band" in nl80211, to avoid all the "5 || 6" since they're really the
same except for very specific places like scanning.
The channel numbers problem came up, of course, but for nl80211 it's not
that relevant since we deal with frequencies only, and we thought inside
the kernel it'd be better to disambiguate them with operating classes,
where needed - only few places really deal with channel numbers to start
with.
Do you have any reason to think that it's better as a separate band enum
(which I notice you put before 60 GHz thus breaking the API/ABI :P)?
Thanks,
johannes
^ permalink raw reply
* Re: [RFC 0/8] nl80211: add 6GHz band support
From: Arend Van Spriel @ 2019-05-24 18:38 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless
In-Reply-To: <df53f969297fea1f3c8101cd2c1571a957985d2a.camel@sipsolutions.net>
On May 24, 2019 1:56:43 PM Johannes Berg <johannes@sipsolutions.net> wrote:
> Hi Arend,
>
> On Mon, 2019-05-20 at 14:00 +0200, Arend van Spriel wrote:
>> In 802.11ax D4.0 a new band has been proposed. This series contains
>> changes to cfg80211 for supporting this band. With 2GHz and 5GHz there
>> was no overlap in channel number. However, this new band has channel
>> numbers with a range from 1 up to 253.
>
> At the wireless workshop in Prague, we looked at this and sort of
> decided that it'd be better to put all the 6 GHz channels into the 5 GHz
> "band" in nl80211, to avoid all the "5 || 6" since they're really the
> same except for very specific places like scanning.
Would have liked to be there, but attending is no longer an option for me.
We now have two autistic, non-verbal children and I am the primary
caregiver for the oldest because my wife can't handle him. Guess I should
have checked the workshop notes before working on this :-) Do you have URL?
Agree that most functional requirements for 6 GHz are same as 5 GHz. There
are some 6 GHz specifics about beaconing as well.
> The channel numbers problem came up, of course, but for nl80211 it's not
> that relevant since we deal with frequencies only, and we thought inside
> the kernel it'd be better to disambiguate them with operating classes,
> where needed - only few places really deal with channel numbers to start
> with.
>
> Do you have any reason to think that it's better as a separate band enum
No specific reason. Just that the few cfg80211-based drivers tend to use
channel number as hwvalue.
> (which I notice you put before 60 GHz thus breaking the API/ABI :P)?
Right. Now I feel wet behind the ears :-p
I will go with 6G being additional 5G range and see how that works for us.
Gr. AvS
^ permalink raw reply
* Re: [PATCH] network: wireless: p54u: Fix race between disconnect and firmware loading
From: Christian Lamparter @ 2019-05-24 21:19 UTC (permalink / raw)
To: Alan Stern
Cc: syzbot, kvalo, davem, andreyknvl, syzkaller-bugs,
Kernel development list, USB list, linux-wireless, netdev
In-Reply-To: <Pine.LNX.4.44L0.1905201042110.1498-100000@iolanthe.rowland.org>
On Monday, May 20, 2019 4:44:21 PM CEST Alan Stern wrote:
> The syzbot fuzzer found a bug in the p54 USB wireless driver. The
> issue involves a race between disconnect and the firmware-loader
> callback routine, and it has several aspects.
>
> One big problem is that when the firmware can't be loaded, the
> callback routine tries to unbind the driver from the USB _device_ (by
> calling device_release_driver) instead of from the USB _interface_ to
> which it is actually bound (by calling usb_driver_release_interface).
>
> The race involves access to the private data structure. The driver's
> disconnect handler waits for a completion that is signalled by the
> firmware-loader callback routine. As soon as the completion is
> signalled, you have to assume that the private data structure may have
> been deallocated by the disconnect handler -- even if the firmware was
> loaded without errors. However, the callback routine does access the
> private data several times after that point.
>
> Another problem is that, in order to ensure that the USB device
> structure hasn't been freed when the callback routine runs, the driver
> takes a reference to it. This isn't good enough any more, because now
> that the callback routine calls usb_driver_release_interface, it has
> to ensure that the interface structure hasn't been freed.
>
> Finally, the driver takes an unnecessary reference to the USB device
> structure in the probe function and drops the reference in the
> disconnect handler. This extra reference doesn't accomplish anything,
> because the USB core already guarantees that a device structure won't
> be deallocated while a driver is still bound to any of its interfaces.
>
> To fix these problems, this patch makes the following changes:
>
> Call usb_driver_release_interface() rather than
> device_release_driver().
>
> Don't signal the completion until after the important
> information has been copied out of the private data structure,
> and don't refer to the private data at all thereafter.
>
> Lock udev (the interface's parent) before unbinding the driver
> instead of locking udev->parent.
>
> During the firmware loading process, take a reference to the
> USB interface instead of the USB device.
>
> Don't take an unnecessary reference to the device during probe
> (and then don't drop it during disconnect).
>
> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> Reported-and-tested-by: syzbot+200d4bb11b23d929335f@syzkaller.appspotmail.com
> CC: <stable@vger.kernel.org>
Finally I'm at home where I have the device. Did some test with replugging
and module unloading, all seems fine. Thanks!
Acked-by: Christian Lamparter <chunkeey@gmail.com>
^ permalink raw reply
* [PATCH] p54: Support boottime in scan results
From: Christian Lamparter @ 2019-05-24 21:33 UTC (permalink / raw)
To: linux-wireless; +Cc: Kalle Valo
This patch fixes an issue with wpa_supplicant dropping all scan
results because their where considered to be "too old" (e.g.:
"skip - scan result not recent enough (121056.086325 seconds too old)")
which is very weird because this looks like that the scan results have
been received before a scan started. This is due to the inaccuracy of
the default timing mechanism for calculating the BSS entry age.
Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
---
drivers/net/wireless/intersil/p54/txrx.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/wireless/intersil/p54/txrx.c b/drivers/net/wireless/intersil/p54/txrx.c
index 5bf1c19ecced..f80c5056ccb7 100644
--- a/drivers/net/wireless/intersil/p54/txrx.c
+++ b/drivers/net/wireless/intersil/p54/txrx.c
@@ -334,6 +334,7 @@ static int p54_rx_data(struct p54_common *priv, struct sk_buff *skb)
u16 freq = le16_to_cpu(hdr->freq);
size_t header_len = sizeof(*hdr);
u32 tsf32;
+ __le16 fc;
u8 rate = hdr->rate & 0xf;
/*
@@ -382,6 +383,11 @@ static int p54_rx_data(struct p54_common *priv, struct sk_buff *skb)
skb_pull(skb, header_len);
skb_trim(skb, le16_to_cpu(hdr->len));
+
+ fc = ((struct ieee80211_hdr *)skb->data)->frame_control;
+ if (ieee80211_is_probe_resp(fc) || ieee80211_is_beacon(fc))
+ rx_status->boottime_ns = ktime_get_boot_ns();
+
if (unlikely(priv->hw->conf.flags & IEEE80211_CONF_PS))
p54_pspoll_workaround(priv, skb);
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] mac80211: mesh: fix RCU warning
From: Thomas Pedersen @ 2019-05-25 3:46 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, Peter Oh
In-Reply-To: <6e162d98ba05a71577c623fe1e8e06a7051eb01c.camel@sipsolutions.net>
On Fri, May 24, 2019 at 1:29 AM Johannes Berg <johannes@sipsolutions.net> wrote:
>
> On Wed, 2019-05-15 at 15:21 -0700, Thomas Pedersen wrote:
> > ifmsh->csa was being dereferenced without the RCU read
> > lock held.
>
> > +++ b/net/mac80211/mesh.c
> > @@ -1220,10 +1220,12 @@ int ieee80211_mesh_finish_csa(struct ieee80211_sub_if_data *sdata)
> > ifmsh->chsw_ttl = 0;
> >
> > /* Remove the CSA and MCSP elements from the beacon */
> > + rcu_read_lock();
> > tmp_csa_settings = rcu_dereference(ifmsh->csa);
> > RCU_INIT_POINTER(ifmsh->csa, NULL);
> > if (tmp_csa_settings)
> > kfree_rcu(tmp_csa_settings, rcu_head);
> > + rcu_read_unlock();
>
> This seems wrong to me.
>
> Really this code is the *writer* side, so you should do something like
> this:
Thanks this looks correct. I should've thought about this a tiny bit more ;)
> diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
> index 766e5e5bab8a..d578147ad7e8 100644
> --- a/net/mac80211/mesh.c
> +++ b/net/mac80211/mesh.c
> @@ -1220,7 +1220,8 @@ int ieee80211_mesh_finish_csa(struct
> ieee80211_sub_if_data *sdata)
> ifmsh->chsw_ttl = 0;
>
> /* Remove the CSA and MCSP elements from the beacon */
> - tmp_csa_settings = rcu_dereference(ifmsh->csa);
> + tmp_csa_settings = rcu_dereference_protected(ifmsh->csa,
> + lockdep_is_held(&sdata->wdev.mtx));
> RCU_INIT_POINTER(ifmsh->csa, NULL);
> if (tmp_csa_settings)
> kfree_rcu(tmp_csa_settings, rcu_head);
> @@ -1242,6 +1243,8 @@ int ieee80211_mesh_csa_beacon(struct
> ieee80211_sub_if_data *sdata,
> struct mesh_csa_settings *tmp_csa_settings;
> int ret = 0;
>
> + lockdep_assert_held(&sdata->wdev.mtx);
> +
> tmp_csa_settings = kmalloc(sizeof(*tmp_csa_settings),
> GFP_ATOMIC);
> if (!tmp_csa_settings)
>
>
> Can you test that and send a proper patch?
>
> johannes
>
--
thomas
^ permalink raw reply
* [PATCH v2] mac80211: mesh: fix RCU warning
From: Thomas Pedersen @ 2019-05-25 4:16 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, Thomas Pedersen
ifmsh->csa is an RCU-protected pointer. The writer context
in ieee80211_mesh_finish_csa() is already mutually
exclusive with wdev->sdata.mtx, but the RCU checker did
not know this. Use rcu_dereference_protected() to avoid a
warning.
fixes the following warning:
[ 12.519089] =============================
[ 12.520042] WARNING: suspicious RCU usage
[ 12.520652] 5.1.0-rc7-wt+ #16 Tainted: G W
[ 12.521409] -----------------------------
[ 12.521972] net/mac80211/mesh.c:1223 suspicious rcu_dereference_check() usage!
[ 12.522928] other info that might help us debug this:
[ 12.523984] rcu_scheduler_active = 2, debug_locks = 1
[ 12.524855] 5 locks held by kworker/u8:2/152:
[ 12.525438] #0: 00000000057be08c ((wq_completion)phy0){+.+.}, at: process_one_work+0x1a2/0x620
[ 12.526607] #1: 0000000059c6b07a ((work_completion)(&sdata->csa_finalize_work)){+.+.}, at: process_one_work+0x1a2/0x620
[ 12.528001] #2: 00000000f184ba7d (&wdev->mtx){+.+.}, at: ieee80211_csa_finalize_work+0x2f/0x90
[ 12.529116] #3: 00000000831a1f54 (&local->mtx){+.+.}, at: ieee80211_csa_finalize_work+0x47/0x90
[ 12.530233] #4: 00000000fd06f988 (&local->chanctx_mtx){+.+.}, at: ieee80211_csa_finalize_work+0x51/0x90
Signed-off-by: Thomas Pedersen <thomas@eero.com>
---
v2: rcu_read_lock() doesn't make sense. Use rcu_dereference_protected() (Johannes)
---
net/mac80211/mesh.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 766e5e5..d5aba50 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -1220,7 +1220,8 @@ int ieee80211_mesh_finish_csa(struct ieee80211_sub_if_data *sdata)
ifmsh->chsw_ttl = 0;
/* Remove the CSA and MCSP elements from the beacon */
- tmp_csa_settings = rcu_dereference(ifmsh->csa);
+ tmp_csa_settings = rcu_dereference_protected(ifmsh->csa,
+ lockdep_is_held(&sdata->wdev.mtx));
RCU_INIT_POINTER(ifmsh->csa, NULL);
if (tmp_csa_settings)
kfree_rcu(tmp_csa_settings, rcu_head);
@@ -1242,6 +1243,8 @@ int ieee80211_mesh_csa_beacon(struct ieee80211_sub_if_data *sdata,
struct mesh_csa_settings *tmp_csa_settings;
int ret = 0;
+ lockdep_assert_held(&sdata->wdev.mtx);
+
tmp_csa_settings = kmalloc(sizeof(*tmp_csa_settings),
GFP_ATOMIC);
if (!tmp_csa_settings)
--
2.7.4
^ permalink raw reply related
* [PATCH] netlink: fix station_info pertid memory leak
From: Andy Strohman @ 2019-05-25 6:27 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless, Andy Strohman
When dumping stations, memory allocated for station_info's
pertid member will leak if the nl80211 header cannot be added to
the sk_buff due to insufficient tail room.
I noticed this leak in the kmalloc-2048 cache.
Fixes: 8689c051a201 ("cfg80211: dynamically allocate per-tid stats for station info")
Signed-off-by: Andy Strohman <andy@uplevelsystems.com>
---
net/wireless/nl80211.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index fffe4b371e23..a82ecfe14d8d 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -4855,8 +4855,10 @@ static int nl80211_send_station(struct sk_buff *msg, u32 cmd, u32 portid,
struct nlattr *sinfoattr, *bss_param;
hdr = nl80211hdr_put(msg, portid, seq, flags, cmd);
- if (!hdr)
+ if (!hdr) {
+ cfg80211_sinfo_release_content(sinfo);
return -1;
+ }
if (nla_put_u32(msg, NL80211_ATTR_IFINDEX, dev->ifindex) ||
nla_put(msg, NL80211_ATTR_MAC, ETH_ALEN, mac_addr) ||
--
2.17.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox