* Re: [PATCH v2] mmc: dw_mmc: Disable SDIO interrupts while suspended to fix suspend/resume
From: Doug Anderson @ 2019-05-28 14:38 UTC (permalink / raw)
To: Ulf Hansson
Cc: Kalle Valo, Heiko Stübner, open list:ARM/Rockchip SoC...,
Guenter Roeck, Brian Norris, linux-wireless, Sonny Rao,
Emil Renner Berthing, Matthias Kaehlcke, Ryan Case, # 4.0+,
Linux MMC List, LKML, Jaehoon Chung, Shawn Lin
In-Reply-To: <CAPDyKFoKN2zUNvDkgciO6r_ohdh2Vaj5qQaAPwMq21y02XAK8A@mail.gmail.com>
Hi,
On Tue, May 28, 2019 at 6:12 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Mon, 20 May 2019 at 20:41, Doug Anderson <dianders@chromium.org> wrote:
> >
> > Hi,
> >
> > On Mon, Apr 29, 2019 at 1:41 PM Douglas Anderson <dianders@chromium.org> wrote:
> > >
> > > Processing SDIO interrupts while dw_mmc is suspended (or partly
> > > suspended) seems like a bad idea. We really don't want to be
> > > processing them until we've gotten ourselves fully powered up.
> > >
> > > You might be wondering how it's even possible to become suspended when
> > > an SDIO interrupt is active. As can be seen in
> > > dw_mci_enable_sdio_irq(), we explicitly keep dw_mmc out of runtime
> > > suspend when the SDIO interrupt is enabled. ...but even though we
> > > stop normal runtime suspend transitions when SDIO interrupts are
> > > enabled, the dw_mci_runtime_suspend() can still get called for a full
> > > system suspend.
> > >
> > > Let's handle all this by explicitly masking SDIO interrupts in the
> > > suspend call and unmasking them later in the resume call. To do this
> > > cleanly I'll keep track of whether the client requested that SDIO
> > > interrupts be enabled so that we can reliably restore them regardless
> > > of whether we're masking them for one reason or another.
> > >
> > > It should be noted that if dw_mci_enable_sdio_irq() is never called
> > > (for instance, if we don't have an SDIO card plugged in) that
> > > "client_sdio_enb" will always be false. In those cases this patch
> > > adds a tiny bit of overhead to suspend/resume (a spinlock and a
> > > read/write of INTMASK) but other than that is a no-op. The
> > > SDMMC_INT_SDIO bit should always be clear and clearing it again won't
> > > hurt.
> > >
> > > Without this fix it can be seen that rk3288-veyron Chromebooks with
> > > Marvell WiFi would sometimes fail to resume WiFi even after picking my
> > > recent mwifiex patch [1]. Specifically you'd see messages like this:
> > > mwifiex_sdio mmc1:0001:1: Firmware wakeup failed
> > > mwifiex_sdio mmc1:0001:1: PREP_CMD: FW in reset state
> > >
> > > ...and tracing through the resume code in the failing cases showed
> > > that we were processing a SDIO interrupt really early in the resume
> > > call.
> > >
> > > NOTE: downstream in Chrome OS 3.14 and 3.18 kernels (both of which
> > > support the Marvell SDIO WiFi card) we had a patch ("CHROMIUM: sdio:
> > > Defer SDIO interrupt handling until after resume") [2]. Presumably
> > > this is the same problem that was solved by that patch.
> > >
> > > [1] https://lkml.kernel.org/r/20190404040106.40519-1-dianders@chromium.org
> > > [2] https://crrev.com/c/230765
> > >
> > > Cc: <stable@vger.kernel.org> # 4.14.x
> > > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > > ---
> > > I didn't put any "Fixes" tag here, but presumably this could be
> > > backported to whichever kernels folks found it useful for. I have at
> > > least confirmed that kernels v4.14 and v4.19 (as well as v5.1-rc2)
> > > show the problem. It is very easy to pick this to v4.19 and it
> > > definitely fixes the problem there.
> > >
> > > I haven't spent the time to pick this to 4.14 myself, but presumably
> > > it wouldn't be too hard to backport this as far as v4.13 since that
> > > contains commit 32dba73772f8 ("mmc: dw_mmc: Convert to use
> > > MMC_CAP2_SDIO_IRQ_NOTHREAD for SDIO IRQs"). Prior to that it might
> > > make sense for anyone experiencing this problem to just pick the old
> > > CHROMIUM patch to fix them.
> > >
> > > Changes in v2:
> > > - Suggested 4.14+ in the stable tag (Sasha-bot)
> > > - Extra note that this is a noop on non-SDIO (Shawn / Emil)
> > > - Make boolean logic cleaner as per https://crrev.com/c/1586207/1
> > > - Hopefully clear comments as per https://crrev.com/c/1586207/1
> > >
> > > drivers/mmc/host/dw_mmc.c | 27 +++++++++++++++++++++++----
> > > drivers/mmc/host/dw_mmc.h | 3 +++
> > > 2 files changed, 26 insertions(+), 4 deletions(-)
> >
> > Ulf: are you the right person to land this? With 5.2-rc1 out it might
> > be a good time for it? To refresh your memory about this patch:
> >
> > * Patch v1 was posted back on April 10th [1] so we're at about 1.5
> > months of time for people to comment about it now. Should be more
> > than enough.
>
> Apologize for the delay, not sure why this has slipped through my
> filters. Anyway, let me have a look at it now.
No worries. If there's something better I can do in the future to
avoid problems, please let me know.
> > * Shawn Lin saw it and didn't hate it. He had some confusion about
> > how it worked and I've hopefully alleviated via extra comments / text.
> >
> > * Emil Renner Berthing thought it caused a regression for him but then
> > tested further and was convinced that it didn't. This is extra
> > confirmation that someone other than me did try the patch and found it
> > to not break things. ;-)
> >
> > * It has been reviewed by Guenter Roeck (in v2)
>
> One question, I am guessing you are considering
> https://lkml.org/lkml/2019/5/17/761 as the long term solution, and
> thus $subject patch should go as fix+stable? No?
No, the two problems are completely separate. ${SUBJECT} patch deals
with full system suspend/resume. I originally reproduced the problems
on a device with Marvell WiFi, though it could possibly affect
Broadcom parts on dw_mmc too. ${SUBJECT} patch is ready to land on
mainline Linux at your leisure. Also: in case you didn't notice
(since it didn't thread properly for me), Shawn Lin gave it a reviewed
by [1].
The patch you refer to [2] is related to something akin to an idle
state (more like Runtime PM) and only affects Broadcom parts. Also
the Broadcom issue ought to happen across all host controllers (AKA
not just dw_mmc).
[1] https://lkml.kernel.org/r/982ffba1-c599-e73d-e5e0-b1be5668851c@rock-chips.com
[2] https://lkml.org/lkml/2019/5/17/761
-Doug
^ permalink raw reply
* Re: [PATCH] network: wireless: p54u: Fix race between disconnect and firmware loading
From: Kalle Valo @ 2019-05-28 14:29 UTC (permalink / raw)
To: Alan Stern
Cc: Christian Lamparter, syzbot, davem, andreyknvl, syzkaller-bugs,
Kernel development list, USB list, linux-wireless, netdev
In-Reply-To: <Pine.LNX.4.44L0.1905281014340.1564-100000@iolanthe.rowland.org>
Alan Stern <stern@rowland.harvard.edu> writes:
> On Tue, 28 May 2019, Kalle Valo wrote:
>
>> The correct prefix is "p54:", but I can fix that during commit.
>
> Oh, okay, thanks.
>
>> > Index: usb-devel/drivers/net/wireless/intersil/p54/p54usb.c
>> > ===================================================================
>> > --- usb-devel.orig/drivers/net/wireless/intersil/p54/p54usb.c
>> > +++ usb-devel/drivers/net/wireless/intersil/p54/p54usb.c
>> > @@ -33,6 +33,8 @@ MODULE_ALIAS("prism54usb");
>> > MODULE_FIRMWARE("isl3886usb");
>> > MODULE_FIRMWARE("isl3887usb");
>> >
>> > +static struct usb_driver p54u_driver;
>>
>> How is it safe to use static variables from a wireless driver? For
>> example, what if there are two p54 usb devices on the host? How do we
>> avoid a race in that case?
>
> There is no race. This structure is not per-device; it refers only to
> the driver. In fact, the line above is only a forward declaration --
> the actual definition of p54u_driver was already in the source file.
Ah, I missed that. Thanks!
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH] network: wireless: p54u: Fix race between disconnect and firmware loading
From: Alan Stern @ 2019-05-28 14:17 UTC (permalink / raw)
To: Kalle Valo
Cc: Christian Lamparter, syzbot, davem, andreyknvl, syzkaller-bugs,
Kernel development list, USB list, linux-wireless, netdev
In-Reply-To: <8736kyvkw9.fsf@purkki.adurom.net>
On Tue, 28 May 2019, Kalle Valo wrote:
> The correct prefix is "p54:", but I can fix that during commit.
Oh, okay, thanks.
> > Index: usb-devel/drivers/net/wireless/intersil/p54/p54usb.c
> > ===================================================================
> > --- usb-devel.orig/drivers/net/wireless/intersil/p54/p54usb.c
> > +++ usb-devel/drivers/net/wireless/intersil/p54/p54usb.c
> > @@ -33,6 +33,8 @@ MODULE_ALIAS("prism54usb");
> > MODULE_FIRMWARE("isl3886usb");
> > MODULE_FIRMWARE("isl3887usb");
> >
> > +static struct usb_driver p54u_driver;
>
> How is it safe to use static variables from a wireless driver? For
> example, what if there are two p54 usb devices on the host? How do we
> avoid a race in that case?
There is no race. This structure is not per-device; it refers only to
the driver. In fact, the line above is only a forward declaration --
the actual definition of p54u_driver was already in the source file.
Alan Stern
^ permalink raw reply
* Re: [PATCH v2] mmc: dw_mmc: Disable SDIO interrupts while suspended to fix suspend/resume
From: Ulf Hansson @ 2019-05-28 13:11 UTC (permalink / raw)
To: Doug Anderson
Cc: Kalle Valo, Heiko Stübner, open list:ARM/Rockchip SoC...,
Guenter Roeck, Brian Norris, linux-wireless, Sonny Rao,
Emil Renner Berthing, Matthias Kaehlcke, Ryan Case, # 4.0+,
Linux MMC List, LKML, Jaehoon Chung, Shawn Lin
In-Reply-To: <CAD=FV=WEDkufoEUYv9U+c+Y_bm8MYEWS25n63vUeNG0LLCFnuw@mail.gmail.com>
On Mon, 20 May 2019 at 20:41, Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Mon, Apr 29, 2019 at 1:41 PM Douglas Anderson <dianders@chromium.org> wrote:
> >
> > Processing SDIO interrupts while dw_mmc is suspended (or partly
> > suspended) seems like a bad idea. We really don't want to be
> > processing them until we've gotten ourselves fully powered up.
> >
> > You might be wondering how it's even possible to become suspended when
> > an SDIO interrupt is active. As can be seen in
> > dw_mci_enable_sdio_irq(), we explicitly keep dw_mmc out of runtime
> > suspend when the SDIO interrupt is enabled. ...but even though we
> > stop normal runtime suspend transitions when SDIO interrupts are
> > enabled, the dw_mci_runtime_suspend() can still get called for a full
> > system suspend.
> >
> > Let's handle all this by explicitly masking SDIO interrupts in the
> > suspend call and unmasking them later in the resume call. To do this
> > cleanly I'll keep track of whether the client requested that SDIO
> > interrupts be enabled so that we can reliably restore them regardless
> > of whether we're masking them for one reason or another.
> >
> > It should be noted that if dw_mci_enable_sdio_irq() is never called
> > (for instance, if we don't have an SDIO card plugged in) that
> > "client_sdio_enb" will always be false. In those cases this patch
> > adds a tiny bit of overhead to suspend/resume (a spinlock and a
> > read/write of INTMASK) but other than that is a no-op. The
> > SDMMC_INT_SDIO bit should always be clear and clearing it again won't
> > hurt.
> >
> > Without this fix it can be seen that rk3288-veyron Chromebooks with
> > Marvell WiFi would sometimes fail to resume WiFi even after picking my
> > recent mwifiex patch [1]. Specifically you'd see messages like this:
> > mwifiex_sdio mmc1:0001:1: Firmware wakeup failed
> > mwifiex_sdio mmc1:0001:1: PREP_CMD: FW in reset state
> >
> > ...and tracing through the resume code in the failing cases showed
> > that we were processing a SDIO interrupt really early in the resume
> > call.
> >
> > NOTE: downstream in Chrome OS 3.14 and 3.18 kernels (both of which
> > support the Marvell SDIO WiFi card) we had a patch ("CHROMIUM: sdio:
> > Defer SDIO interrupt handling until after resume") [2]. Presumably
> > this is the same problem that was solved by that patch.
> >
> > [1] https://lkml.kernel.org/r/20190404040106.40519-1-dianders@chromium.org
> > [2] https://crrev.com/c/230765
> >
> > Cc: <stable@vger.kernel.org> # 4.14.x
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > ---
> > I didn't put any "Fixes" tag here, but presumably this could be
> > backported to whichever kernels folks found it useful for. I have at
> > least confirmed that kernels v4.14 and v4.19 (as well as v5.1-rc2)
> > show the problem. It is very easy to pick this to v4.19 and it
> > definitely fixes the problem there.
> >
> > I haven't spent the time to pick this to 4.14 myself, but presumably
> > it wouldn't be too hard to backport this as far as v4.13 since that
> > contains commit 32dba73772f8 ("mmc: dw_mmc: Convert to use
> > MMC_CAP2_SDIO_IRQ_NOTHREAD for SDIO IRQs"). Prior to that it might
> > make sense for anyone experiencing this problem to just pick the old
> > CHROMIUM patch to fix them.
> >
> > Changes in v2:
> > - Suggested 4.14+ in the stable tag (Sasha-bot)
> > - Extra note that this is a noop on non-SDIO (Shawn / Emil)
> > - Make boolean logic cleaner as per https://crrev.com/c/1586207/1
> > - Hopefully clear comments as per https://crrev.com/c/1586207/1
> >
> > drivers/mmc/host/dw_mmc.c | 27 +++++++++++++++++++++++----
> > drivers/mmc/host/dw_mmc.h | 3 +++
> > 2 files changed, 26 insertions(+), 4 deletions(-)
>
> Ulf: are you the right person to land this? With 5.2-rc1 out it might
> be a good time for it? To refresh your memory about this patch:
>
> * Patch v1 was posted back on April 10th [1] so we're at about 1.5
> months of time for people to comment about it now. Should be more
> than enough.
Apologize for the delay, not sure why this has slipped through my
filters. Anyway, let me have a look at it now.
>
> * Shawn Lin saw it and didn't hate it. He had some confusion about
> how it worked and I've hopefully alleviated via extra comments / text.
>
> * Emil Renner Berthing thought it caused a regression for him but then
> tested further and was convinced that it didn't. This is extra
> confirmation that someone other than me did try the patch and found it
> to not break things. ;-)
>
> * It has been reviewed by Guenter Roeck (in v2)
One question, I am guessing you are considering
https://lkml.org/lkml/2019/5/17/761 as the long term solution, and
thus $subject patch should go as fix+stable? No?
>
> [1] https://lkml.kernel.org/r/20190410221237.160856-1-dianders@chromium.org
>
>
> -Doug
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH] rtlwifi: Fix null-pointer dereferences in error handling code of rtl_pci_probe()
From: Kalle Valo @ 2019-05-28 13:09 UTC (permalink / raw)
To: Larry Finger
Cc: Jia-Ju Bai, pkshih, davem, linux-wireless, netdev, linux-kernel
In-Reply-To: <2658b691-b992-b773-c6cf-85801adc479f@lwfinger.net>
Larry Finger <Larry.Finger@lwfinger.net> writes:
> On 5/28/19 6:55 AM, Kalle Valo wrote:
>> Jia-Ju Bai <baijiaju1990@gmail.com> wrote:
>>
>>> *BUG 1:
>>> In rtl_pci_probe(), when rtlpriv->cfg->ops->init_sw_vars() fails,
>>> rtl_deinit_core() in the error handling code is executed.
>>> rtl_deinit_core() calls rtl_free_entries_from_scan_list(), which uses
>>> rtlpriv->scan_list.list in list_for_each_entry_safe(), but it has been
>>> initialized. Thus a null-pointer dereference occurs.
>>> The reason is that rtlpriv->scan_list.list is initialized by
>>> INIT_LIST_HEAD() in rtl_init_core(), which has not been called.
>>>
>>> To fix this bug, rtl_deinit_core() should not be called when
>>> rtlpriv->cfg->ops->init_sw_vars() fails.
>>>
>>> *BUG 2:
>>> In rtl_pci_probe(), rtl_init_core() can fail when rtl_regd_init() in
>>> this function fails, and rtlpriv->scan_list.list has not been
>>> initialized by INIT_LIST_HEAD(). Then, rtl_deinit_core() in the error
>>> handling code of rtl_pci_probe() is executed. Finally, a null-pointer
>>> dereference occurs due to the same reason of the above bug.
>>>
>>> To fix this bug, the initialization of lists in rtl_init_core() are
>>> performed before the call to rtl_regd_init().
>>>
>>> These bugs are found by a runtime fuzzing tool named FIZZER written by
>>> us.
>>>
>>> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
>>
>> Ping & Larry, is this ok to take?
>>
>
> Kalle,
>
> Not at the moment. In reviewing the code, I was unable to see how this
> situation could develop, and his backtrace did not mention any rtlwifi
> code. For that reason, I asked him to add printk stat4ements to show
> the last part of rtl_pci that executed correctly. In
> https://marc.info/?l=linux-wireless&m=155788322631134&w=2, he promised
> to do that, but I have not seen the result.
Ok, thanks. I'll then drop this, please resubmit once everything is
understood and the patch is ready to be applied.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH] rtlwifi: Fix null-pointer dereferences in error handling code of rtl_pci_probe()
From: Larry Finger @ 2019-05-28 13:00 UTC (permalink / raw)
To: Kalle Valo, Jia-Ju Bai
Cc: pkshih, davem, linux-wireless, netdev, linux-kernel
In-Reply-To: <20190528115555.301E760F3C@smtp.codeaurora.org>
On 5/28/19 6:55 AM, Kalle Valo wrote:
> Jia-Ju Bai <baijiaju1990@gmail.com> wrote:
>
>> *BUG 1:
>> In rtl_pci_probe(), when rtlpriv->cfg->ops->init_sw_vars() fails,
>> rtl_deinit_core() in the error handling code is executed.
>> rtl_deinit_core() calls rtl_free_entries_from_scan_list(), which uses
>> rtlpriv->scan_list.list in list_for_each_entry_safe(), but it has been
>> initialized. Thus a null-pointer dereference occurs.
>> The reason is that rtlpriv->scan_list.list is initialized by
>> INIT_LIST_HEAD() in rtl_init_core(), which has not been called.
>>
>> To fix this bug, rtl_deinit_core() should not be called when
>> rtlpriv->cfg->ops->init_sw_vars() fails.
>>
>> *BUG 2:
>> In rtl_pci_probe(), rtl_init_core() can fail when rtl_regd_init() in
>> this function fails, and rtlpriv->scan_list.list has not been
>> initialized by INIT_LIST_HEAD(). Then, rtl_deinit_core() in the error
>> handling code of rtl_pci_probe() is executed. Finally, a null-pointer
>> dereference occurs due to the same reason of the above bug.
>>
>> To fix this bug, the initialization of lists in rtl_init_core() are
>> performed before the call to rtl_regd_init().
>>
>> These bugs are found by a runtime fuzzing tool named FIZZER written by
>> us.
>>
>> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
>
> Ping & Larry, is this ok to take?
>
Kalle,
Not at the moment. In reviewing the code, I was unable to see how this situation
could develop, and his backtrace did not mention any rtlwifi code. For that
reason, I asked him to add printk stat4ements to show the last part of rtl_pci
that executed correctly. In
https://marc.info/?l=linux-wireless&m=155788322631134&w=2, he promised to do
that, but I have not seen the result.
Larry
^ permalink raw reply
* Re: [PATCH] ath10k: remove mmc_hw_reset while hif power down
From: Ulf Hansson @ 2019-05-28 12:45 UTC (permalink / raw)
To: Kalle Valo, Wen Gong, Grant Grundler
Cc: Wen Gong, linux-wireless@vger.kernel.org,
ath10k@lists.infradead.org
In-Reply-To: <87pnour4jg.fsf@codeaurora.org>
On Tue, 7 May 2019 at 11:35, Kalle Valo <kvalo@codeaurora.org> wrote:
>
> + Ulf to give comments from SDIO point of view
Apologize for the delay, it's been a busy period.
>
> Wen Gong <wgong@qti.qualcomm.com> writes:
>
> >> -----Original Message-----
> >> From: ath10k <ath10k-bounces@lists.infradead.org> On Behalf Of Grant
> >> Grundler
> >> Sent: Saturday, May 4, 2019 2:01 AM
> >> To: Wen Gong <wgong@codeaurora.org>
> >> Cc: linux-wireless@vger.kernel.org; ath10k@lists.infradead.org
> >> Subject: [EXT] Re: [PATCH] ath10k: remove mmc_hw_reset while hif power
> >> down
> >>
> >> [repeating comments I made in the gerrit review for Chrome OS :
> >> https://chromium-
> >> review.googlesource.com/c/chromiumos/third_party/kernel/+/1585667
> >> ]
> >>
> >> On Sat, Apr 27, 2019 at 7:17 PM Wen Gong <wgong@codeaurora.org> wrote:
> >> >
> >> > For sdio 3.0 chip, the clock will drop from 200M Hz to 50M Hz after load
> >> > ath10k driver, it is because mmc_hw_reset will reset the sdio's power,
> >> > then mmc will consider it as sdio 2.0 and drop the clock.
> >>
> >> Wen,
> >> 5468e784c0600551ca03263f5255a375c05f88e7 commit message gives
> >> reasons
> >> for adding the mmc_hw_reset() call. The commit message for removing
> >> gives different reason for removal. Both are good but second one is
> >> incomplete.
> >>
> >> The commit message for removal should ALSO explain why adding this
> >> call wasn't necessary in the first place OR move the call to a
> >> different code path.
> >>
> >> > Remove mmc_hw_reset will avoid the drop of clock.
> >>
> >> This commit message makes it clear the original patch introduced a new
> >> problem. But the original patch fixed a different problem and that
> >> this proposed change seems likely to re-introduce and the commit
> >> message should explain why that isn't true (or how the original was
> >> fixed differently)
> >
> > The mmc_hw_reset's effect depends on the hardware layout/configure
> > software's behavior, recently it will effect the clock of sdio for the
> > platform I used. And it will still work well without mmc_hw_reset for
> > the platform I Used currently. If sdio cannot work on other platform,
> > I think it can add flag in ath10k_hw_params_list for the platform to
> > call the mmc_hw_reset depends on the flag.
>
> I don't see how you can use ath10k_hw_params_list to separate SDIO
> controller functionality, I assume that's the real reason for difference
> of functionality? Maybe this is a bug on the SDIO controller?
Ideally mmc_hw_reset() should not make the SDIO card to become
re-initialized differently than it was originally.
However, as that is the case here, it sounds like that there is a bug
somewhere. Perhaps the boot-loader have done some pre-initialization,
which isn't covered in the second case, or maybe a bug in the mmc host
driver.
>
> Ulf, what do you think? Any suggestions? Full discussion here:
>
> https://patchwork.kernel.org/patch/10920563/
>
> --
> Kalle Valo
In the end, it seems like this needs a more detailed debug study, to
figure out what exactly happens during the re-initialization of the
SDIO card, rather than just papering over the problem by removing the
call to mmc_hw_reset() in the SDIO func driver. Hope this helps.
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH] b43: Avoid possible double calls to b43_one_core_detach()
From: Kalle Valo @ 2019-05-28 12:43 UTC (permalink / raw)
To: Jia-Ju Bai
Cc: davem, colin.king, yuehaibing, linux-wireless, b43-dev, netdev,
linux-kernel, Jia-Ju Bai
In-Reply-To: <20190504091000.18665-1-baijiaju1990@gmail.com>
Jia-Ju Bai <baijiaju1990@gmail.com> wrote:
> In b43_request_firmware(), when ieee80211_register_hw() fails,
> b43_one_core_detach() is called. In b43_bcma_remove() and
> b43_ssb_remove(), b43_one_core_detach() is called again. In this case,
> null-pointer dereferences and double-free problems can occur when
> the driver is removed.
>
> To fix this bug, the call to b43_one_core_detach() in
> b43_request_firmware() is deleted.
>
> This bug is found by a runtime fuzzing tool named FIZZER written by us.
>
> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
Patch applied to wireless-drivers-next.git, thanks.
ec2e93cf1910 b43: Avoid possible double calls to b43_one_core_detach()
--
https://patchwork.kernel.org/patch/10929623/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] libertas: fix spelling mistake "Donwloading" -> "Downloading"
From: Kalle Valo @ 2019-05-28 12:43 UTC (permalink / raw)
To: Colin King
Cc: David S . Miller, libertas-dev, linux-wireless, netdev,
kernel-janitors, linux-kernel
In-Reply-To: <20190514211406.6353-1-colin.king@canonical.com>
Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> There is are two spelling mistakes in lbtf_deb_usb2 messages, fix these.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> Reviewed-by: Mukesh Ojha <mojha@codeaurora.org>
Patch applied to wireless-drivers-next.git, thanks.
aeffda6b10f8 libertas: fix spelling mistake "Donwloading" -> "Downloading"
--
https://patchwork.kernel.org/patch/10943765/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] wlcore: spi: Fix a memory leaking bug in wl1271_probe()
From: Gen Zhang @ 2019-05-28 12:38 UTC (permalink / raw)
To: Kalle Valo; +Cc: davem, linux-wireless, netdev, linux-kernel
In-Reply-To: <87tvde4v3u.fsf@kamboji.qca.qualcomm.com>
On Tue, May 28, 2019 at 03:33:09PM +0300, Kalle Valo wrote:
> Yeah, I don't see how that thread proves that these patches are correct.
>
Sure, I didn't mean that we came to an agreement that these patches are
correct.
> > Further, I e-mailed Greg K-H about when should we use devm_kmalloc().
> >
> > On Tue, May 28, 2019 at 08:32:57AM +0800, Gen Zhang wrote:
> >> devm_kmalloc() is used to allocate memory for a driver dev. Comments
> >> above the definition and doc
> >> (https://www.kernel.org/doc/Documentation/driver-model/devres.txt) all
> >> imply that allocated the memory is automatically freed on driver attach,
> >> no matter allocation fail or not. However, I examined the code, and
> >> there are many sites that devm_kfree() is used to free devm_kmalloc().
> >> e.g. hisi_sas_debugfs_init() in drivers/scsi/hisi_sas/hisi_sas_main.c.
> >> So I am totally confused about this issue. Can anybody give me some
> >> guidance? When should we use devm_kfree()?
> > He replied: If you "know" you need to free the memory now,
> > call devm_kfree(). If you want to wait for it to be cleaned up latter,
> > like normal, then do not call it.
> >
> > So could please look in to this issue?
>
> Sorry, no time to investigate this in detail. If you think the patches
> are correct you can resend them and get someone familiar with the driver
> to provide Reviewed-by, then I will apply them.
>
> --
> Kalle Valo
Ok, thanks for your time. I will follow your suggestions.
Thanks
Gen
^ permalink raw reply
* Re: [PATCH] rtlwifi: btcoex: remove unused function exhalbtc_stack_operation_notify
From: Kalle Valo @ 2019-05-28 12:38 UTC (permalink / raw)
To: YueHaibing
Cc: pkshih, davem, linux-kernel, netdev, linux-wireless, YueHaibing
In-Reply-To: <20190525144844.16976-1-yuehaibing@huawei.com>
YueHaibing <yuehaibing@huawei.com> wrote:
> There is no callers in tree, so can be removed.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Patch applied to wireless-drivers-next.git, thanks.
dfbe36197dbc rtlwifi: btcoex: remove unused function exhalbtc_stack_operation_notify
--
https://patchwork.kernel.org/patch/10960845/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] rtlwifi: btcoex: Remove set but not used variable 'len' and 'asso_type_v2'
From: Kalle Valo @ 2019-05-28 12:37 UTC (permalink / raw)
To: YueHaibing
Cc: pkshih, davem, linux-kernel, netdev, linux-wireless, YueHaibing
In-Reply-To: <20190525144634.10696-1-yuehaibing@huawei.com>
YueHaibing <yuehaibing@huawei.com> wrote:
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.c: In function rtl_btc_btmpinfo_notify:
> drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.c:319:17: warning: variable len set but not used [-Wunused-but-set-variable]
> drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c: In function exhalbtc_connect_notify:
> drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c:1581:16: warning: variable asso_type_v2 set but not used [-Wunused-but-set-variable]
>
> 'len' is never used since commit 6aad6075ccd5 ("rtlwifi:
> Add BT_MP_INFO to c2h handler.") so can be removed.
>
> 'asso_type_v2' is not used since introduction in
> commit 0843e98a3b9a ("rtlwifi: btcoex: add assoc
> type v2 to connection notify")
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Patch applied to wireless-drivers-next.git, thanks.
d477a4856aec rtlwifi: btcoex: Remove set but not used variable 'len' and 'asso_type_v2'
--
https://patchwork.kernel.org/patch/10960843/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] rtlwifi: rtl8821ae: Remove set but not used variables 'cur_txokcnt' and 'b_last_is_cur_rdl_state'
From: Kalle Valo @ 2019-05-28 12:37 UTC (permalink / raw)
To: YueHaibing
Cc: pkshih, davem, linux-kernel, netdev, linux-wireless, YueHaibing
In-Reply-To: <20190525144332.17268-1-yuehaibing@huawei.com>
YueHaibing <yuehaibing@huawei.com> wrote:
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/net/wireless/realtek/rtlwifi/rtl8821ae/dm.c: In function rtl8821ae_dm_check_rssi_monitor:
> drivers/net/wireless/realtek/rtlwifi/rtl8821ae/dm.c:658:6: warning: variable cur_txokcnt set but not used [-Wunused-but-set-variable]
> drivers/net/wireless/realtek/rtlwifi/rtl8821ae/dm.c: In function rtl8821ae_dm_check_edca_turbo:
> drivers/net/wireless/realtek/rtlwifi/rtl8821ae/dm.c:2657:7: warning: variable b_last_is_cur_rdl_state set but not used [-Wunused-but-set-variable]
>
> They are never used so can be removed.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Patch applied to wireless-drivers-next.git, thanks.
3e42a66dfd15 rtlwifi: rtl8821ae: Remove set but not used variables 'cur_txokcnt' and 'b_last_is_cur_rdl_state'
--
https://patchwork.kernel.org/patch/10960841/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH 1/4] rtlwifi: rtl8821ae: Remove unused GET_XXX and SET_XXX descriptor macros
From: Kalle Valo @ 2019-05-28 12:36 UTC (permalink / raw)
To: Larry Finger; +Cc: linux-wireless, pkshih, Larry Finger
In-Reply-To: <20190520172359.9993-2-Larry.Finger@lwfinger.net>
Larry Finger <Larry.Finger@lwfinger.net> wrote:
> As the first step in converting from macros that get/set information
> in the RX and TX descriptors, unused macros are being removed.
>
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
4 patches applied to wireless-drivers-next.git, thanks.
7ffe556230b8 rtlwifi: rtl8821ae: Remove unused GET_XXX and SET_XXX descriptor macros
f5678bfe1cdc rtlwifi: rtl8821ae: Replace local bit manipulation macros
bd421dab7515 rtlwifi: rtl8821ae: Convert macros that set descriptor
f7fbb03f2169 rtlwifi: rtl8821ae: Convert inline routines to little-endian words
--
https://patchwork.kernel.org/patch/10951971/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH 1/4] rtlwifi: 8192de: Reduce indentation and fix coding style
From: Kalle Valo @ 2019-05-28 12:34 UTC (permalink / raw)
To: pkshih; +Cc: Larry.Finger, linux-wireless, colin.king
In-Reply-To: <20190510080333.3789-2-pkshih@realtek.com>
<pkshih@realtek.com> wrote:
> From: Ping-Ke Shih <pkshih@realtek.com>
>
> This commit doesn't change logic at all. Since indentation is lower, the
> wrapped statements can be put in single line that will become more
> readable.
>
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
4 patches applied to wireless-drivers-next.git, thanks.
3bde4ed3da38 rtlwifi: 8192de: Reduce indentation and fix coding style
8a7db8b66b40 rtlwifi: 8192de: make tables to be 'static const'
0e7d38132bf8 rtlwifi: 8192de: Fix used uninitialized variables in power tracking
80429a86e3d9 rtlwifi: 8192de: use le32 to access cckswing tables
--
https://patchwork.kernel.org/patch/10938337/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [wireless-regdb] [PATCH v3] wireless-regdb: Update regulatory rules for South Korea
From: Seth Forshee @ 2019-05-28 12:32 UTC (permalink / raw)
To: Peter Oh; +Cc: wireless-regdb@lists.infradead.org,
linux-wireless@vger.kernel.org
In-Reply-To: <1557948501-12579-1-git-send-email-peter.oh@bowerswilkins.com>
On Wed, May 15, 2019 at 07:28:29PM +0000, Peter Oh wrote:
> From: Peter Oh <peter.oh@bowerswilkins.com>
>
> Update power limit as documented in:
> http://www.law.go.kr/%ED%96%89%EC%A0%95%EA%B7%9C%EC%B9%99/
> %EC%8B%A0%EA%B3%A0%ED%95%98%EC%A7%80%EC%95%84%EB%8B%88%ED
> %95%98%EA%B3%A0%EA%B0%9C%EC%84%A4%ED%95%A0%EC%88%98%EC%9E
> %88%EB%8A%94%EB%AC%B4%EC%84%A0%EA%B5%AD%EC%9A%A9%EB%AC%B4
> %EC%84%A0%EA%B8%B0%EA%B8%B0/(2018-89,20181227)
> which revised on December 27, 2018.
>
> Signed-off-by: Peter Oh <peter.oh@bowerswilkins.com>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH] wlcore: spi: Fix a memory leaking bug in wl1271_probe()
From: Kalle Valo @ 2019-05-28 12:33 UTC (permalink / raw)
To: Gen Zhang; +Cc: davem, linux-wireless, netdev, linux-kernel
In-Reply-To: <20190528121452.GA23464@zhanggen-UX430UQ>
Gen Zhang <blackgod016574@gmail.com> writes:
> On Tue, May 28, 2019 at 11:39:22AM +0000, Kalle Valo wrote:
>> Gen Zhang <blackgod016574@gmail.com> 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.
>> >
>> > 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>
>>
>> Same questions as with similar SDIO patch:
>>
>> https://patchwork.kernel.org/patch/10959049/
>>
>> Patch set to Changes Requested.
>>
>> --
>> https://patchwork.kernel.org/patch/10959053/
>>
>> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
>>
> Thanks for your reply, Kalle. I had debate with Jon about this patch.
> You could kindly refer to lkml: https://lkml.org/lkml/2019/5/23/1547.
> And I don't think a practical conclusion is made there.
Yeah, I don't see how that thread proves that these patches are correct.
> Further, I e-mailed Greg K-H about when should we use devm_kmalloc().
>
> On Tue, May 28, 2019 at 08:32:57AM +0800, Gen Zhang wrote:
>> devm_kmalloc() is used to allocate memory for a driver dev. Comments
>> above the definition and doc
>> (https://www.kernel.org/doc/Documentation/driver-model/devres.txt) all
>> imply that allocated the memory is automatically freed on driver attach,
>> no matter allocation fail or not. However, I examined the code, and
>> there are many sites that devm_kfree() is used to free devm_kmalloc().
>> e.g. hisi_sas_debugfs_init() in drivers/scsi/hisi_sas/hisi_sas_main.c.
>> So I am totally confused about this issue. Can anybody give me some
>> guidance? When should we use devm_kfree()?
> He replied: If you "know" you need to free the memory now,
> call devm_kfree(). If you want to wait for it to be cleaned up latter,
> like normal, then do not call it.
>
> So could please look in to this issue?
Sorry, no time to investigate this in detail. If you think the patches
are correct you can resend them and get someone familiar with the driver
to provide Reviewed-by, then I will apply them.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH 1/7] rt2x00: allow to specify watchdog interval
From: Kalle Valo @ 2019-05-28 12:28 UTC (permalink / raw)
To: Stanislaw Gruszka
Cc: linux-wireless, Tomislav Požega, Daniel Golle, Felix Fietkau,
Mathias Kresin
In-Reply-To: <1556788021-6531-2-git-send-email-sgruszka@redhat.com>
Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> Allow subdriver to change watchdog interval by intialize
> link->watchdog_interval value before rt2x00link_register().
>
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
Failed to apply:
fatal: sha1 information is lacking or useless (drivers/net/wireless/ralink/rt2x00/rt2800lib.c).
error: could not build fake ancestor
Applying: rt2800: initial watchdog implementation
Patch failed at 0001 rt2800: initial watchdog implementation
The copy of the patch that failed is found in: .git/rebase-apply/patch
7 patches set to Changes Requested.
10926395 [1/7] rt2x00: allow to specify watchdog interval
10926397 [2/7] rt2800: add helpers for reading dma done index
10926401 [3/7] rt2800: initial watchdog implementation
10926403 [4/7] rt2800: add pre_reset_hw callback
10926405 [5/7] rt2800: do not nullify initialization vector data
10926407 [6/7] rt2x00: add restart hw
10926409 [7/7] rt2800: do not enable watchdog by default
--
https://patchwork.kernel.org/patch/10926395/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] p54: Support boottime in scan results
From: Kalle Valo @ 2019-05-28 12:27 UTC (permalink / raw)
To: Christian Lamparter; +Cc: linux-wireless
In-Reply-To: <20190524213308.18575-1-chunkeey@gmail.com>
Christian Lamparter <chunkeey@gmail.com> wrote:
> 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>
Patch applied to wireless-drivers-next.git, thanks.
c11c75ec784e p54: Support boottime in scan results
--
https://patchwork.kernel.org/patch/10960463/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] p54: fix crash during initialization
From: Kalle Valo @ 2019-05-28 12:26 UTC (permalink / raw)
To: Christian Lamparter; +Cc: linux-wireless, David S . Miller
In-Reply-To: <20190518200548.7662-1-chunkeey@gmail.com>
Christian Lamparter <chunkeey@gmail.com> wrote:
> This patch fixes a crash that got introduced when the
> mentioned patch replaced the direct list_head access
> with skb_peek_tail(). When the device is starting up,
> there are no entries in the queue, so previously to
> "Use skb_peek_tail() instead..." the target_skb would
> end up as the tail and head pointer which then could
> be used by __skb_queue_after to fill the empty queue.
>
> With skb_peek_tail() in its place will instead just
> return NULL which then causes a crash in the
> __skb_queue_after().
>
> | BUG: unable to handle kernel NULL pointer dereference at 000000
> | #PF error: [normal kernel read fault]
> | PGD 0 P4D 0
> | Oops: 0000 [#1] SMP PTI
> | CPU: 0 PID: 12 Comm: kworker/0:1 Tainted: GO 5.1.0-rc7-wt+ #218
> | Hardware name: MSI MS-7816/Z87-G43 (MS-7816), BIOS V1.11 05/09/2015
> | Workqueue: events request_firmware_work_func
> | RIP: 0010:p54_tx_pending+0x10f/0x1b0 [p54common]
> | Code: 78 06 80 78 28 00 74 6d <48> 8b 07 49 89 7c 24 08 49 89 04 24 4
> | RSP: 0018:ffffa81c81927d90 EFLAGS: 00010086
> | RAX: ffff9bbaaf131048 RBX: 0000000000020670 RCX: 0000000000020264
> | RDX: ffff9bbaa976d660 RSI: 0000000000000202 RDI: 0000000000000000
> | RBP: ffff9bbaa976d620 R08: 00000000000006c0 R09: ffff9bbaa976d660
> | R10: 0000000000000000 R11: ffffe8480dbc5900 R12: ffff9bbb45e87700
> | R13: ffff9bbaa976d648 R14: ffff9bbaa976d674 R15: ffff9bbaaf131048
> | FS: 0000000000000000(0000) GS:ffff9bbb5ec00000(0000) knlGS:00000
> | CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> | CR2: 0000000000000000 CR3: 00000003695fc003 CR4: 00000000001606f0
> | Call Trace:
> | p54_download_eeprom+0xbe/0x120 [p54common]
> | p54_read_eeprom+0x7f/0xc0 [p54common]
> | p54u_load_firmware_cb+0xe0/0x160 [p54usb]
> | request_firmware_work_func+0x42/0x80
> | process_one_work+0x1f5/0x3f0
> | worker_thread+0x28/0x3c0
>
> Cc: stable@vger.kernel.org
> Fixes: e3554197fc8f ("p54: Use skb_peek_tail() instead of direct head pointer accesses.")
> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
Patch applied to wireless-drivers-next.git, thanks.
1645ab931998 p54: fix crash during initialization
--
https://patchwork.kernel.org/patch/10949055/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] brcmfmac: use strlcpy() instead of strcpy()
From: Kalle Valo @ 2019-05-28 12:25 UTC (permalink / raw)
To: neojou
Cc: arend.vanspriel, franky.lin, hante.meuleman, chi-hsien.lin,
wright.feng, davem, rafal, hdegoedg, p.figiel, linux-wireless,
brcm80211-dev-list.pdl, brcm80211-dev-list, netdev, linux-kernel,
Neo Jou
In-Reply-To: <1558429940-8709-1-git-send-email-neojou@gmail.com>
neojou@gmail.com wrote:
> From: Neo Jou <neojou@gmail.com>
>
> The function strcpy() is inherently not safe. Though the function
> works without problems here, it would be better to use other safer
> function, e.g. strlcpy(), to replace strcpy() still.
>
> Signed-off-by: Neo Jou <neojou@gmail.com>
Patch applied to wireless-drivers-next.git, thanks.
bbfab331e3ab brcmfmac: use strlcpy() instead of strcpy()
--
https://patchwork.kernel.org/patch/10953203/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH v2] brcmfmac: fix typos in code comments
From: Kalle Valo @ 2019-05-28 12:25 UTC (permalink / raw)
To: Weitao Hou
Cc: arend.vanspriel, franky.lin, hante.meuleman, chi-hsien.lin,
wright.feng, davem, houweitaoo, rafal, linux-wireless,
brcm80211-dev-list.pdl, brcm80211-dev-list, netdev, linux-kernel
In-Reply-To: <20190520122825.981-1-houweitaoo@gmail.com>
Weitao Hou <houweitaoo@gmail.com> wrote:
> fix lengh to length
>
> Signed-off-by: Weitao Hou <houweitaoo@gmail.com>
Patch applied to wireless-drivers-next.git, thanks.
b07e1ae2ce53 brcmfmac: fix typos in code comments
--
https://patchwork.kernel.org/patch/10950995/
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-28 12:24 UTC (permalink / raw)
To: Arend van Spriel; +Cc: linux-wireless, Arend van Spriel
In-Reply-To: <1558008251-13692-2-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.
>
> 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>
6 patches applied to wireless-drivers-next.git, thanks.
7e5677de8e0c brcm80211: switch common header files to using SPDX license identifier
afe06f822035 brcmutil: switch source files to using SPDX license identifier
f843863d6d10 brcmsmac: switch phy source files to using SPDX license identifier
daeccac2d5e7 brcmfmac: switch source files to using SPDX license identifier
9ff8614a3dbe brcmfmac: use separate Kconfig file for brcmfmac
885a93cf3110 brcm80211: select WANT_DEV_COREDUMP conditionally for brcmfmac
--
https://patchwork.kernel.org/patch/10946467/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* [PATCH] nl80211: fill all policy .type entries
From: Johannes Berg @ 2019-05-28 12:19 UTC (permalink / raw)
To: linux-wireless; +Cc: j, Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
For old commands, it's fine to have .type = NLA_UNSPEC and it
behaves the same as NLA_MIN_LEN. However, for new commands with
strict validation this is no longer true, and for policy export
to userspace these are also ignored.
Fix up the remaining ones that don't have a type.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
net/wireless/nl80211.c | 78 ++++++++++++++++++++++++++++++------------
1 file changed, 57 insertions(+), 21 deletions(-)
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index c391b560d986..4892f307f51d 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -304,8 +304,11 @@ const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_IFINDEX] = { .type = NLA_U32 },
[NL80211_ATTR_IFNAME] = { .type = NLA_NUL_STRING, .len = IFNAMSIZ-1 },
- [NL80211_ATTR_MAC] = { .len = ETH_ALEN },
- [NL80211_ATTR_PREV_BSSID] = { .len = ETH_ALEN },
+ [NL80211_ATTR_MAC] = { .type = NLA_EXACT_LEN_WARN, .len = ETH_ALEN },
+ [NL80211_ATTR_PREV_BSSID] = {
+ .type = NLA_EXACT_LEN_WARN,
+ .len = ETH_ALEN
+ },
[NL80211_ATTR_KEY] = { .type = NLA_NESTED, },
[NL80211_ATTR_KEY_DATA] = { .type = NLA_BINARY,
@@ -356,7 +359,10 @@ const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_MESH_CONFIG] = { .type = NLA_NESTED },
[NL80211_ATTR_SUPPORT_MESH_AUTH] = { .type = NLA_FLAG },
- [NL80211_ATTR_HT_CAPABILITY] = { .len = NL80211_HT_CAPABILITY_LEN },
+ [NL80211_ATTR_HT_CAPABILITY] = {
+ .type = NLA_EXACT_LEN_WARN,
+ .len = NL80211_HT_CAPABILITY_LEN
+ },
[NL80211_ATTR_MGMT_SUBTYPE] = { .type = NLA_U8 },
[NL80211_ATTR_IE] = NLA_POLICY_VALIDATE_FN(NLA_BINARY,
@@ -386,7 +392,10 @@ const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_WPA_VERSIONS] = { .type = NLA_U32 },
[NL80211_ATTR_PID] = { .type = NLA_U32 },
[NL80211_ATTR_4ADDR] = { .type = NLA_U8 },
- [NL80211_ATTR_PMKID] = { .len = WLAN_PMKID_LEN },
+ [NL80211_ATTR_PMKID] = {
+ .type = NLA_EXACT_LEN_WARN,
+ .len = WLAN_PMKID_LEN
+ },
[NL80211_ATTR_DURATION] = { .type = NLA_U32 },
[NL80211_ATTR_COOKIE] = { .type = NLA_U64 },
[NL80211_ATTR_TX_RATES] = { .type = NLA_NESTED },
@@ -448,7 +457,10 @@ const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_WDEV] = { .type = NLA_U64 },
[NL80211_ATTR_USER_REG_HINT_TYPE] = { .type = NLA_U32 },
[NL80211_ATTR_AUTH_DATA] = { .type = NLA_BINARY, },
- [NL80211_ATTR_VHT_CAPABILITY] = { .len = NL80211_VHT_CAPABILITY_LEN },
+ [NL80211_ATTR_VHT_CAPABILITY] = {
+ .type = NLA_EXACT_LEN_WARN,
+ .len = NL80211_VHT_CAPABILITY_LEN
+ },
[NL80211_ATTR_SCAN_FLAGS] = { .type = NLA_U32 },
[NL80211_ATTR_P2P_CTWINDOW] = NLA_POLICY_MAX(NLA_U8, 127),
[NL80211_ATTR_P2P_OPPPS] = NLA_POLICY_MAX(NLA_U8, 1),
@@ -484,7 +496,10 @@ const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_VENDOR_DATA] = { .type = NLA_BINARY },
[NL80211_ATTR_QOS_MAP] = { .type = NLA_BINARY,
.len = IEEE80211_QOS_MAP_LEN_MAX },
- [NL80211_ATTR_MAC_HINT] = { .len = ETH_ALEN },
+ [NL80211_ATTR_MAC_HINT] = {
+ .type = NLA_EXACT_LEN_WARN,
+ .len = ETH_ALEN
+ },
[NL80211_ATTR_WIPHY_FREQ_HINT] = { .type = NLA_U32 },
[NL80211_ATTR_TDLS_PEER_CAPABILITY] = { .type = NLA_U32 },
[NL80211_ATTR_SOCKET_OWNER] = { .type = NLA_FLAG },
@@ -495,7 +510,10 @@ const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
NLA_POLICY_MAX(NLA_U8, IEEE80211_NUM_UPS - 1),
[NL80211_ATTR_ADMITTED_TIME] = { .type = NLA_U16 },
[NL80211_ATTR_SMPS_MODE] = { .type = NLA_U8 },
- [NL80211_ATTR_MAC_MASK] = { .len = ETH_ALEN },
+ [NL80211_ATTR_MAC_MASK] = {
+ .type = NLA_EXACT_LEN_WARN,
+ .len = ETH_ALEN
+ },
[NL80211_ATTR_WIPHY_SELF_MANAGED_REG] = { .type = NLA_FLAG },
[NL80211_ATTR_NETNS_FD] = { .type = NLA_U32 },
[NL80211_ATTR_SCHED_SCAN_DELAY] = { .type = NLA_U32 },
@@ -507,15 +525,21 @@ const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_MU_MIMO_GROUP_DATA] = {
.len = VHT_MUMIMO_GROUPS_DATA_LEN
},
- [NL80211_ATTR_MU_MIMO_FOLLOW_MAC_ADDR] = { .len = ETH_ALEN },
+ [NL80211_ATTR_MU_MIMO_FOLLOW_MAC_ADDR] = {
+ .type = NLA_EXACT_LEN_WARN,
+ .len = ETH_ALEN
+ },
[NL80211_ATTR_NAN_MASTER_PREF] = NLA_POLICY_MIN(NLA_U8, 1),
[NL80211_ATTR_BANDS] = { .type = NLA_U32 },
[NL80211_ATTR_NAN_FUNC] = { .type = NLA_NESTED },
[NL80211_ATTR_FILS_KEK] = { .type = NLA_BINARY,
.len = FILS_MAX_KEK_LEN },
- [NL80211_ATTR_FILS_NONCES] = { .len = 2 * FILS_NONCE_LEN },
+ [NL80211_ATTR_FILS_NONCES] = {
+ .type = NLA_EXACT_LEN_WARN,
+ .len = 2 * FILS_NONCE_LEN
+ },
[NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED] = { .type = NLA_FLAG, },
- [NL80211_ATTR_BSSID] = { .len = ETH_ALEN },
+ [NL80211_ATTR_BSSID] = { .type = NLA_EXACT_LEN_WARN, .len = ETH_ALEN },
[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI] = { .type = NLA_S8 },
[NL80211_ATTR_SCHED_SCAN_RSSI_ADJUST] = {
.len = sizeof(struct nl80211_bss_select_rssi_adjust)
@@ -528,7 +552,7 @@ const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_FILS_ERP_NEXT_SEQ_NUM] = { .type = NLA_U16 },
[NL80211_ATTR_FILS_ERP_RRK] = { .type = NLA_BINARY,
.len = FILS_ERP_MAX_RRK_LEN },
- [NL80211_ATTR_FILS_CACHE_ID] = { .len = 2 },
+ [NL80211_ATTR_FILS_CACHE_ID] = { .type = NLA_EXACT_LEN_WARN, .len = 2 },
[NL80211_ATTR_PMK] = { .type = NLA_BINARY, .len = PMK_MAX_LEN },
[NL80211_ATTR_SCHED_SCAN_MULTI] = { .type = NLA_FLAG },
[NL80211_ATTR_EXTERNAL_AUTH_SUPPORT] = { .type = NLA_FLAG },
@@ -589,10 +613,13 @@ static const struct nla_policy
nl80211_wowlan_tcp_policy[NUM_NL80211_WOWLAN_TCP] = {
[NL80211_WOWLAN_TCP_SRC_IPV4] = { .type = NLA_U32 },
[NL80211_WOWLAN_TCP_DST_IPV4] = { .type = NLA_U32 },
- [NL80211_WOWLAN_TCP_DST_MAC] = { .len = ETH_ALEN },
+ [NL80211_WOWLAN_TCP_DST_MAC] = {
+ .type = NLA_EXACT_LEN_WARN,
+ .len = ETH_ALEN
+ },
[NL80211_WOWLAN_TCP_SRC_PORT] = { .type = NLA_U16 },
[NL80211_WOWLAN_TCP_DST_PORT] = { .type = NLA_U16 },
- [NL80211_WOWLAN_TCP_DATA_PAYLOAD] = { .len = 1 },
+ [NL80211_WOWLAN_TCP_DATA_PAYLOAD] = { .type = NLA_MIN_LEN, .len = 1 },
[NL80211_WOWLAN_TCP_DATA_PAYLOAD_SEQ] = {
.len = sizeof(struct nl80211_wowlan_tcp_data_seq)
},
@@ -600,8 +627,8 @@ nl80211_wowlan_tcp_policy[NUM_NL80211_WOWLAN_TCP] = {
.len = sizeof(struct nl80211_wowlan_tcp_data_token)
},
[NL80211_WOWLAN_TCP_DATA_INTERVAL] = { .type = NLA_U32 },
- [NL80211_WOWLAN_TCP_WAKE_PAYLOAD] = { .len = 1 },
- [NL80211_WOWLAN_TCP_WAKE_MASK] = { .len = 1 },
+ [NL80211_WOWLAN_TCP_WAKE_PAYLOAD] = { .type = NLA_MIN_LEN, .len = 1 },
+ [NL80211_WOWLAN_TCP_WAKE_MASK] = { .type = NLA_MIN_LEN, .len = 1 },
};
#endif /* CONFIG_PM */
@@ -619,9 +646,12 @@ nl80211_coalesce_policy[NUM_NL80211_ATTR_COALESCE_RULE] = {
/* policy for GTK rekey offload attributes */
static const struct nla_policy
nl80211_rekey_policy[NUM_NL80211_REKEY_DATA] = {
- [NL80211_REKEY_DATA_KEK] = { .len = NL80211_KEK_LEN },
- [NL80211_REKEY_DATA_KCK] = { .len = NL80211_KCK_LEN },
- [NL80211_REKEY_DATA_REPLAY_CTR] = { .len = NL80211_REPLAY_CTR_LEN },
+ [NL80211_REKEY_DATA_KEK] = { .type = NLA_EXACT_LEN_WARN, .len = NL80211_KEK_LEN },
+ [NL80211_REKEY_DATA_KCK] = { .type = NLA_EXACT_LEN_WARN, .len = NL80211_KCK_LEN },
+ [NL80211_REKEY_DATA_REPLAY_CTR] = {
+ .type = NLA_EXACT_LEN_WARN,
+ .len = NL80211_REPLAY_CTR_LEN
+ },
};
static const struct nla_policy
@@ -635,7 +665,10 @@ static const struct nla_policy
nl80211_match_policy[NL80211_SCHED_SCAN_MATCH_ATTR_MAX + 1] = {
[NL80211_SCHED_SCAN_MATCH_ATTR_SSID] = { .type = NLA_BINARY,
.len = IEEE80211_MAX_SSID_LEN },
- [NL80211_SCHED_SCAN_MATCH_ATTR_BSSID] = { .len = ETH_ALEN },
+ [NL80211_SCHED_SCAN_MATCH_ATTR_BSSID] = {
+ .type = NLA_EXACT_LEN_WARN,
+ .len = ETH_ALEN
+ },
[NL80211_SCHED_SCAN_MATCH_ATTR_RSSI] = { .type = NLA_U32 },
[NL80211_SCHED_SCAN_MATCH_PER_BAND_RSSI] =
NLA_POLICY_NESTED(nl80211_match_band_rssi_policy),
@@ -667,7 +700,10 @@ nl80211_nan_func_policy[NL80211_NAN_FUNC_ATTR_MAX + 1] = {
[NL80211_NAN_FUNC_SUBSCRIBE_ACTIVE] = { .type = NLA_FLAG },
[NL80211_NAN_FUNC_FOLLOW_UP_ID] = { .type = NLA_U8 },
[NL80211_NAN_FUNC_FOLLOW_UP_REQ_ID] = { .type = NLA_U8 },
- [NL80211_NAN_FUNC_FOLLOW_UP_DEST] = { .len = ETH_ALEN },
+ [NL80211_NAN_FUNC_FOLLOW_UP_DEST] = {
+ .type = NLA_EXACT_LEN_WARN,
+ .len = ETH_ALEN
+ },
[NL80211_NAN_FUNC_CLOSE_RANGE] = { .type = NLA_FLAG },
[NL80211_NAN_FUNC_TTL] = { .type = NLA_U32 },
[NL80211_NAN_FUNC_SERVICE_INFO] = { .type = NLA_BINARY,
@@ -4057,7 +4093,7 @@ static const struct nla_policy nl80211_txattr_policy[NL80211_TXRATE_MAX + 1] = {
.len = NL80211_MAX_SUPP_RATES },
[NL80211_TXRATE_HT] = { .type = NLA_BINARY,
.len = NL80211_MAX_SUPP_HT_RATES },
- [NL80211_TXRATE_VHT] = { .len = sizeof(struct nl80211_txrate_vht)},
+ [NL80211_TXRATE_VHT] = { .type = NLA_EXACT_LEN_WARN, .len = sizeof(struct nl80211_txrate_vht)},
[NL80211_TXRATE_GI] = { .type = NLA_U8 },
};
--
2.17.2
^ permalink raw reply related
* Re: [PATCH 1/3] brcmfmac: re-enable command decode in sdio_aos for BRCM 4354
From: Kalle Valo @ 2019-05-28 12:18 UTC (permalink / raw)
To: Douglas Anderson
Cc: Ulf Hansson, Adrian Hunter, Arend van Spriel, linux-rockchip,
Double Lo, briannorris, Madhan Mohan R, mka, Wright Feng,
Chi-Hsien Lin, Douglas Anderson, brcm80211-dev-list.pdl,
Franky Lin, netdev, linux-wireless, linux-kernel, Madhan Mohan R,
Hante Meuleman, Naveen Gupta, brcm80211-dev-list, YueHaibing,
David S. Miller
In-Reply-To: <20190517225420.176893-2-dianders@chromium.org>
Douglas Anderson <dianders@chromium.org> wrote:
> In commit 29f6589140a1 ("brcmfmac: disable command decode in
> sdio_aos") we disabled something called "command decode in sdio_aos"
> for a whole bunch of Broadcom SDIO WiFi parts.
>
> After that patch landed I find that my kernel log on
> rk3288-veyron-minnie and rk3288-veyron-speedy is filled with:
> brcmfmac: brcmf_sdio_bus_sleep: error while changing bus sleep state -110
>
> This seems to happen every time the Broadcom WiFi transitions out of
> sleep mode. Reverting the part of the commit that affects the WiFi on
> my boards fixes the problem for me, so that's what this patch does.
>
> Note that, in general, the justification in the original commit seemed
> a little weak. It looked like someone was testing on a SD card
> controller that would sometimes die if there were CRC errors on the
> bus. This used to happen back in early days of dw_mmc (the controller
> on my boards), but we fixed it. Disabling a feature on all boards
> just because one SD card controller is broken seems bad. ...so
> instead of just this patch possibly the right thing to do is to fully
> revert the original commit.
>
> Fixes: 29f6589140a1 ("brcmfmac: disable command decode in sdio_aos")
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
I don't see patch 2 in patchwork and I assume discussion continues.
Please resend if/when I need to apply something.
2 patches set to Changes Requested.
10948785 [1/3] brcmfmac: re-enable command decode in sdio_aos for BRCM 4354
10948777 [3/3] brcmfmac: sdio: Disable auto-tuning around commands expected to fail
--
https://patchwork.kernel.org/patch/10948785/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
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