* [PATCH V2] Fix a sleep-in-atomic bug in shash_setkey_unaligned
From: Jia-Ju Bai @ 2017-10-03 2:25 UTC (permalink / raw)
To: davem, herbert, nhorman, vyasevich, luto, kvalo
Cc: linux-crypto, netdev, linux-sctp, linux-wireless, Jia-Ju Bai
The SCTP program may sleep under a spinlock, and the function call path is:
sctp_generate_t3_rtx_event (acquire the spinlock)
sctp_do_sm
sctp_side_effects
sctp_cmd_interpreter
sctp_make_init_ack
sctp_pack_cookie
crypto_shash_setkey
shash_setkey_unaligned
kmalloc(GFP_KERNEL)
For the same reason, the orinoco driver may sleep in interrupt handler,
and the function call path is:
orinoco_rx_isr_tasklet
orinoco_rx
orinoco_mic
crypto_shash_setkey
shash_setkey_unaligned
kmalloc(GFP_KERNEL)
To fix it, GFP_KERNEL is replaced with GFP_ATOMIC.
This bug is found by my static analysis tool and my code review.
Signed-off-by: Jia-Ju Bai <baijiaju1990@163.com>
---
crypto/shash.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/crypto/shash.c b/crypto/shash.c
index 5e31c8d..8fcecc6 100644
--- a/crypto/shash.c
+++ b/crypto/shash.c
@@ -41,7 +41,7 @@ static int shash_setkey_unaligned(struct crypto_shash *tfm, const u8 *key,
int err;
absize = keylen + (alignmask & ~(crypto_tfm_ctx_alignment() - 1));
- buffer = kmalloc(absize, GFP_KERNEL);
+ buffer = kmalloc(absize, GFP_ATOMIC);
if (!buffer)
return -ENOMEM;
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH V2] Fix a sleep-in-atomic bug in shash_setkey_unaligned
From: Andy Lutomirski @ 2017-10-03 4:18 UTC (permalink / raw)
To: Jia-Ju Bai
Cc: David S. Miller, Herbert Xu, Neil Horman, vyasevich,
Andrew Lutomirski, Kalle Valo, Linux Crypto Mailing List,
Network Development, linux-sctp, Linux Wireless List
In-Reply-To: <1506997522-26684-1-git-send-email-baijiaju1990@163.com>
> On Oct 2, 2017, at 7:25 PM, Jia-Ju Bai <baijiaju1990@163.com> wrote:
>
> The SCTP program may sleep under a spinlock, and the function call path is:
> sctp_generate_t3_rtx_event (acquire the spinlock)
> sctp_do_sm
> sctp_side_effects
> sctp_cmd_interpreter
> sctp_make_init_ack
> sctp_pack_cookie
> crypto_shash_setkey
> shash_setkey_unaligned
> kmalloc(GFP_KERNEL)
>
I'm going to go out on a limb here: why on Earth is out crypto API so
full of indirection that we allocate memory at all here?
We're synchronously computing a hash of a small amount of data using
either HMAC-SHA1 or HMAC-SHA256 (determined at runtime) if I read it
right. There's a sane way to do this that doesn't need kmalloc,
alloca, or fancy indirection. And then there's crypto_shash_xyz().
--Andy, who is sick of seeing stupid bugs caused by the fact that it's
basically impossible to use the crypto API in a sane way.
P.S. gnulib has:
int hmac_sha256 (const void *key, size_t keylen, const void *in,
size_t inlen, void *resbuf);
An init/update/final-style API would be nice, but something like this
would be a phenomenal improvement over what we have.
^ permalink raw reply
* Re: [PATCH V2] Fix a sleep-in-atomic bug in shash_setkey_unaligned
From: Herbert Xu @ 2017-10-03 5:26 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Jia-Ju Bai, David S. Miller, Neil Horman, vyasevich, Kalle Valo,
Linux Crypto Mailing List, Network Development, linux-sctp,
Linux Wireless List
In-Reply-To: <CALCETrWdXjTTTywbb3duCEsLYNxkeGx7bf3SM4PYKeErCyiUNQ@mail.gmail.com>
On Mon, Oct 02, 2017 at 09:18:24PM -0700, Andy Lutomirski wrote:
> > On Oct 2, 2017, at 7:25 PM, Jia-Ju Bai <baijiaju1990@163.com> wrote:
> >
> > The SCTP program may sleep under a spinlock, and the function call path is:
> > sctp_generate_t3_rtx_event (acquire the spinlock)
> > sctp_do_sm
> > sctp_side_effects
> > sctp_cmd_interpreter
> > sctp_make_init_ack
> > sctp_pack_cookie
> > crypto_shash_setkey
> > shash_setkey_unaligned
> > kmalloc(GFP_KERNEL)
>
> I'm going to go out on a limb here: why on Earth is out crypto API so
> full of indirection that we allocate memory at all here?
The crypto API operates on a one key per-tfm basis. So normally
tfm allocation and key setting is done once only and not done on
the data path.
I have looked at the SCTP code and it appears to fit this paradigm.
That is, we should be able to allocate the tfm and set the key when
the key is actually generated via get_random_bytes, rather than every
time the key is used which is not only a waste but as you see runs
into API issues.
Usually if you're invoking setkey from a non-sleeping code-path
you're probably doing something wrong.
As someone else noted recently, there is no single forum for
reviewing code that uses the crypto API so buggy code like this
is not surprising.
> We're synchronously computing a hash of a small amount of data using
> either HMAC-SHA1 or HMAC-SHA256 (determined at runtime) if I read it
> right. There's a sane way to do this that doesn't need kmalloc,
> alloca, or fancy indirection. And then there's crypto_shash_xyz().
There are some legitimate cases where you want to use a different
key for every hashing operation. But so far these are uses have
been very few so there has been no need to provide an API for them.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] cfg80211/nl80211: add a port authorized event
From: Johannes Berg @ 2017-10-03 7:37 UTC (permalink / raw)
To: Arend van Spriel, linux-wireless; +Cc: Avraham Stern, luca
In-Reply-To: <0c8ca5c1-eabd-2cfa-ec5c-70dc0dfac61b@broadcom.com>
On Mon, 2017-10-02 at 23:09 +0200, Arend van Spriel wrote:
> On 29-09-17 14:21, Johannes Berg wrote:
> > From: Avraham Stern <avraham.stern@intel.com>
> >
> > Add an event that indicates that a connection is authorized
> > (i.e. the 4 way handshake was performed by the driver). This event
> > should be sent by the driver after sending a connect/roamed event.
>
> So is this event required for drivers supporting 4-way handshake
> offload. If so, the "should" above might need to be "shall" and I
> have some changes to do in brcmfmac ;-)
I'm not sure it's *required*? I guess it would be for 802.1X on the
host/4-way-HS in the device, but not necessarily for the other cases?
johannes
^ permalink raw reply
* Re: [PATCH 05/18] net: use ARRAY_SIZE
From: Andy Shevchenko @ 2017-10-03 8:09 UTC (permalink / raw)
To: Jérémy Lefaure
Cc: Sathya Perla, Ajit Khaparde, Sriharsha Basavapatna, Somnath Kotur,
Jeff Kirsher, Arend van Spriel, Franky Lin, Hante Meuleman,
Chi-Hsien Lin, Wright Feng, Kalle Valo, Larry Finger, Chaoming Li,
David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev,
linux-kernel@vger.kernel.org, intel-wired-lan, USB,
open list:TI WILINK WIRELES...,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
brcm80211-dev-list
In-Reply-To: <20171002212214.30d7013a@blatinox-laptop.localdomain>
On Tue, Oct 3, 2017 at 4:22 AM, J=C3=A9r=C3=A9my Lefaure
<jeremy.lefaure@lse.epita.fr> wrote:
> On Mon, 2 Oct 2017 16:07:36 +0300
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>
>> > + {&gainctrl_lut_core0_rev0, ARRAY_SIZE(gainctrl_lut_core0_rev0)=
, 26, 192,
>> > + 32},
>>
>> For all such cases I would rather put on one line disregard checkpatch
>> warning for better readability.
> I agree that it would be better. I didn't know that it was possible to
> not follow this rule for anything else than a string.
IMO, it increases readability quite enough to overrule checkpatch recomenda=
tion.
--=20
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH 1/2] mwifiex: kill useless list_empty checks
From: Ganapathi Bhat @ 2017-10-03 15:19 UTC (permalink / raw)
To: linux-wireless
Cc: Brian Norris, Cathy Luo, Xinming Hu, Zhiyuan Yang, James Cao,
Mangesh Malusare, Douglas Anderson, Ganapathi Bhat
From: Douglas Anderson <dianders@chromium.org>
There's absolutely no reason to check to see if a list is empty
before iterating through it. It's just like writing code like
this:
if (count != 0) {
for (i = 0; i < count; i++) {
...
}
}
The loop will already be avoided if "count == 0" so there was no
reason to check.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
---
drivers/net/wireless/marvell/mwifiex/11n.c | 9 ---------
drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c | 6 ------
drivers/net/wireless/marvell/mwifiex/init.c | 4 ----
drivers/net/wireless/marvell/mwifiex/tdls.c | 7 -------
4 files changed, 26 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/11n.c b/drivers/net/wireless/marvell/mwifiex/11n.c
index 7252069..8772e39 100644
--- a/drivers/net/wireless/marvell/mwifiex/11n.c
+++ b/drivers/net/wireless/marvell/mwifiex/11n.c
@@ -658,12 +658,6 @@ void mwifiex_11n_delba(struct mwifiex_private *priv, int tid)
unsigned long flags;
spin_lock_irqsave(&priv->rx_reorder_tbl_lock, flags);
- if (list_empty(&priv->rx_reorder_tbl_ptr)) {
- dev_dbg(priv->adapter->dev,
- "mwifiex_11n_delba: rx_reorder_tbl_ptr empty\n");
- goto exit;
- }
-
list_for_each_entry(rx_reor_tbl_ptr, &priv->rx_reorder_tbl_ptr, list) {
if (rx_reor_tbl_ptr->tid == tid) {
dev_dbg(priv->adapter->dev,
@@ -854,9 +848,6 @@ u8 mwifiex_get_sec_chan_offset(int chan)
struct mwifiex_adapter *adapter = priv->adapter;
struct mwifiex_tx_ba_stream_tbl *tx_ba_stream_tbl_ptr;
- if (list_empty(&priv->tx_ba_stream_tbl_ptr))
- return;
-
list_for_each_entry(tx_ba_stream_tbl_ptr,
&priv->tx_ba_stream_tbl_ptr, list) {
if (tx_ba_stream_tbl_ptr->ba_status == BA_SETUP_COMPLETE) {
diff --git a/drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c b/drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c
index 274dd5a..d87df2d 100644
--- a/drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c
+++ b/drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c
@@ -835,12 +835,6 @@ void mwifiex_update_rxreor_flags(struct mwifiex_adapter *adapter, u8 flags)
continue;
spin_lock_irqsave(&priv->rx_reorder_tbl_lock, lock_flags);
- if (list_empty(&priv->rx_reorder_tbl_ptr)) {
- spin_unlock_irqrestore(&priv->rx_reorder_tbl_lock,
- lock_flags);
- continue;
- }
-
list_for_each_entry(tbl, &priv->rx_reorder_tbl_ptr, list)
tbl->flags = flags;
spin_unlock_irqrestore(&priv->rx_reorder_tbl_lock, lock_flags);
diff --git a/drivers/net/wireless/marvell/mwifiex/init.c b/drivers/net/wireless/marvell/mwifiex/init.c
index e11919d..1176706 100644
--- a/drivers/net/wireless/marvell/mwifiex/init.c
+++ b/drivers/net/wireless/marvell/mwifiex/init.c
@@ -579,10 +579,6 @@ static void mwifiex_delete_bss_prio_tbl(struct mwifiex_private *priv)
{
spin_lock_irqsave(lock, flags);
- if (list_empty(head)) {
- spin_unlock_irqrestore(lock, flags);
- continue;
- }
list_for_each_entry_safe(bssprio_node, tmp_node, head,
list) {
if (bssprio_node->priv == priv) {
diff --git a/drivers/net/wireless/marvell/mwifiex/tdls.c b/drivers/net/wireless/marvell/mwifiex/tdls.c
index e76af286..9fe0bae 100644
--- a/drivers/net/wireless/marvell/mwifiex/tdls.c
+++ b/drivers/net/wireless/marvell/mwifiex/tdls.c
@@ -1413,13 +1413,6 @@ void mwifiex_check_auto_tdls(unsigned long context)
priv->check_tdls_tx = false;
- if (list_empty(&priv->auto_tdls_list)) {
- mod_timer(&priv->auto_tdls_timer,
- jiffies +
- msecs_to_jiffies(MWIFIEX_TIMER_10S));
- return;
- }
-
spin_lock_irqsave(&priv->auto_tdls_lock, flags);
list_for_each_entry(tdls_peer, &priv->auto_tdls_list, list) {
if ((jiffies - tdls_peer->rssi_jiffies) >
--
1.9.1
^ permalink raw reply related
* [PATCH 2/2] mwifiex: minor cleanups w/ sta_list_spinlock in cfg80211.c
From: Ganapathi Bhat @ 2017-10-03 15:19 UTC (permalink / raw)
To: linux-wireless
Cc: Brian Norris, Cathy Luo, Xinming Hu, Zhiyuan Yang, James Cao,
Mangesh Malusare, Douglas Anderson, Ganapathi Bhat
In-Reply-To: <1507043984-9832-1-git-send-email-gbhat@marvell.com>
From: Douglas Anderson <dianders@chromium.org>
The sta_list_spinlock looks to be used to control locking of the
list. Specifically when someone has the lock they may be allowed
to modify or delete elements of the list.
That implies that we shouldn't access the fields of the elements
returned by mwifiex_get_sta_entry() after we've released the
spinlock. Let's make some small changes so this is true.
It's unlikely that this matters since it looks to be just error
handling, but it's nice to be clean.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
---
drivers/net/wireless/marvell/mwifiex/cfg80211.c | 14 +++++++++-----
drivers/net/wireless/marvell/mwifiex/sta_event.c | 6 ++----
2 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index cc7d777..3638b613 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -3794,9 +3794,8 @@ static int mwifiex_cfg80211_set_coalesce(struct wiphy *wiphy,
spin_lock_irqsave(&priv->sta_list_spinlock, flags);
sta_ptr = mwifiex_get_sta_entry(priv, addr);
- spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
-
if (!sta_ptr) {
+ spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
wiphy_err(wiphy, "%s: Invalid TDLS peer %pM\n",
__func__, addr);
return -ENOENT;
@@ -3804,15 +3803,18 @@ static int mwifiex_cfg80211_set_coalesce(struct wiphy *wiphy,
if (!(sta_ptr->tdls_cap.extcap.ext_capab[3] &
WLAN_EXT_CAPA4_TDLS_CHAN_SWITCH)) {
+ spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
wiphy_err(wiphy, "%pM do not support tdls cs\n", addr);
return -ENOENT;
}
if (sta_ptr->tdls_status == TDLS_CHAN_SWITCHING ||
sta_ptr->tdls_status == TDLS_IN_OFF_CHAN) {
+ spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
wiphy_err(wiphy, "channel switch is running, abort request\n");
return -EALREADY;
}
+ spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
chan = chandef->chan->hw_value;
second_chan_offset = mwifiex_get_sec_chan_offset(chan);
@@ -3833,18 +3835,20 @@ static int mwifiex_cfg80211_set_coalesce(struct wiphy *wiphy,
spin_lock_irqsave(&priv->sta_list_spinlock, flags);
sta_ptr = mwifiex_get_sta_entry(priv, addr);
- spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
-
if (!sta_ptr) {
+ spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
wiphy_err(wiphy, "%s: Invalid TDLS peer %pM\n",
__func__, addr);
} else if (!(sta_ptr->tdls_status == TDLS_CHAN_SWITCHING ||
sta_ptr->tdls_status == TDLS_IN_BASE_CHAN ||
sta_ptr->tdls_status == TDLS_IN_OFF_CHAN)) {
+ spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
wiphy_err(wiphy, "tdls chan switch not initialize by %pM\n",
addr);
- } else
+ } else {
+ spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
mwifiex_stop_tdls_cs(priv, addr);
+ }
}
static int
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_event.c b/drivers/net/wireless/marvell/mwifiex/sta_event.c
index 839df8a..d8db412 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_event.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_event.c
@@ -359,13 +359,12 @@ static void mwifiex_process_uap_tx_pause(struct mwifiex_private *priv,
} else {
spin_lock_irqsave(&priv->sta_list_spinlock, flags);
sta_ptr = mwifiex_get_sta_entry(priv, tp->peermac);
- spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
-
if (sta_ptr && sta_ptr->tx_pause != tp->tx_pause) {
sta_ptr->tx_pause = tp->tx_pause;
mwifiex_update_ralist_tx_pause(priv, tp->peermac,
tp->tx_pause);
}
+ spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
}
}
@@ -396,14 +395,13 @@ static void mwifiex_process_sta_tx_pause(struct mwifiex_private *priv,
if (mwifiex_is_tdls_link_setup(status)) {
spin_lock_irqsave(&priv->sta_list_spinlock, flags);
sta_ptr = mwifiex_get_sta_entry(priv, tp->peermac);
- spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
-
if (sta_ptr && sta_ptr->tx_pause != tp->tx_pause) {
sta_ptr->tx_pause = tp->tx_pause;
mwifiex_update_ralist_tx_pause(priv,
tp->peermac,
tp->tx_pause);
}
+ spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
}
}
}
--
1.9.1
^ permalink raw reply related
* Re: [PATCH V2] Fix a sleep-in-atomic bug in shash_setkey_unaligned
From: Andy Lutomirski @ 2017-10-03 16:46 UTC (permalink / raw)
To: Herbert Xu
Cc: Andy Lutomirski, Jia-Ju Bai, David S. Miller, Neil Horman,
vyasevich, Kalle Valo, Linux Crypto Mailing List,
Network Development, linux-sctp, Linux Wireless List
In-Reply-To: <20171003052643.GB22750@gondor.apana.org.au>
On Mon, Oct 2, 2017 at 10:26 PM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Mon, Oct 02, 2017 at 09:18:24PM -0700, Andy Lutomirski wrote:
>> > On Oct 2, 2017, at 7:25 PM, Jia-Ju Bai <baijiaju1990@163.com> wrote:
>> >
>> > The SCTP program may sleep under a spinlock, and the function call path is:
>> > sctp_generate_t3_rtx_event (acquire the spinlock)
>> > sctp_do_sm
>> > sctp_side_effects
>> > sctp_cmd_interpreter
>> > sctp_make_init_ack
>> > sctp_pack_cookie
>> > crypto_shash_setkey
>> > shash_setkey_unaligned
>> > kmalloc(GFP_KERNEL)
>>
>> I'm going to go out on a limb here: why on Earth is out crypto API so
>> full of indirection that we allocate memory at all here?
>
> The crypto API operates on a one key per-tfm basis. So normally
> tfm allocation and key setting is done once only and not done on
> the data path.
>
> I have looked at the SCTP code and it appears to fit this paradigm.
> That is, we should be able to allocate the tfm and set the key when
> the key is actually generated via get_random_bytes, rather than every
> time the key is used which is not only a waste but as you see runs
> into API issues.
It's a waste because it loses a pre-computation advantage.
The fact that it has memory allocation issues is crypto API's fault,
full stop. There is no legit reason to need to allocate anything.
^ permalink raw reply
* [PATCH v3 0/2] Allwinner XR819 SDIO Wi-Fi DT binding and OPi Zero XR819 IRQ
From: Icenowy Zheng @ 2017-10-03 16:59 UTC (permalink / raw)
To: Kalle Valo, Rob Herring, Maxime Ripard, Chen-Yu Tsai
Cc: linux-wireless, netdev, devicetree, linux-kernel,
linux-arm-kernel, linux-sunxi, Icenowy Zheng
The Allwinner XR819 SDIO Wi-Fi chip supports an out-of-band interrupt line,
and the in-band interrupt is also supported.
However the current out-of-tree driver uses the out-of-band interrupt by
default.
This patchset adds the device tree binding for the chip as well as the
out-of-band interrupt, then adds the interrupt to the device tree of
Orange Pi Zero.
Icenowy Zheng (1):
dt-bindings: add device tree binding for Allwinner XR819 SDIO Wi-Fi
Sergey Matyukevich (1):
ARM: sun8i: h2+: specify wifi interrupts for Orange Pi Zero
.../bindings/net/wireless/allwinner,xr819.txt | 38 ++++++++++++++++++++++
arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts | 3 ++
2 files changed, 41 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/wireless/allwinner,xr819.txt
--
2.13.5
^ permalink raw reply
* [PATCH v3 1/2] dt-bindings: add device tree binding for Allwinner XR819 SDIO Wi-Fi
From: Icenowy Zheng @ 2017-10-03 16:59 UTC (permalink / raw)
To: Kalle Valo, Rob Herring, Maxime Ripard, Chen-Yu Tsai
Cc: linux-wireless, netdev, devicetree, linux-kernel,
linux-arm-kernel, linux-sunxi, Icenowy Zheng
In-Reply-To: <20171003165944.13056-1-icenowy@aosc.io>
Allwinner XR819 is a SDIO Wi-Fi chip, which has the functionality to use
an out-of-band interrupt pin instead of SDIO in-band interrupt.
Add the device tree binding of this chip, in order to make it possible
to add this interrupt pin to device trees.
Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
Acked-by: Rob Herring <robh@kernel.org>
---
Changes in v3:
- Renames the node name.
- Adds ACK from Rob.
Changes in v2:
- Removed status property in example.
- Added required property reg.
.../bindings/net/wireless/allwinner,xr819.txt | 38 ++++++++++++++++++++++
1 file changed, 38 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/wireless/allwinner,xr819.txt
diff --git a/Documentation/devicetree/bindings/net/wireless/allwinner,xr819.txt b/Documentation/devicetree/bindings/net/wireless/allwinner,xr819.txt
new file mode 100644
index 000000000000..7ae40441e343
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/allwinner,xr819.txt
@@ -0,0 +1,38 @@
+Allwinner XRadio wireless SDIO devices
+
+This node provides properties for controlling the XRadio wireless device. The
+node is expected to be specified as a child node to the SDIO controller that
+connects the device to the system.
+
+Required properties:
+
+ - reg : The SDIO function number, see "Use of function subnodes" in
+ ../../mmc/mmc.txt.
+ - compatible : Should be "allwinner,xr819".
+
+Optional properties:
+ - interrupt-parent : the phandle for the interrupt controller to which the
+ device interrupts are connected.
+ - interrupts : specifies attributes for the out-of-band interrupt (host-wake).
+ When not specified the device will use in-band SDIO interrupts.
+
+Example:
+
+mmc1: mmc@01c10000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&mmc1_pins_a>;
+ vmmc-supply = <®_vcc_wifi>;
+ mmc-pwrseq = <&wifi_pwrseq>;
+ bus-width = <4>;
+ non-removable;
+
+ xr819: wifi@1 {
+ reg = <1>;
+ compatible = "allwinner,xr819";
+ interrupt-parent = <&pio>;
+ interrupts = <6 10 IRQ_TYPE_EDGE_RISING>;
+ };
+};
--
2.13.5
^ permalink raw reply related
* [PATCH v3 2/2] ARM: sun8i: h2+: specify wifi interrupts for Orange Pi Zero
From: Icenowy Zheng @ 2017-10-03 16:59 UTC (permalink / raw)
To: Kalle Valo, Rob Herring, Maxime Ripard, Chen-Yu Tsai
Cc: linux-wireless, netdev, devicetree, linux-kernel,
linux-arm-kernel, linux-sunxi, Sergey Matyukevich, Icenowy Zheng
In-Reply-To: <20171003165944.13056-1-icenowy@aosc.io>
From: Sergey Matyukevich <geomatsi@gmail.com>
The Orange Pi Zero board has Allwinner XR819 SDIO wifi chip. The board
dts file provides a node enabling mmc1 controller, and a out-of-band
interrupt line of the chip is also connected, although the chip also
supports in-band interrupt.
The current out-of-tree driver is hardcoded to use out-of-band interrupt
as default, and it needs to be modified to use the in-band interrupt.
This commit adds the out-of-band interrupt line into the device tree.
Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
[Icenowy: Changed vendor prefix to allwinner and modify commit message]
Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
Changes in v3 by Icenowy:
- Change the compatible string vendor prefix to "allwinner".
- Modify the commit message.
Changes in v2 by Sergey:
- Adds the compatible string.
arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts b/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts
index b1502df7b509..6595617204b3 100644
--- a/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts
+++ b/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts
@@ -127,6 +127,9 @@
*/
xr819: sdio_wifi@1 {
reg = <1>;
+ compatible = "allwinner,xr819";
+ interrupt-parent = <&pio>;
+ interrupts = <6 10 IRQ_TYPE_EDGE_RISING>;
};
};
--
2.13.5
^ permalink raw reply related
* lockdep splat on 4.13.3 (plus hacks)
From: Ben Greear @ 2017-10-03 18:17 UTC (permalink / raw)
To: linux-wireless@vger.kernel.org
We are seeing deadlocks related to wifi in our 4.13.3+ kernels,
so I enabled lockdep and immediately saw this. Anyone know if this
is a known issue? Otherwise, I guess it could be related to some local
patch I have added...
[ 476.172823] ============================================
[ 476.176863] WARNING: possible recursive locking detected
[ 476.180895] 4.13.3+ #1 Not tainted
[ 476.183025] --------------------------------------------
[ 476.187053] kworker/u8:2/281 is trying to acquire lock:
[ 476.190993] (&sta->ampdu_mlme.mtx){+.+...}, at: [<ffffffffa09cd4e8>] __ieee80211_start_rx_ba_session+0x178/0x670 [mac80211]
[ 476.201004]
but task is already holding lock:
[ 476.204270] (&sta->ampdu_mlme.mtx){+.+...}, at: [<ffffffffa09c93a6>] ieee80211_ba_session_work+0x46/0x2b0 [mac80211]
[ 476.213645]
other info that might help us debug this:
[ 476.217587] Possible unsafe locking scenario:
[ 476.220930] CPU0
[ 476.222082] ----
[ 476.223236] lock(&sta->ampdu_mlme.mtx);
[ 476.225957] lock(&sta->ampdu_mlme.mtx);
[ 476.228673]
*** DEADLOCK ***
[ 476.230689] May be due to missing lock nesting notation
[ 476.234879] 3 locks held by kworker/u8:2/281:
[ 476.237941] #0: ("%s"wiphy_name(local->hw.wiphy)){++++.+}, at: [<ffffffff8113df8f>] process_one_work+0x14f/0x6a0
[ 476.247033] #1: ((&sta->ampdu_mlme.work)){+.+...}, at: [<ffffffff8113df8f>] process_one_work+0x14f/0x6a0
[ 476.255393] #2: (&sta->ampdu_mlme.mtx){+.+...}, at: [<ffffffffa09c93a6>] ieee80211_ba_session_work+0x46/0x2b0 [mac80211]
[ 476.265170]
stack backtrace:
[ 476.266928] CPU: 0 PID: 281 Comm: kworker/u8:2 Not tainted 4.13.3+ #1
[ 476.272073] Hardware name: _ _/ , BIOS 5.11 08/26/2016
[ 476.275927] Workqueue: phy1 ieee80211_ba_session_work [mac80211]
[ 476.280640] Call Trace:
[ 476.281792] dump_stack+0x85/0xc7
[ 476.283811] __lock_acquire+0x14ba/0x1520
[ 476.286526] ? __save_stack_trace+0x6e/0xd0
[ 476.289412] ? ret_from_fork+0x2a/0x40
[ 476.291867] lock_acquire+0xac/0x200
[ 476.294145] ? lock_acquire+0xac/0x200
[ 476.296610] ? __ieee80211_start_rx_ba_session+0x178/0x670 [mac80211]
[ 476.301766] ? __ieee80211_start_rx_ba_session+0x178/0x670 [mac80211]
[ 476.306910] __mutex_lock+0x69/0x930
[ 476.309194] ? __ieee80211_start_rx_ba_session+0x178/0x670 [mac80211]
[ 476.314343] ? rcu_read_lock_sched_held+0x6d/0x80
[ 476.317766] ? __sdata_dbg+0x14a/0x1a0 [mac80211]
[ 476.321181] mutex_lock_nested+0x16/0x20
[ 476.323810] ? mutex_lock_nested+0x16/0x20
[ 476.326626] __ieee80211_start_rx_ba_session+0x178/0x670 [mac80211]
[ 476.331627] ieee80211_ba_session_work+0x157/0x2b0 [mac80211]
[ 476.336078] ? process_one_work+0x14f/0x6a0
[ 476.338985] process_one_work+0x1ce/0x6a0
[ 476.341696] worker_thread+0x46/0x400
[ 476.344061] kthread+0x10f/0x150
[ 476.346001] ? process_one_work+0x6a0/0x6a0
[ 476.348886] ? kthread_create_on_node+0x40/0x40
[ 476.352122] ret_from_fork+0x2a/0x40
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: lockdep splat on 4.13.3 (plus hacks)
From: Ben Greear @ 2017-10-03 18:50 UTC (permalink / raw)
To: linux-wireless@vger.kernel.org
In-Reply-To: <c54e3dd0-8e6e-41f2-cd0f-960cb623e8fa@candelatech.com>
On 10/03/2017 11:17 AM, Ben Greear wrote:
> We are seeing deadlocks related to wifi in our 4.13.3+ kernels,
> so I enabled lockdep and immediately saw this. Anyone know if this
> is a known issue? Otherwise, I guess it could be related to some local
> patch I have added...
I think I found the fix in the stable queue, so I guess it will be in
4.13.5 when that is out...
Will continue testing...
Thanks,
Ben
>
>
> [ 476.172823] ============================================
> [ 476.176863] WARNING: possible recursive locking detected
> [ 476.180895] 4.13.3+ #1 Not tainted
> [ 476.183025] --------------------------------------------
> [ 476.187053] kworker/u8:2/281 is trying to acquire lock:
> [ 476.190993] (&sta->ampdu_mlme.mtx){+.+...}, at: [<ffffffffa09cd4e8>] __ieee80211_start_rx_ba_session+0x178/0x670 [mac80211]
> [ 476.201004]
> but task is already holding lock:
> [ 476.204270] (&sta->ampdu_mlme.mtx){+.+...}, at: [<ffffffffa09c93a6>] ieee80211_ba_session_work+0x46/0x2b0 [mac80211]
> [ 476.213645]
> other info that might help us debug this:
> [ 476.217587] Possible unsafe locking scenario:
>
> [ 476.220930] CPU0
> [ 476.222082] ----
> [ 476.223236] lock(&sta->ampdu_mlme.mtx);
> [ 476.225957] lock(&sta->ampdu_mlme.mtx);
> [ 476.228673]
> *** DEADLOCK ***
>
> [ 476.230689] May be due to missing lock nesting notation
>
> [ 476.234879] 3 locks held by kworker/u8:2/281:
> [ 476.237941] #0: ("%s"wiphy_name(local->hw.wiphy)){++++.+}, at: [<ffffffff8113df8f>] process_one_work+0x14f/0x6a0
> [ 476.247033] #1: ((&sta->ampdu_mlme.work)){+.+...}, at: [<ffffffff8113df8f>] process_one_work+0x14f/0x6a0
> [ 476.255393] #2: (&sta->ampdu_mlme.mtx){+.+...}, at: [<ffffffffa09c93a6>] ieee80211_ba_session_work+0x46/0x2b0 [mac80211]
> [ 476.265170]
> stack backtrace:
> [ 476.266928] CPU: 0 PID: 281 Comm: kworker/u8:2 Not tainted 4.13.3+ #1
> [ 476.272073] Hardware name: _ _/ , BIOS 5.11 08/26/2016
> [ 476.275927] Workqueue: phy1 ieee80211_ba_session_work [mac80211]
> [ 476.280640] Call Trace:
> [ 476.281792] dump_stack+0x85/0xc7
> [ 476.283811] __lock_acquire+0x14ba/0x1520
> [ 476.286526] ? __save_stack_trace+0x6e/0xd0
> [ 476.289412] ? ret_from_fork+0x2a/0x40
> [ 476.291867] lock_acquire+0xac/0x200
> [ 476.294145] ? lock_acquire+0xac/0x200
> [ 476.296610] ? __ieee80211_start_rx_ba_session+0x178/0x670 [mac80211]
> [ 476.301766] ? __ieee80211_start_rx_ba_session+0x178/0x670 [mac80211]
> [ 476.306910] __mutex_lock+0x69/0x930
> [ 476.309194] ? __ieee80211_start_rx_ba_session+0x178/0x670 [mac80211]
> [ 476.314343] ? rcu_read_lock_sched_held+0x6d/0x80
> [ 476.317766] ? __sdata_dbg+0x14a/0x1a0 [mac80211]
> [ 476.321181] mutex_lock_nested+0x16/0x20
> [ 476.323810] ? mutex_lock_nested+0x16/0x20
> [ 476.326626] __ieee80211_start_rx_ba_session+0x178/0x670 [mac80211]
> [ 476.331627] ieee80211_ba_session_work+0x157/0x2b0 [mac80211]
> [ 476.336078] ? process_one_work+0x14f/0x6a0
> [ 476.338985] process_one_work+0x1ce/0x6a0
> [ 476.341696] worker_thread+0x46/0x400
> [ 476.344061] kthread+0x10f/0x150
> [ 476.346001] ? process_one_work+0x6a0/0x6a0
> [ 476.348886] ? kthread_create_on_node+0x40/0x40
> [ 476.352122] ret_from_fork+0x2a/0x40
>
> Thanks,
> Ben
>
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH] PCI MSI: allow alignment restrictions on vector allocation
From: Thomas Gleixner @ 2017-10-03 21:07 UTC (permalink / raw)
To: Daniel Drake
Cc: linux-kernel, linux-pci, x86, linux-wireless, ath9k-devel, linux
In-Reply-To: <alpine.DEB.2.20.1710021817500.9529@nanos>
On Mon, 2 Oct 2017, Thomas Gleixner wrote:
> On Mon, 2 Oct 2017, Thomas Gleixner wrote:
> > On Mon, 2 Oct 2017, Daniel Drake wrote:
> > 2) The affinity setting of straight MSI interrupts (w/o remapping) on x86
> > requires to make the affinity change from the interrupt context of the
> > current active vector in order not to lose interrupts or worst case
> > getting into a stale state.
> >
> > That works for single vectors, but trying to move all vectors in one
> > go is more or less impossible, as there is no reliable way to
> > determine that none of the other vectors is on flight.
> >
> > There might be some 'workarounds' for that, but I rather avoid that
> > unless we get an official documented one from Intel/AMD.
>
> Thinking more about it. That might be actually a non issue for MSI, but we
> have that modus operandi in the current code and we need to address that
> first before even thinking about multi MSI support.
But even if its possible, it's very debatable whether it's worth the effort
when this driver just can use the legacy INTx.and be done with it.
Thanks,
tglx
^ permalink raw reply
* Re: [PATCH] PCI MSI: allow alignment restrictions on vector allocation
From: Bjorn Helgaas @ 2017-10-03 21:34 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Daniel Drake, linux-kernel, linux-pci, x86, linux-wireless,
ath9k-devel, linux
In-Reply-To: <alpine.DEB.2.20.1710032306170.2278@nanos>
On Tue, Oct 03, 2017 at 11:07:58PM +0200, Thomas Gleixner wrote:
> On Mon, 2 Oct 2017, Thomas Gleixner wrote:
> > On Mon, 2 Oct 2017, Thomas Gleixner wrote:
> > > On Mon, 2 Oct 2017, Daniel Drake wrote:
> > > 2) The affinity setting of straight MSI interrupts (w/o remapping) on x86
> > > requires to make the affinity change from the interrupt context of the
> > > current active vector in order not to lose interrupts or worst case
> > > getting into a stale state.
> > >
> > > That works for single vectors, but trying to move all vectors in one
> > > go is more or less impossible, as there is no reliable way to
> > > determine that none of the other vectors is on flight.
> > >
> > > There might be some 'workarounds' for that, but I rather avoid that
> > > unless we get an official documented one from Intel/AMD.
> >
> > Thinking more about it. That might be actually a non issue for MSI, but we
> > have that modus operandi in the current code and we need to address that
> > first before even thinking about multi MSI support.
>
> But even if its possible, it's very debatable whether it's worth the effort
> when this driver just can use the legacy INTx.and be done with it.
Daniel said "Legacy interrupts do not work on that module, so MSI
support is required," so I assume he means INTx doesn't work. Maybe
the driver could poll? I don't know how much slower that would be,
but at least it would penalize the broken device, not everybody.
Bjorn
^ permalink raw reply
* Re: [PATCH] PCI MSI: allow alignment restrictions on vector allocation
From: Thomas Gleixner @ 2017-10-03 21:46 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Daniel Drake, linux-kernel, linux-pci, x86, linux-wireless,
ath9k-devel, linux
In-Reply-To: <20171003213434.GI25517@bhelgaas-glaptop.roam.corp.google.com>
On Tue, 3 Oct 2017, Bjorn Helgaas wrote:
> On Tue, Oct 03, 2017 at 11:07:58PM +0200, Thomas Gleixner wrote:
> > On Mon, 2 Oct 2017, Thomas Gleixner wrote:
> > > On Mon, 2 Oct 2017, Thomas Gleixner wrote:
> > > > On Mon, 2 Oct 2017, Daniel Drake wrote:
> > > > 2) The affinity setting of straight MSI interrupts (w/o remapping) on x86
> > > > requires to make the affinity change from the interrupt context of the
> > > > current active vector in order not to lose interrupts or worst case
> > > > getting into a stale state.
> > > >
> > > > That works for single vectors, but trying to move all vectors in one
> > > > go is more or less impossible, as there is no reliable way to
> > > > determine that none of the other vectors is on flight.
> > > >
> > > > There might be some 'workarounds' for that, but I rather avoid that
> > > > unless we get an official documented one from Intel/AMD.
> > >
> > > Thinking more about it. That might be actually a non issue for MSI, but we
> > > have that modus operandi in the current code and we need to address that
> > > first before even thinking about multi MSI support.
> >
> > But even if its possible, it's very debatable whether it's worth the effort
> > when this driver just can use the legacy INTx.and be done with it.
>
> Daniel said "Legacy interrupts do not work on that module, so MSI
> support is required," so I assume he means INTx doesn't work. Maybe
Hmm, I missed that detail obviously.
> the driver could poll? I don't know how much slower that would be,
> but at least it would penalize the broken device, not everybody.
That would definitely be prefered.
Thanks,
tglx
^ permalink raw reply
* [PATCH] ath10k: Store coverage-class in case firmware is not booted.
From: greearb @ 2017-10-03 22:11 UTC (permalink / raw)
To: linux-wireless; +Cc: ath10k, Ben Greear
From: Ben Greear <greearb@candelatech.com>
This way, we can apply the values when the NIC does come up.
Signed-off-by: Ben Greear <greearb@candelatech.com>
---
drivers/net/wireless/ath/ath10k/hw.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/hw.c b/drivers/net/wireless/ath/ath10k/hw.c
index afb0c01..31c0589 100644
--- a/drivers/net/wireless/ath/ath10k/hw.c
+++ b/drivers/net/wireless/ath/ath10k/hw.c
@@ -454,8 +454,13 @@ static void ath10k_hw_qca988x_set_coverage_class(struct ath10k *ar,
/* Only modify registers if the core is started. */
if ((ar->state != ATH10K_STATE_ON) &&
- (ar->state != ATH10K_STATE_RESTARTED))
+ (ar->state != ATH10K_STATE_RESTARTED)) {
+ spin_lock_bh(&ar->data_lock);
+ /* Store config value for when radio boots up */
+ ar->fw_coverage.coverage_class = value;
+ spin_unlock_bh(&ar->data_lock);
goto unlock;
+ }
/* Retrieve the current values of the two registers that need to be
* adjusted.
@@ -487,7 +492,7 @@ static void ath10k_hw_qca988x_set_coverage_class(struct ath10k *ar,
ar->fw_coverage.reg_ack_cts_timeout_orig = timeout_reg;
ar->fw_coverage.reg_phyclk = phyclk_reg;
- /* Calculat new value based on the (original) firmware calculation. */
+ /* Calculate new value based on the (original) firmware calculation. */
slottime_reg = ar->fw_coverage.reg_slottime_orig;
timeout_reg = ar->fw_coverage.reg_ack_cts_timeout_orig;
--
2.4.11
^ permalink raw reply related
* [PATCH v2] ath10k: Retry pci probe on failure.
From: greearb @ 2017-10-03 22:13 UTC (permalink / raw)
To: linux-wireless; +Cc: ath10k, Ben Greear
From: Ben Greear <greearb@candelatech.com>
This works around a problem we see when sometimes the wifi NIC does
not respond the first time. This seems to happen especially often on
some of the 9984 NICs in mid-range platforms.
Signed-off-by: Ben Greear <greearb@candelatech.com>
---
v2: Change to mdelay instead of udelay to fix compile issue on ARM.
drivers/net/wireless/ath/ath10k/pci.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 77beb13..0861f7f 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -3487,8 +3487,8 @@ static const struct ath10k_bus_ops ath10k_pci_bus_ops = {
.get_num_banks = ath10k_pci_get_num_banks,
};
-static int ath10k_pci_probe(struct pci_dev *pdev,
- const struct pci_device_id *pci_dev)
+static int __ath10k_pci_probe(struct pci_dev *pdev,
+ const struct pci_device_id *pci_dev)
{
int ret = 0;
struct ath10k *ar;
@@ -3672,6 +3672,22 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
return ret;
}
+static int ath10k_pci_probe(struct pci_dev *pdev,
+ const struct pci_device_id *pci_dev)
+{
+ int cnt = 0;
+ int rv;
+ do {
+ rv = __ath10k_pci_probe(pdev, pci_dev);
+ if (rv == 0)
+ return rv;
+ pr_err("ath10k: failed to probe PCI : %d, retry-count: %d\n", rv, cnt);
+ mdelay(10); /* let the ath10k firmware gerbil take a small break */
+ } while (cnt++ < 10);
+ return rv;
+}
+
+
static void ath10k_pci_remove(struct pci_dev *pdev)
{
struct ath10k *ar = pci_get_drvdata(pdev);
--
2.4.11
^ permalink raw reply related
* Re: [PATCH V2] Fix a sleep-in-atomic bug in shash_setkey_unaligned
From: Marcelo Ricardo Leitner @ 2017-10-03 22:33 UTC (permalink / raw)
To: Jia-Ju Bai
Cc: davem, herbert, nhorman, vyasevich, luto, kvalo, linux-crypto,
netdev, linux-sctp, linux-wireless
In-Reply-To: <1506997522-26684-1-git-send-email-baijiaju1990@163.com>
On Tue, Oct 03, 2017 at 10:25:22AM +0800, Jia-Ju Bai wrote:
> The SCTP program may sleep under a spinlock, and the function call path is:
> sctp_generate_t3_rtx_event (acquire the spinlock)
> sctp_do_sm
> sctp_side_effects
> sctp_cmd_interpreter
> sctp_make_init_ack
> sctp_pack_cookie
> crypto_shash_setkey
> shash_setkey_unaligned
> kmalloc(GFP_KERNEL)
Are you sure this can happen?
The host is not supposed to store any information when replying to an
INIT packet (which generated the INIT_ACK listed above). That said,
it's weird to see the timer function triggering so.
Checking now, that code is dead actually:
$ git grep -A 2 SCTP_CMD_GEN_INIT_ACK
sm_sideeffect.c: case SCTP_CMD_GEN_INIT_ACK:
sm_sideeffect.c- /* Generate an INIT ACK chunk.
*/
sm_sideeffect.c- new_obj =
sctp_make_init_ack(asoc, chunk, GFP_ATOMIC,
Nobody is triggering a call to sctp_cmd_interpreter with
SCTP_CMD_GEN_INIT_ACK command, which would generate the callstack
above.
Marcelo
^ permalink raw reply
* Re: [PATCH V2] Fix a sleep-in-atomic bug in shash_setkey_unaligned
From: Marcelo Ricardo Leitner @ 2017-10-03 22:45 UTC (permalink / raw)
To: Herbert Xu
Cc: Andy Lutomirski, Jia-Ju Bai, David S. Miller, Neil Horman,
vyasevich, Kalle Valo, Linux Crypto Mailing List,
Network Development, linux-sctp, Linux Wireless List
In-Reply-To: <20171003052643.GB22750@gondor.apana.org.au>
On Tue, Oct 03, 2017 at 01:26:43PM +0800, Herbert Xu wrote:
> On Mon, Oct 02, 2017 at 09:18:24PM -0700, Andy Lutomirski wrote:
> > > On Oct 2, 2017, at 7:25 PM, Jia-Ju Bai <baijiaju1990@163.com> wrote:
> > >
> > > The SCTP program may sleep under a spinlock, and the function call path is:
> > > sctp_generate_t3_rtx_event (acquire the spinlock)
> > > sctp_do_sm
> > > sctp_side_effects
> > > sctp_cmd_interpreter
> > > sctp_make_init_ack
> > > sctp_pack_cookie
> > > crypto_shash_setkey
> > > shash_setkey_unaligned
> > > kmalloc(GFP_KERNEL)
> >
> > I'm going to go out on a limb here: why on Earth is out crypto API so
> > full of indirection that we allocate memory at all here?
>
> The crypto API operates on a one key per-tfm basis. So normally
> tfm allocation and key setting is done once only and not done on
> the data path.
>
> I have looked at the SCTP code and it appears to fit this paradigm.
> That is, we should be able to allocate the tfm and set the key when
> the key is actually generated via get_random_bytes, rather than every
> time the key is used which is not only a waste but as you see runs
> into API issues.
Fair point, but
>
> Usually if you're invoking setkey from a non-sleeping code-path
> you're probably doing something wrong.
Usually but not always. There are 3 calls to that function on SCTP
code:
- pack a cookie, which is sent on an INIT_ACK packet to the client
- unpack the cookie above, after it is sent back by the client on a
COOKIE_ECHO packet
- send a chunk authenticated by a hash
the first two happen during softirq processing, while processing a
packet that was received.
As I explained on the other email, SCTP code is not supposed to store
any information about the peer between the 1st and the 2nd moments
above, to be less vulnerable to DoS attacks (it's planned so by the
RFC), thus why it uses the cookie.
The 3rd one we probably can improve, but I don't think we can do much
about the 2 first ones from the SCTP side.
Note on sctp_sf_do_5_1B_init() how sctp_make_init_ack() is explicitly
called with GFP_ATOMIC, and also on sctp_sf_do_unexpected_init().
Though we can't propagate that to crypto_shash_setkey.
Ideas?
Thanks,
Marcelo
>
> As someone else noted recently, there is no single forum for
> reviewing code that uses the crypto API so buggy code like this
> is not surprising.
>
> > We're synchronously computing a hash of a small amount of data using
> > either HMAC-SHA1 or HMAC-SHA256 (determined at runtime) if I read it
> > right. There's a sane way to do this that doesn't need kmalloc,
> > alloca, or fancy indirection. And then there's crypto_shash_xyz().
>
> There are some legitimate cases where you want to use a different
> key for every hashing operation. But so far these are uses have
> been very few so there has been no need to provide an API for them.
>
> Cheers,
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH V2] Fix a sleep-in-atomic bug in shash_setkey_unaligned
From: Marcelo Ricardo Leitner @ 2017-10-03 22:46 UTC (permalink / raw)
To: Jia-Ju Bai
Cc: davem, herbert, nhorman, vyasevich, luto, kvalo, linux-crypto,
netdev, linux-sctp, linux-wireless
In-Reply-To: <20171003223308.GD19750@localhost.localdomain>
On Tue, Oct 03, 2017 at 07:33:08PM -0300, Marcelo Ricardo Leitner wrote:
> On Tue, Oct 03, 2017 at 10:25:22AM +0800, Jia-Ju Bai wrote:
> > The SCTP program may sleep under a spinlock, and the function call path is:
> > sctp_generate_t3_rtx_event (acquire the spinlock)
> > sctp_do_sm
> > sctp_side_effects
> > sctp_cmd_interpreter
> > sctp_make_init_ack
> > sctp_pack_cookie
> > crypto_shash_setkey
> > shash_setkey_unaligned
> > kmalloc(GFP_KERNEL)
>
> Are you sure this can happen?
> The host is not supposed to store any information when replying to an
> INIT packet (which generated the INIT_ACK listed above). That said,
> it's weird to see the timer function triggering so.
>
> Checking now, that code is dead actually:
> $ git grep -A 2 SCTP_CMD_GEN_INIT_ACK
> sm_sideeffect.c: case SCTP_CMD_GEN_INIT_ACK:
> sm_sideeffect.c- /* Generate an INIT ACK chunk.
> */
> sm_sideeffect.c- new_obj =
> sctp_make_init_ack(asoc, chunk, GFP_ATOMIC,
>
> Nobody is triggering a call to sctp_cmd_interpreter with
> SCTP_CMD_GEN_INIT_ACK command, which would generate the callstack
> above.
Nevertheless, the issue is real through other call paths.
Thanks,
Marcelo
^ permalink raw reply
* Re: iwlwifi: ToF usage
From: Luciano Coelho @ 2017-10-04 4:52 UTC (permalink / raw)
To: Joel Bjurström, linux-wireless, assaf.krauss, johannes.berg,
emmanuel.grumbach
Cc: linuxwifi
In-Reply-To: <d3356a70-b90e-d0cc-6328-17f055b2ed1c@maxxflow.com>
Hi Joel,
Unfortunately we don't have full support for ToF on the mainline kernel
yet. But you can try to use one of our Core releases (which is a
backports-based tree) that you can find here:
You could try the release/Core30 branch, for example.
--
Cheers,
Luca.
On Thu, 2017-09-21 at 21:35 +0200, Joel Bjurström wrote:
> Including some people I've seen mentioned in ToF-related e-mails...
>
> This is what I've tried:
>
>
> -- Responder --
>
> * hostapd running, hostapd.conf:
> driver=nl80211
> interface=wlan1
> hw_mode=g
> channel=1
> wmm_enabled=1
> ssid=tof_test
> ftm_responder=1
> ftm_initiator=1
>
> $ cd /sys/kernel/debug/iwlwifi/0000:06:00.0/iwlmvm/netdev:wlan1
> $ echo channel_num=1 > tof_responder_params
> $ echo bssid=bb:bb:bb:bb:bb:bb > tof_responder_params
> $ echo rate=1 > tof_responder_params
> $ echo ftm_per_burst=5 > tof_responder_params
>
> $ echo send_responder_cfg=1 > tof_responder_params
>
>
> -- Initiator --
>
> $ cd /sys/kernel/debug/iwlwifi/0000:06:00.0/iwlmvm/netdev:wlan1
> $ echo send_tof_cfg=1 > tof_enable
> $ echo 'num_of_ap=1' > tof_range_request
> $ echo 'ap=0 1 0 0 bb:bb:bb:bb:bb:bb 0 10 0 5 5 0 0 0 0 -40' \
> > tof_range_request
>
> $ echo 'send_range_request=1' > tof_range_request
>
> $ cat tof_range_response
> request_id = 0
> status = 2
> last_in_batch = 1
> num_of_aps = 0
>
>
>
> (bb:bb:bb:bb:bb:bb is the responder's BSSID)
>
> Am I even close? Fumbling in the dark here.
>
> The 'status = 2' from tof_range_response comes directly from the FW,
> AFAICT. What do different values mean?
>
> Cheers,
> Joel
>
>
> On 09/14/2017 09:44 PM, Joel B wrote:
> > Hi,
> >
> > Starting to play around with the FTM/ToF support in iwlwifi, but
> > documentation is (understandably) scarce at this point. Using a
> > pair of
> > 8260:s I had lying around.
> >
> > Can someone give me some hints on how to work the debugfs API for
> > a
> > successful measurement?
> >
> > I've set up one card as AP with hostapd (with ftm_responder=1 and
> > ftm_initiator=1 in hostapd.conf), the other as a STA.
> >
> > From the STA, I've played with 'tof_range_request' in debugfs.
> > Writing
> > send_range_request=1, which it doesn't choke on or anything, but
> > nothing
> > seems to happen. Guess I need to set things up a bit first, but
> > how?
> >
> > If it's possible to get this to work at all at this stage, some
> > hints on
> > how to do it would be great.
> >
> >
> > Thanks,
> > Joel
^ permalink raw reply
* Re: [PATCH 00/11] SDIO support for ath10k
From: Alagu Sankar @ 2017-10-04 6:22 UTC (permalink / raw)
To: Erik Stromdahl, silexcommon, ath10k; +Cc: linux-wireless
In-Reply-To: <b4bbf022-77d7-f907-f35b-7afacfc227e5@gmail.com>
Hi Erik,
We will work to have this support mainlined as soon as possible. Would
appreciate your support
in making sure that the patches do not affect the USB high latency path.
On 02-10-2017 14:32, Erik Stromdahl wrote:
> Hi Alagu,
>
> It is great to see that we are finally about have fully working
> mainline support for QCA9377 SDIO chipsets!
>
> Great job!
>
> On 2017-09-30 19:37, silexcommon@gmail.com wrote:
>> From: Alagu Sankar <alagusankar@silex-india.com>
>>
>> This patchset, generated against master-pending branch, enables a fully
>> functional SDIO interface driver for ath10k. Patches have been
>> verified on
>> QCA9377-3 WB396 and Silex's SX-SDCAC reference cards with Station,
>> Access Point
>> and P2P modes.
>>
>> The driver is verified with the firmware WLAN.TF.1.1.1-00061-QCATFSWPZ-1
>> with the board data from respective SDIO card vendors. Receive
>> performance
>> matches the QCA reference driver when used with SDIO3.0 enabled
>> platforms.
>> iperf tests indicate a downlink UDP of 275Mbit/s and TCP of 150Mbit/s
>>
> Can you share any scripts etc. (wrapping hostapd and wpa_supplicant
> stuff)
> or provide some more info about you test setup?
>
I am not using any specific scripts for this. The standard ones from the
ath10k configuration page
https://wireless.wiki.kernel.org/en/users/drivers/ath10k/configuration
works just fine with the
NL80211 path.
> I made a quick socat based test on an old laptop (I don't think it has
> SDIO
> 3.0 support) and I did unfortunately not get the same figures as you
> did :(
>
If it is SDIO v1.x, then the max bus speed is only 100Mbit/s. With v2.x
it is 200Mbit/s and 3.x it is
832 Mbit/s. Throughput primarily depends on it. We used i.MX6 SoloX
Sabre SDB platform
which supports SDIO3.x and could see the maximum performance.
>> This patchset differs from the previous high latency patches,
>> specific to SDIO.
>> HI_ACS_FLAGS_SDIO_REDUCE_TX_COMPL_SET is enabled for HI_ACS. This
>> instructs the
>> firmware to use HTT_T2H_MSG_TYPE_TX_COMPL_IND for outgoing packets.
>> Without
>> this flag, the management frames are not sent out by the firmware.
>> Possibility
>> of management frames being sent via WMI and data frames through the
>> reduced Tx
>> completion needs to be probed further.
>>
> Ah, so that explains why I couldn't see any messages in the air.
>
>> Further improvements can be done on the transmit path by implementing
>> packet
>> bundle. Scatter Gather is another area of improvement for both
>> Transmit and
>> Receive, but may not work on all platforms
>>
>> Known issues: Surprise removal of the card, when the device is in
>> connected
>> state, delays sdio function remove due to delayed WMI command failures.
>> Existing ath10k framework can not differentiate between a kernel module
>> removal and the surprise removal of teh card.
>>
>> Alagu Sankar (11):
>> ath10k_sdio: sdio htt data transfer fixes
>> ath10k_sdio: wb396 reference card fix
>> ath10k_sdio: DMA bounce buffers for read write
>> ath10k_sdio: reduce transmit msdu count
>> ath10k_sdio: use clean packet headers
>> ath10k_sdio: high latency fixes for beacon buffer
>> ath10k_sdio: fix rssi indication
>> ath10k_sdio: common read write
>> ath10k_sdio: virtual scatter gather for receive
>> ath10k_sdio: enable firmware crash dump
>> ath10k_sdio: hif start once addition
>>
>> drivers/net/wireless/ath/ath10k/core.c | 35 ++-
>> drivers/net/wireless/ath/ath10k/debug.c | 3 +
>> drivers/net/wireless/ath/ath10k/htc.c | 4 +-
>> drivers/net/wireless/ath/ath10k/htc.h | 1 +
>> drivers/net/wireless/ath/ath10k/htt_rx.c | 19 +-
>> drivers/net/wireless/ath/ath10k/htt_tx.c | 24 +-
>> drivers/net/wireless/ath/ath10k/hw.c | 2 +
>> drivers/net/wireless/ath/ath10k/hw.h | 1 +
>> drivers/net/wireless/ath/ath10k/mac.c | 31 ++-
>> drivers/net/wireless/ath/ath10k/sdio.c | 398
>> ++++++++++++++++++++++--------
>> drivers/net/wireless/ath/ath10k/sdio.h | 10 +-
>> drivers/net/wireless/ath/ath10k/wmi-tlv.c | 2 +-
>> 12 files changed, 403 insertions(+), 127 deletions(-)
>>
>
^ permalink raw reply
* Re: [PATCH 0/3] linux-firmware: ath10k update 20170918
From: Kalle Valo @ 2017-10-04 8:44 UTC (permalink / raw)
To: linux-firmware@kernel.org
Cc: linux-wireless@vger.kernel.org, ath10k@lists.infradead.org
In-Reply-To: <1505746857-31938-1-git-send-email-kvalo@qca.qualcomm.com>
Kalle Valo <kvalo@qca.qualcomm.com> writes:
> Few firmware fixes to user reported issues for ath10k, but the other "fix=
"
> being a revert back to older firmware. Also a small board-2.bin update.
>
> Kalle Valo (3):
> ath10k: QCA6174 hw3.0: update firmware-6.bin to
> WLAN.RM.4.4.1-00051-QCARMSWP-1
> ath10k: QCA6174 hw3.0: update board-2.bin
> Revert "ath10k: QCA988X hw2.0: update firmware to 10.2.4.70.63-2"
>
> WHENCE | 4 +-
> ath10k/QCA6174/hw3.0/board-2.bin | Bin 477060 -> 501748=
bytes
> ath10k/QCA6174/hw3.0/firmware-6.bin | Bin 711408 -> 727776=
bytes
> ath10k/QCA6174/hw3.0/notice_ath10k_firmware-6.txt | 97 +++++-----------=
------
> ath10k/QCA988X/hw2.0/firmware-5.bin | Bin 249384 -> 248188=
bytes
> 5 files changed, 25 insertions(+), 76 deletions(-)
Ping? These fix serious regressions reported by many so it would be
really good to get these to linux-firmware soon.
--=20
Kalle Valo=
^ permalink raw reply
* Re: QCA988x and kernel 4.13
From: Kalle Valo @ 2017-10-04 8:50 UTC (permalink / raw)
To: Ben Greear
Cc: Sebastian Gottschall, svp, ath10k@lists.infradead.org,
linux-wireless@vger.kernel.org
In-Reply-To: <48aebea8-9231-b41e-cbaa-d592d8540c53@candelatech.com>
Ben Greear <greearb@candelatech.com> writes:
> On 10/01/2017 07:06 AM, Sebastian Gottschall wrote:
>> you have to update to the latest mac80211 code. there is a well
>> known bug in mac80211 which causes a deadlock with ath10k
>
> Do you have a pointer to the patch that fixes this? I'd like to check if=
it has
> made it into 4.13 stable yet...
I think it's this one:
mac80211: fix deadlock in driver-managed RX BA session start
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?=
id=3Dbde59c475e0883e4c4294bcd9b9c7e08ae18c828
Adding linux-wireless so that someone can correct me in case I'm wrong.
--=20
Kalle Valo=
^ 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