Re: [RESEND] rsi: Remove stack VLA usage

[Tobin C. Harding]

Added Konstantin in case he is in charge of administering patchwork.kernel.org?

On Tue, Mar 13, 2018 at 07:53:34PM -0700, Kees Cook wrote:
> On Tue, Mar 13, 2018 at 7:11 PM, Tobin C. Harding  wrote:
> > On Tue, Mar 13, 2018 at 11:00:47PM +0200, Andy Shevchenko wrote:
> >> On Tue, Mar 13, 2018 at 10:17 PM, tcharding  wrote:
> >> > On Mon, Mar 12, 2018 at 09:46:06AM +, Kalle Valo wrote:
> >> >> tcharding  wrote:
> >>
> >> I'm pretty much sure it depends on the original email headers, like
> >> above ^^^ — no name.
> >> Perhaps git config on your side should be done.
> >
> > Thanks for the suggestion Andy but the 'tcharding' as the name was
> > munged by either Kalle or patchwork.  I'm guessing patchwork.
> 
> Something you're sending from is using "tcharding" (see the email Andy
> quotes). I see the headers as:
> 
> Date: Wed, 14 Mar 2018 07:17:57 +1100
> From: tcharding 
> ...
> Message-ID: <20180313201757.GK8631@eros>
> X-Mailer: Mutt 1.5.24 (2015-08-30)
> User-Agent: Mutt/1.5.24 (2015-08-30)
> 
> Your most recently email shows "Tobin C. Harding" though, and also
> sent with Mutt...
>
> Do you have multiple Mutt configurations? Is something lacking a
> "From" header insertion and your MTA is filling it in for you from
> your username?

Thanks for taking the time to respond Kees (and Tycho).  I have mutt
configured to reply from whichever email address I receive from so if
patchwork sent an email to 'tcharding ' (which is the
details it has) and I hit reply it would have come from 'tcharding',
hence Andy's reply.  I wouldn't bet my life on it but I'm kinda
confident that I cannot initiate an email from 'tcharding' with my
current set up.

Super bad form to blame someone (or something else) but I think this is
a problem with how my patchwork account is configured.  Either way, that
is still my fault I should have added my real name to patchwork when I
signed up (not just username 'tcharding').

Is patchwork.kernel.org administered by Konstantin Ryabitsev? Added
Konstantin to CC's.

thanks,
Tobin.

Re: [RESEND] rsi: Remove stack VLA usage

[Kees Cook]

On Tue, Mar 13, 2018 at 7:11 PM, Tobin C. Harding  wrote:
> On Tue, Mar 13, 2018 at 11:00:47PM +0200, Andy Shevchenko wrote:
>> On Tue, Mar 13, 2018 at 10:17 PM, tcharding  wrote:
>> > On Mon, Mar 12, 2018 at 09:46:06AM +, Kalle Valo wrote:
>> >> tcharding  wrote:
>>
>> I'm pretty much sure it depends on the original email headers, like
>> above ^^^ — no name.
>> Perhaps git config on your side should be done.
>
> Thanks for the suggestion Andy but the 'tcharding' as the name was
> munged by either Kalle or patchwork.  I'm guessing patchwork.

Something you're sending from is using "tcharding" (see the email Andy
quotes). I see the headers as:

Date: Wed, 14 Mar 2018 07:17:57 +1100
From: tcharding 
...
Message-ID: <20180313201757.GK8631@eros>
X-Mailer: Mutt 1.5.24 (2015-08-30)
User-Agent: Mutt/1.5.24 (2015-08-30)

Your most recently email shows "Tobin C. Harding" though, and also
sent with Mutt...

Do you have multiple Mutt configurations? Is something lacking a
"From" header insertion and your MTA is filling it in for you from
your username?

-Kees

-- 
Kees Cook
Pixel Security

Re: [RESEND] rsi: Remove stack VLA usage

[Tobin C. Harding]

On Tue, Mar 13, 2018 at 11:00:47PM +0200, Andy Shevchenko wrote:
> On Tue, Mar 13, 2018 at 10:17 PM, tcharding  wrote:
> > On Mon, Mar 12, 2018 at 09:46:06AM +, Kalle Valo wrote:
> >> tcharding  wrote:
> 
> I'm pretty much sure it depends on the original email headers, like
> above ^^^ — no name.
> Perhaps git config on your side should be done.

Thanks for the suggestion Andy but the 'tcharding' as the name was
munged by either Kalle or patchwork.  I'm guessing patchwork.


thanks,
Tobin.

Re: [PATCH] drivers: net: wireless: ath: ath9: dfs: remove VLA usage

[Miguel Ojeda]

On Sun, Mar 11, 2018 at 12:12 AM, Kees Cook  wrote:
>
> The problem is that it's not a "constant expression", so the compiler
> frontend still yells about it under -Wvla. I would characterize this
> mainly as a fix for "accidental VLA" or "misdetected VLA" or something
> like that. AIUI, there really isn't a functional change here.

C++11's constexpr variables would be nice to have.

Cheers,
Miguel

Re: [PATCH] drivers: net: wireless: ath: ath9: dfs: remove VLA usage

[Daniel Micay]

> I don't think the difference between C and C++ const pointer
> conversions

I mean I don't think the difference between them was intended.

Re: [PATCH] drivers: net: wireless: ath: ath9: dfs: remove VLA usage

[Daniel Micay]

No, it's undefined behavior to write to a const variable. The `static`
and `const` on the variable both change the code generation in the
real world as permitted / encouraged by the standard. It's placed in
read-only memory. Trying to write to it will break. It's not
"implemented defined" to write to it, it's "undefined behavior" i.e.
it's considered incorrect. There a clear distinction between those in
the standard.

You're confusing having a real `const` for a variable with having it
applied to a pointer. It's well-defined to cast away const from a
pointer and write to what it points at if it's not actually const. If
it is const, that's broken.

There's nothing implementation defined about either case.

The C standard could have considered `static const` variables to work
as constant expressions just like the C++ standard. They borrowed it
from there but made it less useful than const in what became the C++
standard. They also used stricter rules for the permitted implicit
conversions of const pointers which made those much less usable, i.e.
converting `int **` to `const int *const *` wasn't permitted like C++.
I don't think the difference between C and C++ const pointer
conversions, it's a quirk of them being standardized on different
timelines and ending up with different versions of the same thing. On
the other hand, they might have left out having ever so slightly more
useful constant expressions on purpose since people can use #define.

[RFC v5 4/9] cfg80211: Support all iftypes in autodisconnect_wk

[Denis Kenzior]

Currently autodisconnect_wk assumes that only interface types of
P2P_CLIENT and STATION use conn_owner_nlportid.  Change this so all
interface types are supported.

Signed-off-by: Denis Kenzior 
---
 net/wireless/sme.c | 43 ---
 1 file changed, 32 insertions(+), 11 deletions(-)

diff --git a/net/wireless/sme.c b/net/wireless/sme.c
index 701cfd7acc1b..5df6b33db786 100644
--- a/net/wireless/sme.c
+++ b/net/wireless/sme.c
@@ -1239,17 +1239,38 @@ void cfg80211_autodisconnect_wk(struct work_struct 
*work)
wdev_lock(wdev);
 
if (wdev->conn_owner_nlportid) {
-   /*
-* Use disconnect_bssid if still connecting and ops->disconnect
-* not implemented.  Otherwise we can use cfg80211_disconnect.
-*/
-   if (rdev->ops->disconnect || wdev->current_bss)
-   cfg80211_disconnect(rdev, wdev->netdev,
-   WLAN_REASON_DEAUTH_LEAVING, true);
-   else
-   cfg80211_mlme_deauth(rdev, wdev->netdev,
-wdev->disconnect_bssid, NULL, 0,
-WLAN_REASON_DEAUTH_LEAVING, false);
+   switch (wdev->iftype) {
+   case NL80211_IFTYPE_ADHOC:
+   cfg80211_leave_ibss(rdev, wdev->netdev, false);
+   break;
+   case NL80211_IFTYPE_AP:
+   case NL80211_IFTYPE_P2P_GO:
+   cfg80211_stop_ap(rdev, wdev->netdev, false);
+   break;
+   case NL80211_IFTYPE_MESH_POINT:
+   cfg80211_leave_mesh(rdev, wdev->netdev);
+   break;
+   case NL80211_IFTYPE_STATION:
+   case NL80211_IFTYPE_P2P_CLIENT:
+   /*
+* Use disconnect_bssid if still connecting and
+* ops->disconnect not implemented.  Otherwise we can
+* use cfg80211_disconnect.
+*/
+   if (rdev->ops->disconnect || wdev->current_bss)
+   cfg80211_disconnect(rdev, wdev->netdev,
+   WLAN_REASON_DEAUTH_LEAVING,
+   true);
+   else
+   cfg80211_mlme_deauth(rdev, wdev->netdev,
+wdev->disconnect_bssid,
+NULL, 0,
+WLAN_REASON_DEAUTH_LEAVING,
+false);
+   break;
+   default:
+   break;
+   }
}
 
wdev_unlock(wdev);
-- 
2.13.5

[RFC v5 3/9] nl80211: Add CONTROL_PORT_OVER_NL80211 attribute

[Denis Kenzior]

Signed-off-by: Denis Kenzior 
---
 include/net/cfg80211.h   |  3 +++
 include/uapi/linux/nl80211.h | 14 +-
 net/wireless/nl80211.c   | 13 +
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 76b6783f35f6..2e7f30c66913 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -646,6 +646,8 @@ struct survey_info {
  * allowed through even on unauthorized ports
  * @control_port_no_encrypt: TRUE to prevent encryption of control port
  * protocol frames.
+ * @control_port_over_nl80211: TRUE if userspace expects to exchange control
+ * port frames over NL80211 instead of the network interface.
  * @wep_keys: static WEP keys, if not NULL points to an array of
  * CFG80211_MAX_WEP_KEYS WEP keys
  * @wep_tx_key: key index (0..3) of the default TX static WEP key
@@ -661,6 +663,7 @@ struct cfg80211_crypto_settings {
bool control_port;
__be16 control_port_ethertype;
bool control_port_no_encrypt;
+   bool control_port_over_nl80211;
struct key_params *wep_keys;
int wep_tx_key;
const u8 *psk;
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 77675ae3e475..1cdac3d732c1 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -542,7 +542,8 @@
  * IEs in %NL80211_ATTR_IE, %NL80211_ATTR_AUTH_TYPE, %NL80211_ATTR_USE_MFP,
  * %NL80211_ATTR_MAC, %NL80211_ATTR_WIPHY_FREQ, %NL80211_ATTR_CONTROL_PORT,
  * %NL80211_ATTR_CONTROL_PORT_ETHERTYPE,
- * %NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT, %NL80211_ATTR_MAC_HINT, and
+ * %NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT,
+ * %NL80211_ATTR_CONTROL_PORT_OVER_NL80211, %NL80211_ATTR_MAC_HINT, and
  * %NL80211_ATTR_WIPHY_FREQ_HINT.
  * If included, %NL80211_ATTR_MAC and %NL80211_ATTR_WIPHY_FREQ are
  * restrictions on BSS selection, i.e., they effectively prevent roaming
@@ -1488,6 +1489,15 @@ enum nl80211_commands {
  * @NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT: When included along with
  * %NL80211_ATTR_CONTROL_PORT_ETHERTYPE, indicates that the custom
  * ethertype frames used for key negotiation must not be encrypted.
+ * @NL80211_ATTR_CONTROL_PORT_OVER_NL80211: A flag indicating whether control
+ * port frames (e.g. of type given in %NL80211_ATTR_CONTROL_PORT_ETHERTYPE)
+ * will be sent directly to the network interface or sent via the NL80211
+ * socket.  If this attribute is missing, then legacy behavior of sending
+ * control port frames directly to the network interface is used.  If the
+ * flag is included, then control port frames are sent over NL80211 instead
+ * using %CMD_CONTROL_PORT_FRAME.  If control port routing over NL80211 is
+ * to be used then userspace must also use the %NL80211_ATTR_SOCKET_OWNER
+ * flag.
  *
  * @NL80211_ATTR_TESTDATA: Testmode data blob, passed through to the driver.
  * We recommend using nested, driver-specific attributes within this.
@@ -2641,6 +2651,8 @@ enum nl80211_attrs {
NL80211_ATTR_NSS,
NL80211_ATTR_ACK_SIGNAL,
 
+   NL80211_ATTR_CONTROL_PORT_OVER_NL80211,
+
/* add attributes here, update the policy in nl80211.c */
 
__NL80211_ATTR_AFTER_LAST,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 3c4dbfa0ca71..24b1bd940fca 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -287,6 +287,7 @@ static const struct nla_policy 
nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_CONTROL_PORT] = { .type = NLA_FLAG },
[NL80211_ATTR_CONTROL_PORT_ETHERTYPE] = { .type = NLA_U16 },
[NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT] = { .type = NLA_FLAG },
+   [NL80211_ATTR_CONTROL_PORT_OVER_NL80211] = { .type = NLA_FLAG },
[NL80211_ATTR_PRIVACY] = { .type = NLA_FLAG },
[NL80211_ATTR_CIPHER_SUITE_GROUP] = { .type = NLA_U32 },
[NL80211_ATTR_WPA_VERSIONS] = { .type = NLA_U32 },
@@ -8227,6 +8228,18 @@ static int nl80211_crypto_settings(struct 
cfg80211_registered_device *rdev,
} else
settings->control_port_ethertype = cpu_to_be16(ETH_P_PAE);
 
+   if (info->attrs[NL80211_ATTR_CONTROL_PORT_OVER_NL80211]) {
+   if (!info->attrs[NL80211_ATTR_SOCKET_OWNER])
+   return -EINVAL;
+
+   if (!rdev->ops->tx_control_port ||
+   !wiphy_ext_feature_isset(>wiphy,
+
NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211))
+   return -EOPNOTSUPP;
+
+   settings->control_port_over_nl80211 = true;
+   }
+
if (info->attrs[NL80211_ATTR_CIPHER_SUITES_PAIRWISE]) {
void *data;
int len, i;
-- 
2.13.5

[RFC v5 5/9] nl80211: Add SOCKET_OWNER support to JOIN_IBSS

[Denis Kenzior]

Signed-off-by: Denis Kenzior 
---
 include/uapi/linux/nl80211.h | 2 ++
 net/wireless/ibss.c  | 1 +
 net/wireless/nl80211.c   | 6 ++
 3 files changed, 9 insertions(+)

diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 1cdac3d732c1..877fab2836ec 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1985,6 +1985,8 @@ enum nl80211_commands {
  * multicast group.
  * If set during %NL80211_CMD_ASSOCIATE or %NL80211_CMD_CONNECT the
  * station will deauthenticate when the socket is closed.
+ * If set during %NL80211_CMD_JOIN_IBSS the IBSS will be automatically
+ * torn down when the socket is closed.
  *
  * @NL80211_ATTR_TDLS_INITIATOR: flag attribute indicating the current end is
  * the TDLS link initiator.
diff --git a/net/wireless/ibss.c b/net/wireless/ibss.c
index a1d10993d08a..d5d26fc5b853 100644
--- a/net/wireless/ibss.c
+++ b/net/wireless/ibss.c
@@ -224,6 +224,7 @@ int __cfg80211_leave_ibss(struct cfg80211_registered_device 
*rdev,
if (err)
return err;
 
+   wdev->conn_owner_nlportid = 0;
__cfg80211_clear_ibss(dev, nowext);
 
return 0;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 24b1bd940fca..e678dc510f3a 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -8691,6 +8691,12 @@ static int nl80211_join_ibss(struct sk_buff *skb, struct 
genl_info *info)
err = cfg80211_join_ibss(rdev, dev, , connkeys);
if (err)
kzfree(connkeys);
+   else if (info->attrs[NL80211_ATTR_SOCKET_OWNER]) {
+   wdev_lock(dev->ieee80211_ptr);
+   dev->ieee80211_ptr->conn_owner_nlportid = info->snd_portid;
+   wdev_unlock(dev->ieee80211_ptr);
+   }
+
return err;
 }
 
-- 
2.13.5

[RFC v5 6/9] nl80211: Add SOCKET_OWNER support to JOIN_MESH

[Denis Kenzior]

Signed-off-by: Denis Kenzior 
---
 include/uapi/linux/nl80211.h |  2 ++
 net/wireless/mesh.c  |  1 +
 net/wireless/nl80211.c   | 10 +-
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 877fab2836ec..e3329bc4644b 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1987,6 +1987,8 @@ enum nl80211_commands {
  * station will deauthenticate when the socket is closed.
  * If set during %NL80211_CMD_JOIN_IBSS the IBSS will be automatically
  * torn down when the socket is closed.
+ * If set during %NL80211_CMD_JOIN_MESH the mesh setup will be
+ * automatically torn down when the socket is closed.
  *
  * @NL80211_ATTR_TDLS_INITIATOR: flag attribute indicating the current end is
  * the TDLS link initiator.
diff --git a/net/wireless/mesh.c b/net/wireless/mesh.c
index b12da6ef3c12..e91a5078615b 100644
--- a/net/wireless/mesh.c
+++ b/net/wireless/mesh.c
@@ -286,6 +286,7 @@ int __cfg80211_leave_mesh(struct cfg80211_registered_device 
*rdev,
 
err = rdev_leave_mesh(rdev, dev);
if (!err) {
+   wdev->conn_owner_nlportid = 0;
wdev->mesh_id_len = 0;
wdev->beacon_interval = 0;
memset(>chandef, 0, sizeof(wdev->chandef));
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index e678dc510f3a..e38d55cf34f7 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -10140,7 +10140,15 @@ static int nl80211_join_mesh(struct sk_buff *skb, 
struct genl_info *info)
setup.userspace_handles_dfs =
nla_get_flag(info->attrs[NL80211_ATTR_HANDLE_DFS]);
 
-   return cfg80211_join_mesh(rdev, dev, , );
+   err = cfg80211_join_mesh(rdev, dev, , );
+
+   if (!err && info->attrs[NL80211_ATTR_SOCKET_OWNER]) {
+   wdev_lock(dev->ieee80211_ptr);
+   dev->ieee80211_ptr->conn_owner_nlportid = info->snd_portid;
+   wdev_unlock(dev->ieee80211_ptr);
+   }
+
+   return err;
 }
 
 static int nl80211_leave_mesh(struct sk_buff *skb, struct genl_info *info)
-- 
2.13.5

[RFC v5 8/9] mac80211: Add support for tx_control_port

[Denis Kenzior]

Signed-off-by: Denis Kenzior 
---
 net/mac80211/cfg.c |  1 +
 net/mac80211/ieee80211_i.h |  3 +++
 net/mac80211/tx.c  | 46 ++
 3 files changed, 50 insertions(+)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index fd68f6fb02d7..9294acb495ee 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3786,4 +3786,5 @@ const struct cfg80211_ops mac80211_config_ops = {
.add_nan_func = ieee80211_add_nan_func,
.del_nan_func = ieee80211_del_nan_func,
.set_multicast_to_unicast = ieee80211_set_multicast_to_unicast,
+   .tx_control_port = ieee80211_tx_control_port,
 };
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index ae9c33cd8ada..a52bd2a61a27 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1734,6 +1734,9 @@ void ieee80211_check_fast_xmit(struct sta_info *sta);
 void ieee80211_check_fast_xmit_all(struct ieee80211_local *local);
 void ieee80211_check_fast_xmit_iface(struct ieee80211_sub_if_data *sdata);
 void ieee80211_clear_fast_xmit(struct sta_info *sta);
+int ieee80211_tx_control_port(struct wiphy *wiphy, struct net_device *dev,
+ const u8 *buf, size_t len,
+ const u8 *dest, __be16 proto, bool unencrypted);
 
 /* HT */
 void ieee80211_apply_htcap_overrides(struct ieee80211_sub_if_data *sdata,
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 7643178ef132..6ae8fe121500 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -4749,3 +4749,49 @@ void __ieee80211_tx_skb_tid_band(struct 
ieee80211_sub_if_data *sdata,
ieee80211_xmit(sdata, NULL, skb);
local_bh_enable();
 }
+
+int ieee80211_tx_control_port(struct wiphy *wiphy, struct net_device *dev,
+ const u8 *buf, size_t len,
+ const u8 *dest, __be16 proto, bool unencrypted)
+{
+   struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+   struct ieee80211_local *local = sdata->local;
+   struct sk_buff *skb;
+   struct ethhdr *ehdr;
+   u32 flags;
+
+   /* Only accept CONTROL_PORT_PROTOCOL configured in CONNECT/ASSOCIATE
+* or Pre-Authentication
+*/
+   if (proto != sdata->control_port_protocol &&
+   proto != cpu_to_be16(ETH_P_PREAUTH))
+   return -EINVAL;
+
+   if (unencrypted)
+   flags = IEEE80211_TX_INTFL_DONT_ENCRYPT;
+   else
+   flags = 0;
+
+   skb = dev_alloc_skb(local->hw.extra_tx_headroom +
+   sizeof(struct ethhdr) + len);
+   if (!skb)
+   return -ENOMEM;
+
+   skb_reserve(skb, local->hw.extra_tx_headroom + sizeof(struct ethhdr));
+
+   skb_put_data(skb, buf, len);
+
+   ehdr = skb_push(skb, sizeof(struct ethhdr));
+   memcpy(ehdr->h_dest, dest, ETH_ALEN);
+   memcpy(ehdr->h_source, sdata->vif.addr, ETH_ALEN);
+   ehdr->h_proto = proto;
+
+   skb->dev = dev;
+   skb->protocol = htons(ETH_P_802_3);
+   skb_reset_network_header(skb);
+   skb_reset_mac_header(skb);
+
+   __ieee80211_subif_start_xmit(skb, skb->dev, flags);
+
+   return 0;
+}
-- 
2.13.5

[RFC v5 9/9] mac80211: Send control port frames over nl80211

[Denis Kenzior]

If userspace requested control port frames to go over 80211, then do so.
The control packets are intercepted just prior to delivery of the packet
to the underlying network device.

Pre-authentication type frames (protocol: 0x88c7) are also forwarded
over nl80211.

Signed-off-by: Denis Kenzior 
---
 net/mac80211/cfg.c |  4 
 net/mac80211/ieee80211_i.h |  1 +
 net/mac80211/iface.c   |  2 ++
 net/mac80211/main.c|  2 ++
 net/mac80211/mlme.c|  2 ++
 net/mac80211/rx.c  | 33 -
 6 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 9294acb495ee..180653fcdb94 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -925,6 +925,8 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct 
net_device *dev,
 */
sdata->control_port_protocol = params->crypto.control_port_ethertype;
sdata->control_port_no_encrypt = params->crypto.control_port_no_encrypt;
+   sdata->control_port_over_nl80211 =
+   params->crypto.control_port_over_nl80211;
sdata->encrypt_headroom = ieee80211_cs_headroom(sdata->local,
>crypto,
sdata->vif.type);
@@ -934,6 +936,8 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct 
net_device *dev,
params->crypto.control_port_ethertype;
vlan->control_port_no_encrypt =
params->crypto.control_port_no_encrypt;
+   vlan->control_port_over_nl80211 =
+   params->crypto.control_port_over_nl80211;
vlan->encrypt_headroom =
ieee80211_cs_headroom(sdata->local,
  >crypto,
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index a52bd2a61a27..00dbc6a1b79d 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -899,6 +899,7 @@ struct ieee80211_sub_if_data {
u16 sequence_number;
__be16 control_port_protocol;
bool control_port_no_encrypt;
+   bool control_port_over_nl80211;
int encrypt_headroom;
 
atomic_t num_tx_queued;
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index d13ba064951f..555e389b7dfa 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -519,6 +519,8 @@ int ieee80211_do_open(struct wireless_dev *wdev, bool 
coming_up)
master->control_port_protocol;
sdata->control_port_no_encrypt =
master->control_port_no_encrypt;
+   sdata->control_port_over_nl80211 =
+   master->control_port_over_nl80211;
sdata->vif.cab_queue = master->vif.cab_queue;
memcpy(sdata->vif.hw_queue, master->vif.hw_queue,
   sizeof(sdata->vif.hw_queue));
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 0785d04a80bc..e5a51267c75d 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -554,6 +554,8 @@ struct ieee80211_hw *ieee80211_alloc_hw_nm(size_t 
priv_data_len,
   NL80211_FEATURE_USERSPACE_MPM |
   NL80211_FEATURE_FULL_AP_CLIENT_STATE;
wiphy_ext_feature_set(wiphy, NL80211_EXT_FEATURE_FILS_STA);
+   wiphy_ext_feature_set(wiphy,
+ NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211);
 
if (!ops->hw_scan)
wiphy->features |= NL80211_FEATURE_LOW_PRIORITY_SCAN |
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 0024eff9bb84..b3665b857883 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -4844,6 +4844,8 @@ int ieee80211_mgd_assoc(struct ieee80211_sub_if_data 
*sdata,
 
sdata->control_port_protocol = req->crypto.control_port_ethertype;
sdata->control_port_no_encrypt = req->crypto.control_port_no_encrypt;
+   sdata->control_port_over_nl80211 =
+   req->crypto.control_port_over_nl80211;
sdata->encrypt_headroom = ieee80211_cs_headroom(local, >crypto,
sdata->vif.type);
 
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index de7d10732fd5..bbb8bc6cac2a 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -2245,6 +2245,32 @@ static bool ieee80211_frame_allowed(struct 
ieee80211_rx_data *rx, __le16 fc)
return true;
 }
 
+static void ieee80211_deliver_skb_to_local_stack(struct sk_buff *skb,
+struct ieee80211_rx_data *rx)
+{
+   struct ieee80211_sub_if_data *sdata = rx->sdata;
+   struct net_device *dev = sdata->dev;
+
+   if (unlikely((skb->protocol == sdata->control_port_protocol ||
+ skb->protocol == cpu_to_be16(ETH_P_PREAUTH)) &&
+ 

[RFC v5 2/9] nl80211: Implement TX of control port frames

[Denis Kenzior]

This commit implements the TX side of NL80211_CMD_CONTROL_PORT_FRAME.
Userspace provides the raw EAPoL frame using NL80211_ATTR_FRAME.
Userspace should also provide the destination address and the protocol
type to use when sending the frame.  This is used to implement TX of
Pre-authentication frames.  If CONTROL_PORT_ETHERTYPE_NO_ENCRYPT is
specified, then the driver will be asked not to encrypt the outgoing
frame.

A new EXT_FEATURE flag is introduced so that nl80211 code can check
whether a given wiphy has capability to pass EAPoL frames over NL80211.

Signed-off-by: Denis Kenzior 
---
 include/net/cfg80211.h   |  9 ++
 include/uapi/linux/nl80211.h |  3 ++
 net/wireless/nl80211.c   | 70 +++-
 net/wireless/rdev-ops.h  | 15 ++
 net/wireless/trace.h | 26 
 5 files changed, 122 insertions(+), 1 deletion(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 6dee630ee66d..76b6783f35f6 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2960,6 +2960,9 @@ struct cfg80211_external_auth_params {
  *
  * @external_auth: indicates result of offloaded authentication processing from
  * user space
+ *
+ * @tx_control_port: TX a control port frame (EAPoL).  The noencrypt parameter
+ * tells the driver that the frame should not be encrypted.
  */
 struct cfg80211_ops {
int (*suspend)(struct wiphy *wiphy, struct cfg80211_wowlan *wow);
@@ -3255,6 +3258,12 @@ struct cfg80211_ops {
   const u8 *aa);
int (*external_auth)(struct wiphy *wiphy, struct net_device *dev,
 struct cfg80211_external_auth_params *params);
+
+   int (*tx_control_port)(struct wiphy *wiphy,
+  struct net_device *dev,
+  const u8 *buf, size_t len,
+  const u8 *dest, const __be16 proto,
+  const bool noencrypt);
 };
 
 /*
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 1334f810f7b4..77675ae3e475 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -5012,6 +5012,8 @@ enum nl80211_feature_flags {
  * @NL80211_EXT_FEATURE_LOW_SPAN_SCAN: Driver supports low span scan.
  * @NL80211_EXT_FEATURE_LOW_POWER_SCAN: Driver supports low power scan.
  * @NL80211_EXT_FEATURE_HIGH_ACCURACY_SCAN: Driver supports high accuracy scan.
+ * @NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211: Driver supports sending and
+ * receiving control port frames over NL80211 instead of the netdevice.
  *
  * @NUM_NL80211_EXT_FEATURES: number of extended features.
  * @MAX_NL80211_EXT_FEATURES: highest extended feature index.
@@ -5042,6 +5044,7 @@ enum nl80211_ext_feature_index {
NL80211_EXT_FEATURE_LOW_SPAN_SCAN,
NL80211_EXT_FEATURE_LOW_POWER_SCAN,
NL80211_EXT_FEATURE_HIGH_ACCURACY_SCAN,
+   NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211,
 
/* add new features before the definition below */
NUM_NL80211_EXT_FEATURES,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index d7dcc2d05025..3c4dbfa0ca71 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -12517,6 +12517,67 @@ static int nl80211_external_auth(struct sk_buff *skb, 
struct genl_info *info)
return rdev_external_auth(rdev, dev, );
 }
 
+static int nl80211_tx_control_port(struct sk_buff *skb, struct genl_info *info)
+{
+   struct cfg80211_registered_device *rdev = info->user_ptr[0];
+   struct net_device *dev = info->user_ptr[1];
+   struct wireless_dev *wdev = dev->ieee80211_ptr;
+   const u8 *buf;
+   size_t len;
+   u8 *dest;
+   u16 proto;
+   bool noencrypt;
+   int err;
+
+   if (!wiphy_ext_feature_isset(>wiphy,
+
NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211))
+   return -EOPNOTSUPP;
+
+   if (!rdev->ops->tx_control_port)
+   return -EOPNOTSUPP;
+
+   if (!info->attrs[NL80211_ATTR_FRAME] ||
+   !info->attrs[NL80211_ATTR_MAC] ||
+   !info->attrs[NL80211_ATTR_CONTROL_PORT_ETHERTYPE])
+   return -EINVAL;
+
+   wdev_lock(wdev);
+
+   switch (wdev->iftype) {
+   case NL80211_IFTYPE_AP:
+   case NL80211_IFTYPE_AP_VLAN:
+   case NL80211_IFTYPE_P2P_GO:
+   case NL80211_IFTYPE_MESH_POINT:
+   break;
+   case NL80211_IFTYPE_ADHOC:
+   case NL80211_IFTYPE_STATION:
+   case NL80211_IFTYPE_P2P_CLIENT:
+   if (wdev->current_bss)
+   break;
+   err = -ENOTCONN;
+   goto out;
+   default:
+   err = -EOPNOTSUPP;
+   goto out;
+   }
+
+   wdev_unlock(wdev);
+
+   buf = nla_data(info->attrs[NL80211_ATTR_FRAME]);
+   len = nla_len(info->attrs[NL80211_ATTR_FRAME]);
+   

[RFC v5 7/9] nl80211: Add SOCKET_OWNER support to START_AP

[Denis Kenzior]

Signed-off-by: Denis Kenzior 
---
 include/uapi/linux/nl80211.h | 2 ++
 net/wireless/ap.c| 1 +
 net/wireless/nl80211.c   | 4 
 3 files changed, 7 insertions(+)

diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index e3329bc4644b..9b4fd4bca141 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1989,6 +1989,8 @@ enum nl80211_commands {
  * torn down when the socket is closed.
  * If set during %NL80211_CMD_JOIN_MESH the mesh setup will be
  * automatically torn down when the socket is closed.
+ * If set during %NL80211_CMD_START_AP the AP will be automatically
+ * disabled when the socket is closed.
  *
  * @NL80211_ATTR_TDLS_INITIATOR: flag attribute indicating the current end is
  * the TDLS link initiator.
diff --git a/net/wireless/ap.c b/net/wireless/ap.c
index 63682176c96c..882d97bdc6bf 100644
--- a/net/wireless/ap.c
+++ b/net/wireless/ap.c
@@ -27,6 +27,7 @@ int __cfg80211_stop_ap(struct cfg80211_registered_device 
*rdev,
 
err = rdev_stop_ap(rdev, dev);
if (!err) {
+   wdev->conn_owner_nlportid = 0;
wdev->beacon_interval = 0;
memset(>chandef, 0, sizeof(wdev->chandef));
wdev->ssid_len = 0;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index e38d55cf34f7..bbdcc61a738c 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -4135,6 +4135,10 @@ static int nl80211_start_ap(struct sk_buff *skb, struct 
genl_info *info)
wdev->chandef = params.chandef;
wdev->ssid_len = params.ssid_len;
memcpy(wdev->ssid, params.ssid, wdev->ssid_len);
+
+   if (info->attrs[NL80211_ATTR_SOCKET_OWNER])
+   wdev->conn_owner_nlportid = info->snd_portid;
+
}
wdev_unlock(wdev);
 
-- 
2.13.5

[RFC v5 0/9] EAPoL over NL80211

[Denis Kenzior]

This patchset adds support for running 802.11 authentication mechanisms (e.g.
802.1X, 4-Way Handshake, etc) over NL80211 instead of putting them onto the
network device.  This has the advantage of fixing several long-standing race
conditions that result from userspace operating on multiple transports in order
to manage a 802.11 connection (e.g. NL80211 and wireless netdev, wlan0, etc).

For example, userspace would sometimes see 4-Way handshake packets before
NL80211 signaled that the connection has been established.  Leading to ugly
hacks or having the STA wait for retransmissions from the AP.

This also provides a way to mitigate a particularly nasty race condition where
the encryption key could be set prior to the 4-way handshake packet 4/4 being
sent.  This would result in the packet being sent encrypted and discarded by
the peer.  The mitigation strategy for this race is for userspace to explicitly
tell the kernel that a particular EAPoL packet should not be encrypted.

To make this possible this patchset introduces a new NL80211 command and several
new attributes.  A userspace that is capable of processing EAPoL packets over
NL80211 includes a new NL80211_ATTR_CONTROL_PORT_OVER_NL80211 attribute in its
NL80211_CMD_ASSOCIATE or NL80211_CMD_CONNECT requests being sent to the kernel.
The previously added NL80211_ATTR_SOCKET_OWNER attribute must also be included.
The latter is used by the kernel to send NL80211_CMD_CONTROL_PORT_FRAME
notifications back to userspace via a netlink unicast.  If the
NL80211_ATTR_CONTROL_PORT_OVER_NL80211 attribute is not specified, then legacy
behavior is kept and control port packets continue to flow over the network
interface.

If control port over nl80211 transport is requested, then control port packets
are intercepted just prior to being handed to the network device and sent over
netlink via the NL80211_CMD_CONTROL_PORT_FRAME notification.
NL80211_ATTR_CONTROL_PORT_ETHERTYPE and NL80211_ATTR_MAC are included to
specify the control port frame protocol and source address respectively.  If
the control port frame was received unencrypted then
NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT flag is also included.  NL80211_ATTR_FRAME
attribute contains the raw control port frame with all transport layer headers
stripped (e.g. this would be the raw EAPoL frame).

Userspace can reply to control port frames either via legacy methods (by sending
frames to the network device) or via NL80211_CMD_CONTROL_PORT_FRAME request.
Userspace would included NL80211_ATTR_FRAME with the raw control port frame as
well as NL80211_Attr_MAC and NL80211_ATTR_CONTROL_PORT_ETHERTYPE attributes to
specify the destination address and protocol respectively.  This allows
Pre-Authentication (protocol 0x88c7) frames to be sent via this mechanism as
well.  Finally, NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT flag can be included to
tell the driver to send the frame unencrypted, e.g. for 4-Way handshake 4/4
frames.

The proposed patchset has been tested in a mac80211_hwsim based environment with
hostapd and iwd.

ChangeLog

v5

- Johannes' main comment was that we're not handling interface types other than
STATION inside tx_control_port (patch 2).  This patch was modified to support
all interface types that seemed relevant.
- Since tx_control_port relies on wdev->conn_owner_nlportid being set,
SOCKET_OWNER support was added to JOIN_IBSS, JOIN_MESH and START_AP
- SOCKET_OWNER auto-destruction logic was updated to support interface types
other than STATION/P2P_CLIENT
- Last patch was modified to support control_port_over_nl80211 for mac80211
based AP mode.  It also copies necessary bits for AP_VLAN interfaces.

This version has been tested on both STATION and AP mode interfaces with
SOCKET_OWNER & CONTROL_PORT_OVER_NL80211 attributes provided to CMD_CONNECT
and CMD_START_AP.

TODO:

- It is unclear to me how AP_VLAN and AP interfaces should synchronize on
conn_owner_nlportid.  This is required for tx_control_port to work.
- JOIN_IBSS & JOIN_MESH don't seem to support control_port_ethertype or
control_port_no_encrypt.  Should struct cfg80211_crypto_settings parsed inside
nl80211_crypto_settings be added to ibss_params or mesh_config/mesh_setup?

v4

- Reordered the patches to make sure that: when CONTROL_PORT_OVER_NL80211 is
provided by userspace, nl80211 checks that both EXT_FEATURE bit is set and
the tx_control_port is present in rdev ops.
- Fixed up various issues Johannes found in his review

v3

- Added ETH_P_PREAUTH to if_ether.h
- Moved NL80211 feature bit from wiphy features to ext features
- Addressed various comments from Johannes

v2

- Added WIPHY_FLAG_CONTROL_PORT_OVER_NL80211 flag.  This is a capability flag
used by the drivers, e.g. that the driver supports control port over nl80211
capability.  This capability is now checked when CONTROL_PORT_OVER_NL80211 is
requested.

- mac80211 rx path now forwards Pre-Authentication frames over NL80211 as well,
if requested.  Tweaked the signature of 

[RFC v5 1/9] nl80211: Add CMD_CONTROL_PORT_FRAME API

[Denis Kenzior]

This commit also adds cfg80211_rx_control_port function.  This is used
to generate a CMD_CONTROL_PORT_FRAME event out to userspace.  The
conn_owner_nlportid is used as the unicast destination.  This means that
userspace must specify NL80211_ATTR_SOCKET_OWNER flag if control port
over nl80211 routing is requested in NL80211_CMD_CONNECT,
NL80211_CMD_ASSOCIATE or NL80211_CMD_START_AP

Signed-off-by: Denis Kenzior 
---
 include/net/cfg80211.h   | 22 +
 include/uapi/linux/nl80211.h | 13 ++
 net/wireless/nl80211.c   | 58 
 net/wireless/trace.h | 21 
 4 files changed, 114 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index fc40843baed3..6dee630ee66d 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -5694,6 +5694,28 @@ void cfg80211_mgmt_tx_status(struct wireless_dev *wdev, 
u64 cookie,
 
 
 /**
+ * cfg80211_rx_control_port - notification about a received control port frame
+ * @dev: The device the frame matched to
+ * @buf: control port frame
+ * @len: length of the frame data
+ * @addr: The peer from which the frame was received
+ * @proto: frame protocol, typically PAE or Pre-authentication
+ * @unencrypted: Whether the frame was received unencrypted
+ *
+ * This function is used to inform userspace about a received control port
+ * frame.  It should only be used if userspace indicated it wants to receive
+ * control port frames over NL80211.
+ *
+ * The frame is the data portion of the 802.3 or 802.11 data frame with all
+ * network layer headers removed (e.g. the raw EAPoL frame).
+ *
+ * Return: %true if the frame was passed to userspace
+ */
+bool cfg80211_rx_control_port(struct net_device *dev,
+ const u8 *buf, size_t len,
+ const u8 *addr, u16 proto, bool unencrypted);
+
+/**
  * cfg80211_cqm_rssi_notify - connection quality monitoring rssi event
  * @dev: network device
  * @rssi_event: the triggered RSSI event
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index c13c84304be3..1334f810f7b4 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -990,6 +990,17 @@
  * _CMD_CONNECT or _CMD_ROAM. If the 4 way handshake failed
  * _CMD_DISCONNECT should be indicated instead.
  *
+ * @NL80211_CMD_CONTROL_PORT_FRAME: Control Port (e.g. PAE) frame TX request
+ * and RX notification.  This command is used both as a request to transmit
+ * a control port frame and as a notification that a control port frame
+ * has been received. %NL80211_ATTR_FRAME is used to specify the
+ * frame contents.  The frame is the raw EAPoL data, without ethernet or
+ * 802.11 headers.
+ * When used as an event indication %NL80211_ATTR_CONTROL_PORT_ETHERTYPE,
+ * %NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT and %NL80211_ATTR_MAC are added
+ * indicating the protocol type of the received frame; whether the frame
+ * was received unencrypted and the MAC address of the peer respectively.
+ *
  * @NL80211_CMD_RELOAD_REGDB: Request that the regdb firmware file is reloaded.
  *
  * @NL80211_CMD_EXTERNAL_AUTH: This interface is exclusively defined for host
@@ -1228,6 +1239,8 @@ enum nl80211_commands {
 
NL80211_CMD_STA_OPMODE_CHANGED,
 
+   NL80211_CMD_CONTROL_PORT_FRAME,
+
/* add new commands above here */
 
/* used to define NL80211_CMD_MAX below */
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index a910150f8169..d7dcc2d05025 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -14535,6 +14535,64 @@ void cfg80211_mgmt_tx_status(struct wireless_dev 
*wdev, u64 cookie,
 }
 EXPORT_SYMBOL(cfg80211_mgmt_tx_status);
 
+static int __nl80211_rx_control_port(struct net_device *dev,
+const u8 *buf, size_t len,
+const u8 *addr, u16 proto,
+bool unencrypted, gfp_t gfp)
+{
+   struct wireless_dev *wdev = dev->ieee80211_ptr;
+   struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
+   struct sk_buff *msg;
+   void *hdr;
+   u32 nlportid = READ_ONCE(wdev->conn_owner_nlportid);
+
+   if (!nlportid)
+   return -ENOENT;
+
+   msg = nlmsg_new(100 + len, gfp);
+   if (!msg)
+   return -ENOMEM;
+
+   hdr = nl80211hdr_put(msg, 0, 0, 0, NL80211_CMD_CONTROL_PORT_FRAME);
+   if (!hdr) {
+   nlmsg_free(msg);
+   return -ENOMEM;
+   }
+
+   if (nla_put_u32(msg, NL80211_ATTR_WIPHY, rdev->wiphy_idx) ||
+   nla_put_u32(msg, NL80211_ATTR_IFINDEX, dev->ifindex) ||
+   nla_put_u64_64bit(msg, NL80211_ATTR_WDEV, wdev_id(wdev),
+ NL80211_ATTR_PAD) ||
+   nla_put(msg, NL80211_ATTR_FRAME, len, buf) ||
+   nla_put(msg, NL80211_ATTR_MAC, 

Re: [RESEND] rsi: Remove stack VLA usage

[Andy Shevchenko]

On Tue, Mar 13, 2018 at 10:17 PM, tcharding  wrote:
> On Mon, Mar 12, 2018 at 09:46:06AM +, Kalle Valo wrote:
>> tcharding  wrote:

I'm pretty much sure it depends on the original email headers, like
above ^^^ — no name.
Perhaps git config on your side should be done.

-- 
With Best Regards,
Andy Shevchenko

Re: [2/3] mwifiex: support sysfs initiated device coredump

[Arend van Spriel]

On 3/13/2018 9:19 PM, Marcel Holtmann wrote:

Hi Arend,


Since commit 3c47d19ff4dc ("drivers: base: add coredump driver ops")
it is possible to initiate a device coredump from user-space. This
patch adds support for it adding the .coredump() driver callback.
As there is no longer a need to initiate it through debugfs remove
that code.

Signed-off-by: Arend van Spriel 


Based on the discussion I assume this is ok to take to w-d-next. If that's not
the case, please let me know ASAP.


It is up to the mwifiex maintainers to decide, I guess. The ABI
documentation need to be revised and change the callback to void
return type. I am not sure what the best approach is. 1) apply this
and fix return type later, or 2) fix return type and resubmit this.
What is your opinion?


I guess the callback change will go through Greg's tree? Then I suspect
it's easier that you submit the callback change to Greg first and wait
for it to trickle down to wireless-drivers-next (after the next merge
window) and then I can apply the driver patches. Otherwise there might
be a conflict between my and Greg's tree.


That was my assessment, but unfortunately Marcel already applied the btmrvl 
patch before I could reply. So how do I move from here? Option 1) revert brmrvl 
and fix callback return type, or 2) apply mwifiex patch and fix callback return 
type later for both drivers.


I can take the patch back out of bluetooth-next if needed. It is your call.


Thanks, Marcel

Let's go for that. Please revert/remove the patch.

Regards,
Arend

Re: [PATCH] brcmfmac: Add support for getting nvram contents from EFI variables

[Arend van Spriel]

On 3/12/2018 10:45 AM, Hans de Goede wrote:

Hi,

On 12-03-18 09:55, Arend van Spriel wrote:

On 3/11/2018 10:47 PM, Hans de Goede wrote:

Various models Asus laptops with a SDIO attached brcmfmac wifi chip,
store
the nvram contents in a special EFI variable. This commit adds
support for
getting nvram directly from this EFI variable, without the user
needing to
manually copy it.

This makes Wifi / Bluetooth work out of the box on these devices
instead of
requiring manual setup.

Note that at least on the Asus T200TA the nvram from the EFI variable
wrongly contains "ccode=ALL" which the firmware does not understand, the
code to fetch the nvram from the EFI variable will fix this up to:
"ccode=XV" which is the correct way to specify the worldwide broadcast
regime.

This has been tested on the following models: Asus T100CHI, Asus T100HA,
Asus T100TA and Asus T200TA


Hi Hans,

I like to have this as well, but  (see below)


Tested-by: Hans de Goede 


Duh. I would expect anyone to have tested their patches although you
can not cover every system out there obviously :-p


Heh, I added it in this case as I went to the trouble of finding 4 devices
which use this and test it on all 4 devices.


Not really a problem, but it looked funny :-)


Signed-off-by: Hans de Goede 
---
  .../broadcom/brcm80211/brcmfmac/firmware.c | 68
+-
  1 file changed, 67 insertions(+), 1 deletion(-)

diff --git
a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
index 091b52979e03..cbac407ae132 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
@@ -14,6 +14,8 @@
   * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
   */

+#include 
+#include 
  #include 
  #include 
  #include 
@@ -446,6 +448,67 @@ struct brcmf_fw {
   void *nvram_image, u32 nvram_len);
  };

+#ifdef CONFIG_EFI
+static u8 *brcmf_fw_nvram_from_efi(size_t *data_len_ret)
+{
+const u16 name[] = { 'n', 'v', 'r', 'a', 'm', 0 };


Isn't there some helper function to make this conversion to u16 array?
Hmm, maybe it is my itch to scratch later :-)


Nope, I copied this style from existing code under drivers/firmware/efi


Maybe I add a helper function at some point.


+struct efivar_entry *nvram_efivar;
+unsigned long data_len = 0;
+u8 *data = NULL;
+char *ccode;
+int err;
+
+/* So far only Asus devices store the nvram in an EFI var */
+if (!dmi_name_in_vendors("ASUSTeK COMPUTER INC."))
+return NULL;


Actually had a Sony device with nvram in EFI. Why not just drop this
optimization.


Ok, do you know if that variable had the same name and guid though ?
Because
if it doesn't then this code is not going to work for the Sony case.


If I am not mistaken the name and guid are defined by Broadcom/Microsoft.


Anyways the overhead is small and this only runs once, so I will drop the
check for v2.


Due to XV issue we may want to keep the check for now.


+nvram_efivar = kzalloc(sizeof(*nvram_efivar), GFP_KERNEL);
+if (!nvram_efivar)
+return NULL;
+
+memcpy(_efivar->var.VariableName, name, sizeof(name));
+nvram_efivar->var.VendorGuid = EFI_GUID(0x74b00bd9, 0x805a, 0x4d61,
+0xb5, 0x1f, 0x43, 0x26,
+0x81, 0x23, 0xd1, 0x13);
+
+err = efivar_entry_size(nvram_efivar, _len);
+if (err)
+goto fail;
+
+data = kmalloc(data_len, GFP_KERNEL);
+if (!data)
+goto fail;
+
+err = efivar_entry_get(nvram_efivar, NULL, _len, data);
+if (err)
+goto fail;
+
+/* In some cases the EFI-var stored nvram contains "ccode=ALL" but
+ * the firmware does not understand "ALL" change this to "XV" which
+ * is the correct way to specify the "worldwide" compatible
settings.
+ */


This is actually quite bad. The ALL ccode should never end up on the
market as it is intended for internal use only during bringup project
phase so these seems to be programmed wrong.


I see lots of cheap Windows 10 and ARM devices with brcmfmac sdio wifi
which ship with on disk nvram files using the "ALL" ccode. I actually have
pinned my home wifi at channel 13 to catch this, because channel 13 does
not work with the ALL ccode. If I understand correctly the worldwide
domain starts with channel 13/14 in passive/listen only mode and allows
using them once it has seen valid wifi traffic on them, so they do allow
communicating with an AP on channel 13 here in the Netherlands where that
is allowed?


*sigh* (regarding ALL being shipped)


Also simply selecting XV instead is not correct. There is not just one
worldwide domain in the regulatory database of the firmware image.


Right, I've read elsewhere that "X2" is the right magic value to use and
I've tested that on some other devices and that does seem to work.

I've 

Re: [2/3] mwifiex: support sysfs initiated device coredump

[Marcel Holtmann]

Hi Arend,

> Since commit 3c47d19ff4dc ("drivers: base: add coredump driver ops")
> it is possible to initiate a device coredump from user-space. This
> patch adds support for it adding the .coredump() driver callback.
> As there is no longer a need to initiate it through debugfs remove
> that code.
> 
> Signed-off-by: Arend van Spriel 
 
 Based on the discussion I assume this is ok to take to w-d-next. If that's 
 not
 the case, please let me know ASAP.
>>> 
>>> It is up to the mwifiex maintainers to decide, I guess. The ABI
>>> documentation need to be revised and change the callback to void
>>> return type. I am not sure what the best approach is. 1) apply this
>>> and fix return type later, or 2) fix return type and resubmit this.
>>> What is your opinion?
>> 
>> I guess the callback change will go through Greg's tree? Then I suspect
>> it's easier that you submit the callback change to Greg first and wait
>> for it to trickle down to wireless-drivers-next (after the next merge
>> window) and then I can apply the driver patches. Otherwise there might
>> be a conflict between my and Greg's tree.
> 
> That was my assessment, but unfortunately Marcel already applied the btmrvl 
> patch before I could reply. So how do I move from here? Option 1) revert 
> brmrvl and fix callback return type, or 2) apply mwifiex patch and fix 
> callback return type later for both drivers.

I can take the patch back out of bluetooth-next if needed. It is your call.

Regards

Marcel

Re: [RESEND] rsi: Remove stack VLA usage

[tcharding]

On Mon, Mar 12, 2018 at 09:46:06AM +, Kalle Valo wrote:
> tcharding  wrote:
> 
> > The kernel would like to have all stack VLA usage removed[1].  rsi uses
> > a VLA based on 'blksize'.  Elsewhere in the SDIO code maximum block size
> > is defined using a magic number.  We can use a pre-processor defined
> > constant and declare the array to maximum size.  We add a check before
> > accessing the array in case of programmer error.
> > 
> > [1]: https://lkml.org/lkml/2018/3/7/621
> > 
> > Signed-off-by: Tobin C. Harding 
> 
> Tobin, your name in patchwork.kernel.org is just "tcharding" then it should be
> "Tobin C. Harding". Patchwork is braindead in a way as it takes the name from
> it's database instead of the From header of the patch in question.
> 
> I can fix that manually but it would be helpful if you could register to
> patchwork and fix your name during registration. You have only one chance to
> fix your name (another braindead feature!) so be careful :)

Hi Kalle,

I logged into my patchwork account but I don't see any way to set the
name.  Within 'profile' there is only 'change password' and 'link
email'.  I thought I could unregister then re-register but I can't see
how to do that either.  Is there a maintainer of patchwork.kernel.org
who I can email to manually remove me from the system?

thanks,
Tobin.

Re: [RESEND PATCH] rsi: Remove stack VLA usage

[Tobin C. Harding]

On Sun, Mar 11, 2018 at 09:06:10PM -0500, Larry Finger wrote:
> On 03/11/2018 08:43 PM, Tobin C. Harding wrote:
> >The kernel would like to have all stack VLA usage removed[1].  rsi uses
> >a VLA based on 'blksize'.  Elsewhere in the SDIO code maximum block size
> >is defined using a magic number.  We can use a pre-processor defined
> >constant and declare the array to maximum size.  We add a check before
> >accessing the array in case of programmer error.
> >
> >[1]: https://lkml.org/lkml/2018/3/7/621
> >
> >Signed-off-by: Tobin C. Harding 
> >---
> >
> >RESEND: add wireless mailing list to CC's (requested by Kalle)
> >
> >  drivers/net/wireless/rsi/rsi_91x_hal.c  | 13 +++--
> >  drivers/net/wireless/rsi/rsi_91x_sdio.c |  9 +++--
> >  2 files changed, 14 insertions(+), 8 deletions(-)
> >
> >diff --git a/drivers/net/wireless/rsi/rsi_91x_hal.c 
> >b/drivers/net/wireless/rsi/rsi_91x_hal.c
> >index 1176de646942..839ebdd602df 100644
> >--- a/drivers/net/wireless/rsi/rsi_91x_hal.c
> >+++ b/drivers/net/wireless/rsi/rsi_91x_hal.c
> >@@ -641,7 +641,7 @@ static int ping_pong_write(struct rsi_hw *adapter, u8 
> >cmd, u8 *addr, u32 size)
> > u32 cmd_addr;
> > u16 cmd_resp, cmd_req;
> > u8 *str;
> >-int status;
> >+int status, ret;
> > if (cmd == PING_WRITE) {
> > cmd_addr = PING_BUFFER_ADDRESS;
> >@@ -655,12 +655,13 @@ static int ping_pong_write(struct rsi_hw *adapter, u8 
> >cmd, u8 *addr, u32 size)
> > str = "PONG_VALID";
> > }
> >-status = hif_ops->load_data_master_write(adapter, cmd_addr, size,
> >+ret = hif_ops->load_data_master_write(adapter, cmd_addr, size,
> > block_size, addr);
> >-if (status) {
> >-rsi_dbg(ERR_ZONE, "%s: Unable to write blk at addr %0x\n",
> >-__func__, *addr);
> >-return status;
> >+if (ret) {
> >+if (ret != -EINVAL)
> >+rsi_dbg(ERR_ZONE, "%s: Unable to write blk at addr 
> >%0x\n",
> >+__func__, *addr);
> >+return ret;
> > }
> > status = bl_cmd(adapter, cmd_req, cmd_resp, str);
> >diff --git a/drivers/net/wireless/rsi/rsi_91x_sdio.c 
> >b/drivers/net/wireless/rsi/rsi_91x_sdio.c
> >index b0cf41195051..b766578b591a 100644
> >--- a/drivers/net/wireless/rsi/rsi_91x_sdio.c
> >+++ b/drivers/net/wireless/rsi/rsi_91x_sdio.c
> >@@ -20,6 +20,8 @@
> >  #include "rsi_common.h"
> >  #include "rsi_hal.h"
> >+#define RSI_MAX_BLOCK_SIZE 256
> >+
> >  /**
> >   * rsi_sdio_set_cmd52_arg() - This function prepares cmd 52 read/write arg.
> >   * @rw: Read/write
> >@@ -362,7 +364,7 @@ static int rsi_setblocklength(struct rsi_hw *adapter, 
> >u32 length)
> > rsi_dbg(INIT_ZONE, "%s: Setting the block length\n", __func__);
> > status = sdio_set_block_size(dev->pfunction, length);
> >-dev->pfunction->max_blksize = 256;
> >+dev->pfunction->max_blksize = RSI_MAX_BLOCK_SIZE;
> > adapter->block_size = dev->pfunction->max_blksize;
> > rsi_dbg(INFO_ZONE,
> >@@ -567,9 +569,12 @@ static int rsi_sdio_load_data_master_write(struct 
> >rsi_hw *adapter,
> >  {
> > u32 num_blocks, offset, i;
> > u16 msb_address, lsb_address;
> >-u8 temp_buf[block_size];
> >+u8 temp_buf[RSI_MAX_BLOCK_SIZE];
> > int status;
> >+if (block_size > RSI_MAX_BLOCK_SIZE)
> >+return -EINVAL;
> >+
> > num_blocks = instructions_sz / block_size;
> > msb_address = base_address >> 16;
> 
> I am not giving this patch a negative review, but my solution to the same
> problem has been to change the on-stack array into a u8 pointer, use
> kmalloc() to assign the space, and then free that space at the end. That way
> large stack allocations are avoided, with a minimum of changes.

Your idea is better Larry, have you got a patch done already or do you
want me to knock one up?

thanks,
Tobin.

Re: [2/3] mwifiex: support sysfs initiated device coredump

[Arend van Spriel]

On 3/13/2018 2:10 PM, Kalle Valo wrote:

Arend van Spriel  writes:


On 3/12/2018 10:41 AM, Kalle Valo wrote:

Arend Van Spriel  wrote:


Since commit 3c47d19ff4dc ("drivers: base: add coredump driver ops")
it is possible to initiate a device coredump from user-space. This
patch adds support for it adding the .coredump() driver callback.
As there is no longer a need to initiate it through debugfs remove
that code.

Signed-off-by: Arend van Spriel 


Based on the discussion I assume this is ok to take to w-d-next. If that's not
the case, please let me know ASAP.


It is up to the mwifiex maintainers to decide, I guess. The ABI
documentation need to be revised and change the callback to void
return type. I am not sure what the best approach is. 1) apply this
and fix return type later, or 2) fix return type and resubmit this.
What is your opinion?


I guess the callback change will go through Greg's tree? Then I suspect
it's easier that you submit the callback change to Greg first and wait
for it to trickle down to wireless-drivers-next (after the next merge
window) and then I can apply the driver patches. Otherwise there might
be a conflict between my and Greg's tree.


That was my assessment, but unfortunately Marcel already applied the 
btmrvl patch before I could reply. So how do I move from here? Option 1) 
revert brmrvl and fix callback return type, or 2) apply mwifiex patch 
and fix callback return type later for both drivers.


Regards,
Arend

Re: Support on vendor id and device id

[Arend van Spriel]

+ Greg, +wilc1000 maintainers

On 3/1/2018 11:10 AM, Harsha Rao wrote:

On Wed, Feb 28, 2018 at 3:08 PM, Arend van Spriel
 wrote:

On 2/28/2018 12:14 AM, Harsha Rao wrote:


My suspicion is that your device, is fundamentally a wilc1000 and that

the existing wilc1000 driver will likely largely work for it and all
you really need to do is modify the existing driver to handle the
quirks of your particular implementation of the wilc1000 chip. And,
often WiFi chips will let you change the VID/PID somewhere within
whatever non-volatile storage it has (like where it stores the MAC
address).





So it seems the wilc1000 devices from Microchip/Atmel are also using a
vendor id they did not buy. Could be that the mentioned 3rd party
providing
the SDIO IP actually owns that vendor id, but if you are building your
wifi
chip on that you should better buy you own vendor id from the SD
Association. Now if Harsha is actually working for Microchip (unclear to
me)
there is basically one party that should go shopping.



I would like to clarify that I am not building anything on top of
microchip wifi device.
We have a different HW . Its been just that 3rd party vendor providing
SDIO IP has given
same ID to different customers.



So it is as I said, ie. you are using the 3rd party SDIO IP as is and add
your own wifi IP to it? So what does the term "SDIO IP" mean here. Is it a
piece of hardware that you hook up to your wifi hardware or is it
VHDL/verilog in which the vendor id is defined. If it is VHDL you should
really get your own vendor id from the SD Association and fix it. Otherwise,
the 3rd party hardware should have means to change it. If not, you better
find another party.

Regards,
Arend


Thank you folks for your comments.
The SDIO IP is VHDL IP core integrated on our SoC. And we figured out
a way to update vendor ID at run-time during boot.
We would get our own vendor ID  from SD association and proceed .


Coming back to this thread as it seems that wilc1000 has exactly the 
same issue or blindly reusing the SDIO vendor id. Below excerpt from 
earlier email in this thread:


"""
>> In theory the vendor IDs shouldn't be duplicated on fundamentally
>> different devices, assuming that the manufacturers are doing things
>> "right". The VID is paid for by buying it from the SD Association.

Indeed. And this is fun already. If the sdio.ids file in systemd has any 
value the vendor id 0x296 is assigned to:


0296  GCT Semiconductor, Inc.
5347  GDM72xx WiMAX
"""

So claiming it for wilc1000 seems wrong unless GCT and Microchip are 
actually the same company, but I could not find any evidence for that. 
The bad news is probably that this device is already on the market :-(


Regards,
Arend

Re: [PATCH 08/10] rsi: device disconnect changes

[Kalle Valo]

Amitkumar Karwar  writes:

> From: Prameela Rani Garnepudi 
>
> Below changes are done:
> 1. When HCI detach is called, making bt_adapter null and checking
> for this variable where ever required.
> 2. In USB case, one extra register write is added to disable
> firmware watchdog.
> 3. When interface down is called as part of disconnect, don't
> send RX filter frame.

Why? What does this patch fix? Please explain that (symptoms) in the
commit log.

-- 
Kalle Valo

Re: [PATCH 09/10] rsi: tx improvements

[Kalle Valo]

Amitkumar Karwar  writes:

> From: Prameela Rani Garnepudi 
>
> Changes done to improve TX path:
> 1. Max number of frames to aggregate is increased to 8
> 2. Bootup parameters updated.
> 3. registering 5g band is done only when device supports
> 4. Vif is properly taken to enable power save.
> 5. When coex mode exist, power save on by default is set

If you have a list of changes in the commit log to me that's a strong
indication that you need to split the patches.

Also I don't see any answers to question "Why?".

-- 
Kalle Valo

Re: [1/3] Revert "mwifiex: fix incorrect ht capability problem"

[Kalle Valo]

Ganapathi Bhat  wrote:

> This reverts commit bcc920e8f08336cbbdcdba7c4449c27137e6b4b9.
> 
> Drivers gets hardware info and updates ht_cap field of
> wiphy->bands during initialization. Once updated during init,
> ht_cap must not be modified as it reflects the capability
> supported by hardwawre. Above patch tries to modify the ht_cap
> field and this results in wrongly advertising capabilities during
> association.
> 
> Signed-off-by: Cathy Luo 
> Signed-off-by: Ganapathi Bhat 

3 patches applied to wireless-drivers-next.git, thanks.

53a7094204b7 Revert "mwifiex: fix incorrect ht capability problem"
77423fa73927 mwifiex: fix incorrect ht capability problem
28bf8312a983 mwifiex: get_channel from firmware

-- 
https://patchwork.kernel.org/patch/10271595/

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

Re: mwifiex: correct antenna number with high bits reserved

[Kalle Valo]

Xinming Hu  wrote:

> High bits of antenna number are reserved in hardware spec,
> using low 4 bits represent supported antenna.
> 
> Signed-off-by: Cathy Luo 
> Signed-off-by: Xinming Hu 

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

eaab43e505d0 mwifiex: correct antenna number with high bits reserved

-- 
https://patchwork.kernel.org/patch/10266469/

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

Re: [RESEND] rsi: Remove stack VLA usage

[Kalle Valo]

"Tobin C. Harding"  wrote:

> The kernel would like to have all stack VLA usage removed[1].  rsi uses
> a VLA based on 'blksize'.  Elsewhere in the SDIO code maximum block size
> is defined using a magic number.  We can use a pre-processor defined
> constant and declare the array to maximum size.  We add a check before
> accessing the array in case of programmer error.
> 
> [1]: https://lkml.org/lkml/2018/3/7/621
> 
> Signed-off-by: Tobin C. Harding 

There were conflicts. Can you rebase on top of wireless-drivers-next and
resend, please?

Recorded preimage for 'drivers/net/wireless/rsi/rsi_91x_sdio.c'
error: Failed to merge in the changes.
Applying: rsi: Remove stack VLA usage
Using index info to reconstruct a base tree...
M   drivers/net/wireless/rsi/rsi_91x_hal.c
M   drivers/net/wireless/rsi/rsi_91x_sdio.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/net/wireless/rsi/rsi_91x_sdio.c
CONFLICT (content): Merge conflict in drivers/net/wireless/rsi/rsi_91x_sdio.c
Auto-merging drivers/net/wireless/rsi/rsi_91x_hal.c
Patch failed at 0001 rsi: Remove stack VLA usage
The copy of the patch that failed is found in: .git/rebase-apply/patch

Patch set to Changes Requested.

-- 
https://patchwork.kernel.org/patch/10274983/

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

Re: net/wireless: fix spaces and grammar copy/paste in vendor Kconfig help text

[Kalle Valo]

Randy Dunlap  wrote:

> From: Randy Dunlap 
> 
> Lots of the wireless driver vendor Kconfig symol help text says
> "questions about  cards." (2 spaces between "about" and "cards")
> 
> Besides dropping one of those spaces, it also needs some other word
> inserted there. Instead of putting each vendor's name there, I chose
> to say "these" cards in all of the Kconfig help text.
> 
> Cc: Kalle Valo 
> Signed-off-by: Randy Dunlap 

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

a6cf02e6485a net/wireless: fix spaces and grammar copy/paste in vendor Kconfig 
help text

-- 
https://patchwork.kernel.org/patch/10255933/

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

Re: [v3] ssb: use put_device() if device_register fail

[Kalle Valo]

Arvind Yadav  wrote:

> Never directly free @dev after calling device_register(), even
> if it returned an error! Always use put_device() to give up the
> reference initialized.
> 
> Signed-off-by: Arvind Yadav 

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

a24853aab591 ssb: use put_device() if device_register fail

-- 
https://patchwork.kernel.org/patch/10266711/

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

Re: [v2] bcma: Prevent build of PCI host features in module

[Kalle Valo]

Matt Redfearn  wrote:

> Attempting to build bcma.ko with BCMA_DRIVER_PCI_HOSTMODE=y results in
> a build error due to use of symbols not exported from vmlinux:
> 
> ERROR: "pcibios_enable_device" [drivers/bcma/bcma.ko] undefined!
> ERROR: "register_pci_controller" [drivers/bcma/bcma.ko] undefined!
> make[1]: *** [scripts/Makefile.modpost:92: __modpost] Error 1
> 
> To prevent this, don't allow the host mode feature to be built if
> CONFIG_BCMA=m
> 
> Signed-off-by: Matt Redfearn 

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

79ca239a68f8 bcma: Prevent build of PCI host features in module

-- 
https://patchwork.kernel.org/patch/10250739/

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

Re: [PATCH] firmware: add a function to load optional firmware v2

[Kalle Valo]

"Luis R. Rodriguez"  writes:

> On Tue, Mar 13, 2018 at 03:16:34PM +0200, Kalle Valo wrote:
>> "Luis R. Rodriguez"  writes:
>> 
>> >> +/**
>> >> + * request_firmware_optional: - request for an optional fw module
>> >> + * @firmware_p: pointer to firmware image
>> >> + * @name: name of firmware file
>> >> + * @device: device for which firmware is being loaded
>> >> + *
>> >> + * This function is similar in behaviour to request_firmware(), except
>> >> + * it doesn't produce warning messages when the file is not found.
>> >> + **/
>> >> +int
>> >> +request_firmware_optional(const struct firmware **firmware_p, const char 
>> >> *name,
>> >> + struct device *device)
>> >> +{
>> >> +   int ret;
>> >> +
>> >> +   /* Need to pin this module until return */
>> >> +   __module_get(THIS_MODULE);
>> >> +   ret = _request_firmware(firmware_p, name, device, NULL, 0,
>> >> +   FW_OPT_UEVENT | FW_OPT_NO_WARN );
>> >> +   module_put(THIS_MODULE);
>> >> +   return ret;
>> >> +}
>> >> +EXPORT_SYMBOL(request_firmware_optional);
>> >
>> > New exported symbols for the firmware API should be EXPORT_SYMBOL_GPL().
>> 
>> To me the word optional feels weird to me. For example, in ath10k I
>> suspect we would be only calling request_firmware_optional() with all
>> firmware and not request_firmware() at all.
>> 
>> How about request_firmware_nowarn()? That would even match the
>> documentation above.
>
> _nowarn() works with me. Do you at least want the return value to give
> an error value if no file was found? This way the driver can decide
> when to issue an error if it wants to.

Yes, it would be very good to return the error value to ath10k. That way
we can give a proper error message to the user if we can't find a
suitable firmware image.

-- 
Kalle Valo

Re: [1/3] rsi: improve RX handling in SDIO interface

[Kalle Valo]

Amitkumar Karwar  wrote:

> From: Prameela Rani Garnepudi 
> 
> Currently, RX packets are handled in interrupt context in SDIO
> interface. To improve the efficiency of processing RX packets,
> RX thread and RX skb queues are introduced.
> When the packet is read from device, driver prepares skb, add to
> RX queue and trigger RX thread event. RX thread processes the
> packets from RX queue.
> 
> Signed-off-by: Prameela Rani Garnepudi 
> Signed-off-by: Amitkumar Karwar 

3 patches applied to wireless-drivers-next.git, thanks.

50117605770c rsi: improve RX handling in SDIO interface
8809f08cdc0b rsi: use dynamic RX control blocks instead of MAX_RX_URB
a1854fae1414 rsi: improve RX packet handling in USB interface

-- 
https://patchwork.kernel.org/patch/10246955/

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

Re: [PATCH] firmware: add a function to load optional firmware v2

[Luis R. Rodriguez]

On Tue, Mar 13, 2018 at 03:16:34PM +0200, Kalle Valo wrote:
> "Luis R. Rodriguez"  writes:
> 
> >> +/**
> >> + * request_firmware_optional: - request for an optional fw module
> >> + * @firmware_p: pointer to firmware image
> >> + * @name: name of firmware file
> >> + * @device: device for which firmware is being loaded
> >> + *
> >> + * This function is similar in behaviour to request_firmware(), except
> >> + * it doesn't produce warning messages when the file is not found.
> >> + **/
> >> +int
> >> +request_firmware_optional(const struct firmware **firmware_p, const char 
> >> *name,
> >> + struct device *device)
> >> +{
> >> +   int ret;
> >> +
> >> +   /* Need to pin this module until return */
> >> +   __module_get(THIS_MODULE);
> >> +   ret = _request_firmware(firmware_p, name, device, NULL, 0,
> >> +   FW_OPT_UEVENT | FW_OPT_NO_WARN );
> >> +   module_put(THIS_MODULE);
> >> +   return ret;
> >> +}
> >> +EXPORT_SYMBOL(request_firmware_optional);
> >
> > New exported symbols for the firmware API should be EXPORT_SYMBOL_GPL().
> 
> To me the word optional feels weird to me. For example, in ath10k I
> suspect we would be only calling request_firmware_optional() with all
> firmware and not request_firmware() at all.
> 
> How about request_firmware_nowarn()? That would even match the
> documentation above.

_nowarn() works with me. Do you at least want the return value to give
an error value if no file was found? This way the driver can decide
when to issue an error if it wants to.

  Luis

Re: [v9,1/8] rsi: add rx control block to handle rx packets in USB

[Kalle Valo]

Amitkumar Karwar  wrote:

> From: Prameela Rani Garnepudi 
> 
> Rx bluetooth endpoint shall be added in further patches. Rx control
> block is introduced here to handle Rx packets properly. Separate
> function is written to initialize the RX control blocks.
> 
> Signed-off-by: Prameela Rani Garnepudi 
> Signed-off-by: Siva Rebbagondla 
> Signed-off-by: Amitkumar Karwar 

8 patches applied to wireless-drivers-next.git, thanks.

1100f81bbcd1 rsi: add rx control block to handle rx packets in USB
a4302bff28e2 rsi: add bluetooth rx endpoint
4c10d56a76bb rsi: add header file rsi_91x
2108df3c4b18 rsi: add coex support
38aa4da50483 Bluetooth: btrsi: add new rsi bluetooth driver
716b840c7641 rsi: handle BT traffic in driver
898b25533931 rsi: add module parameter operating mode
681805b1401b rsi: sdio changes to support BT

-- 
https://patchwork.kernel.org/patch/10245547/

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

Re: [PATCH] firmware: add a function to load optional firmware v2

[Luis R. Rodriguez]

On Tue, Mar 13, 2018 at 03:39:23PM +0200, Kalle Valo wrote:
> "Luis R. Rodriguez"  writes:
> 
> > On Mon, Mar 12, 2018 at 12:10:47AM +0100, Arend van Spriel wrote:
> >> On 3/11/2018 5:05 PM, Andres Rodriguez wrote:
> >> > > Your patch series then should also have the driver callers who you
> >> > > want to modify to use this new API. Collect from the 802.11 folks the
> >> > > other drivers which I think they wanted changed as well.
> >> > 
> >> > Arend, Kalle, would love to hear your feedback.
> >> 
> >> I am not sure if it was ath10k, but Kalle will surely know. The other 
> >> driver
> >> firing a whole batch of firmware requests is iwlwifi. These basically try 
> >> to
> >> get latest firmware version and if not there try an older one.
> >
> > Ah I recall now. At least for iwlwifi its that it requests firmware with a
> > range of api files, and we don't need information about files in the middle
> > not found, given all we need to know if is if at lest one file was found
> > or not.
> >
> > I have future code to also enable use of a range request which would replace
> > the recursive nature of iwlwifi's firmware API request, so it simplifies it
> > considerably.
> >
> > Once we get this flag to be silent in, this can be used later. Ie, the new
> > API I'd add would replace the complex api revision thing for an internal 
> > set.
> 
> TBH I doubt we would use this kind of "range" request in ath10k, 

Well it doesn't have the form to use a range either so it wouldn't make sense.

> the
> current code works just fine only if we can get rid of the annoying
> warning from request_firmware(). Unless if it's significantly faster or
> something.

Thanks, I see ath10k uses the sync request_firmware() call, so indeed it
would be a trivial conversion.

Andres can you roll that in for your patch series?

  Luis

Re: mt7601u: let mac80211 validate rx CCMP PN

[Kalle Valo]

Lorenzo Bianconi  wrote:

> Apparently the hardware does not perform CCMP PN validation so
> let mac80211 take care of possible replay attacks in sw.
> Moreover indicate ICV and MIC had been stripped setting corresponding
> bits in ieee80211_rx_status.
> The fix has been validated using 4.2.1 and 4.1.3 tests from the WiFi
> Alliance vulnerability detection tool.
> 
> Fixes: c869f77d6abb ("add mt7601u driver")
> Acked-by: Jakub Kicinski 
> Tested-by: David Park 
> Signed-off-by: Lorenzo Bianconi 

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

a9eab62d4164 mt7601u: let mac80211 validate rx CCMP PN

-- 
https://patchwork.kernel.org/patch/10270239/

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

Re: mt7601u: simplify mt7601u_mcu_msg_alloc signature

[Kalle Valo]

Lorenzo Bianconi  wrote:

> Remove mt7601u_dev parameter from mt7601u_mcu_msg_alloc signature since
> dev pointer is never used in routine body
> 
> Signed-off-by: Lorenzo Bianconi 
> Acked-by: Jakub Kicinski 

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

2f04652f891a mt7601u: simplify mt7601u_mcu_msg_alloc signature

-- 
https://patchwork.kernel.org/patch/10263687/

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

Re: mt76x2: remove unnecessary len variable in mt76x2_eeprom_load()

[Kalle Valo]

Lorenzo Bianconi  wrote:

> Substitute unnecessary len variable in mt76x2_eeprom_load() with
> MT7662_EEPROM_SIZE macro since len is used just to store eeprom
> default size.
> 
> Signed-off-by: Lorenzo Bianconi 

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

fbae9c7490b7 mt76x2: remove unnecessary len variable in mt76x2_eeprom_load()

-- 
https://patchwork.kernel.org/patch/10249983/

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

Re: mt7601u: remove a warning in mt7601u_efuse_physical_size_check()

[Kalle Valo]

Lorenzo Bianconi  wrote:

> Fix the following sparse warning in mt7601u_efuse_physical_size_check:
> - drivers/net/wireless/mediatek/mt7601u/eeprom.c:77:27: warning:
>   Variable length array is used
> 
> Signed-off-by: Lorenzo Bianconi 
> Acked-by: Jakub Kicinski 

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

3fb2f6a4db98 mt7601u: remove a warning in mt7601u_efuse_physical_size_check()

-- 
https://patchwork.kernel.org/patch/10247613/

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

Re: [1/3] mt76: initialize available_antennas_{tx,rx} info

[Kalle Valo]

Lorenzo Bianconi  wrote:

> Initialize available_antennas related info in wiphy data structure
> according to antenna_mask field; antenna_mask info is initialized
> in device specific code and will be used in mac80211 {set,get}_antenna
> callbacks
> 
> Signed-off-by: Lorenzo Bianconi 

3 patches applied to wireless-drivers-next.git, thanks.

24114a5f9470 mt76: initialize available_antennas_{tx,rx} info
551e1ef4d291 mt76: add mt76_init_stream_cap routine
5ebdc3e0692f mt76x2: add mac80211 {set,get}_antenna callbacks

-- 
https://patchwork.kernel.org/patch/10240417/

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

Re: [PATCH 10/10] rsi: drop RX broadcast/multicast packets with invalid PN

[Kalle Valo]

Amitkumar Karwar  writes:

> From: Siva Rebbagondla 
>
> This patch adds a check to drop received broadcast/multicast frames if
> PN is invalid (i.e. not greater than last PN). bc_mc_pn
> variable added for each interface
>
> Signed-off-by: Siva Rebbagondla 
> Signed-off-by: Amitkumar Karwar 

[...]

> +static int rsi_validate_pn(struct rsi_hw *adapter, struct ieee80211_hdr *hdr)
> +{
> + struct ieee80211_vif *vif;
> + struct ieee80211_bss_conf *bss;
> + struct vif_priv *vif_info = NULL;
> + u8 cur_pn[IEEE80211_CCMP_PN_LEN];
> + u8 *last_pn;
> + int i, hdrlen;
> +
> + if (!is_broadcast_ether_addr(hdr->addr1) &&
> + !is_multicast_ether_addr(hdr->addr1))
> + return 1;
> +
> + hdrlen = ieee80211_hdrlen(hdr->frame_control);
> + for (i = 0; i < adapter->sc_nvifs; i++) {
> + vif = adapter->vifs[i];
> +
> + if (!vif)
> + continue;
> + if (vif->type != NL80211_IFTYPE_STATION &&
> + vif->type != NL80211_IFTYPE_P2P_CLIENT)
> + continue;
> + bss = >bss_conf;
> + if (!bss->assoc)
> + continue;
> + if (!ether_addr_equal(bss->bssid, hdr->addr2))
> + continue;
> + vif_info = (struct vif_priv *)vif->drv_priv;
> + if (!vif_info->key) {
> + vif_info = NULL;
> + continue;
> + }
> + if (!vif_info->rx_pn_valid) {
> + vif_info = NULL;
> + continue;
> + }
> + }
> + if (!vif_info)
> + return 1;

Why +1 here?

> + last_pn = vif_info->rx_bcmc_pn;
> + if (vif_info->key->cipher == WLAN_CIPHER_SUITE_CCMP) {
> + struct dot11_ccmp_hdr *ccmp =
> + (struct dot11_ccmp_hdr *)&((u8 *)hdr)[hdrlen];
> +
> + cur_pn[0] = ccmp->pn0;
> + cur_pn[1] = ccmp->pn1;
> + cur_pn[2] = ccmp->pn2;
> + cur_pn[3] = ccmp->pn3;
> + cur_pn[4] = ccmp->pn4;
> + cur_pn[5] = ccmp->pn5;
> + } else {
> + struct dot11_tkip_hdr *tkip =
> + (struct dot11_tkip_hdr *)&((u8 *)hdr)[hdrlen];
> +
> + cur_pn[0] = tkip->tsc0;
> + cur_pn[1] = tkip->tsc1;
> + cur_pn[2] = tkip->tsc2;
> + cur_pn[3] = tkip->tsc3;
> + cur_pn[4] = tkip->tsc4;
> + cur_pn[5] = tkip->tsc5;
> + }
> + for (i = (IEEE80211_CCMP_PN_LEN - 1); i >= 0; i--)
> + if (last_pn[i] ^ cur_pn[i])
> + break;
> + if (i < 0)
> + return -1;

And why -1 here? Please use real error codes (-EINVAL etc).

> @@ -1341,14 +1488,14 @@ static void rsi_fill_rx_status(struct ieee80211_hw 
> *hw,
>   }
>   }
>   if (!bss)
> - return;
> + return -1;

Here as well.

-- 
Kalle Valo

Re: [PATCH 06/10] rsi: add module parameter rsi_reg

[Kalle Valo]

Amitkumar Karwar  writes:

> From: Prameela Rani Garnepudi 
>
> For few regulatory customizations, we are taking rsi_reg
> as module parameter.

Why?

> +static u16 rsi_reg = RSI_REG_DEF;
> +module_param(rsi_reg, ushort, 0444);
> +MODULE_PARM_DESC(rsi_reg, "0 - RSI_REG_DEF, 1 - RSI_REG_DLCAR");

The documentation tells nothing.

-- 
Kalle Valo

Re: [PATCH 05/10] rsi: roaming enhancements

[Kalle Valo]

Amitkumar Karwar  writes:

> From: Prameela Rani Garnepudi 
>
> To support roaming below changes are done:
> * Station notify frame is send to firmware after sending assoc
>   request. This will avoid dropping of first EAPOL frame due to
>   delay in creation of station control block in firmware.
> * Data queues are unblocked after sending station notify in open
>   mode, after configuring key in WEP mode, and after receiving
>   EAPOL4 confirm in WPA mode.
> * Initial EAPOL frames priority is chaged to MGMT, rekey EAPOL
>   frames priority changed to VO.
> * Data frames with wrong BSSID are dropped.

I don't know what Johannes thinks but IMHO the driver should not drop
any data frames, it should just submit what mac80211 gives to it. All
filtering should happen in mac80211 (or controlled by it).

-- 
Kalle Valo

Re: [PATCH 01/10] rsi: add support for hardware scan offload

[Kalle Valo]

Kalle Valo  writes:

> Amitkumar Karwar  writes:
>
>> From: Prameela Rani Garnepudi 
>>
>> With the current approach of scanning, roaming delays
>> are observed. Firmware has support for back ground scanning.
>> To get this advantage, mac80211 hardware scan is implemented.
>> In this method, foreground scan is performed in driver and
>> back ground scan is configured to firmware.
>
> To me doesn't like a good idea to duplicate scan functionality in the
> driver.

Also a pro tip: Don't place controversial patches as the first patch in
a big patchset, instead put them last so that I can apply rest of
patches anyway. Even better to submit them separately as RFC.

-- 
Kalle Valo

Re: [PATCH 01/10] rsi: add support for hardware scan offload

[Kalle Valo]

Amitkumar Karwar  writes:

> From: Prameela Rani Garnepudi 
>
> With the current approach of scanning, roaming delays
> are observed. Firmware has support for back ground scanning.
> To get this advantage, mac80211 hardware scan is implemented.
> In this method, foreground scan is performed in driver and
> back ground scan is configured to firmware.

To me doesn't like a good idea to duplicate scan functionality in the
driver.

> --- a/drivers/net/wireless/rsi/rsi_91x_main.c
> +++ b/drivers/net/wireless/rsi/rsi_91x_main.c
> @@ -324,6 +324,14 @@ struct rsi_hw *rsi_91x_init(u16 oper_mode)
>   mutex_init(>rx_lock);
>   mutex_init(>tx_bus_mutex);
>  
> + rsi_init_event(>chan_set_event);
> + rsi_init_event(>probe_cfm_event);
> + rsi_init_event(>chan_change_event);
> + rsi_init_event(>cancel_hw_scan_event);

And I'm starting to dislike this rsi_init_event() even more (see my
other mail). In upstream driver's custom abstractions are very much
frowned upon, especially that it makes review harder.

-- 
Kalle Valo

Re: [PATCH 1/3] rsi: improve RX handling in SDIO interface

[Kalle Valo]

Amitkumar Karwar  writes:

> From: Prameela Rani Garnepudi 
>
> Currently, RX packets are handled in interrupt context in SDIO
> interface. To improve the efficiency of processing RX packets,
> RX thread and RX skb queues are introduced.
> When the packet is read from device, driver prepares skb, add to
> RX queue and trigger RX thread event. RX thread processes the
> packets from RX queue.
>
> Signed-off-by: Prameela Rani Garnepudi 
> Signed-off-by: Amitkumar Karwar 

[...]

> +void rsi_sdio_rx_thread(struct rsi_common *common)
> +{
> + struct rsi_hw *adapter = common->priv;
> + struct rsi_91x_sdiodev *sdev = adapter->rsi_dev;
> + struct sk_buff *skb;
> + int status;
> +
> + do {
> + rsi_wait_event(>rx_thread.event, EVENT_WAIT_FOREVER);
> + rsi_reset_event(>rx_thread.event);

I started to wonder what these rsi_wait_event() and rsi_reset_event()
are and indeed, at least to me, they look suspicious (creating own
kthread and use of atomics etc). Why reinvent the wheel and not use
standard kernel frameworks, like workqueue? Most of the time trying to
be too clever ends up just being buggy.

No need for any immeadiate action but something to keep in mind and
hopefully clean up later.

-- 
Kalle Valo

Re: [PATCH 1/2] cfg80211: Add support to enable or disable btcoex

[Kalle Valo]

Arend van Spriel  writes:

> + Marcel
>
> On 3/2/2018 6:14 AM, Kalle Valo wrote:
>> Arend van Spriel  writes:
>>
>>> On 3/1/2018 6:59 PM, Tamizh chelvam wrote:
 From: Tamizh chelvam 

 This patch introduces NL80211_CMD_SET_BTCOEX command and
 NL80211_ATTR_BTCOEX_OP attribute to enable or disable btcoex.
>>>
>>> What kind of btcoex are we talking about here? Is it signalling
>>> between wlan and bt to get access to the shared RF.
>>
>> Yes, at least that's how I understand this.
>>
>>> Why would it require user-space interaction? Are there no options for
>>> wlan to detect bt is in use, ie. bt hci is setup, and vice versa. Can
>>> it be indicated in platform data or device tree. Trying to understand
>>> the use-case here.
>>
>> One use case is being able to disable btcoex in case of problems or to
>> test if it's btcoex related. I think during the last five years the need
>> for this interface has come every once in a while.
>
> Well, you would want to disable btcoex *and* bt to verify wlan is
> working properly on its own. And similarly disable btcoex *and* wlan
> to verify bt works properly.

Sure. But my main motiviation with this is to replace the module
parameters we already have:

drivers/net/wireless/ath/ath9k/htc_drv_init.c:module_param_named(btcoex_enable, 
ath9k_htc_btcoex_enable, int, 0444);
drivers/net/wireless/ath/ath9k/htc_drv_init.c:MODULE_PARM_DESC(btcoex_enable, 
"Enable wifi-BT coexistence");

drivers/net/wireless/ath/ath9k/init.c:module_param_named(btcoex_enable, 
ath9k_btcoex_enable, int, 0444);
drivers/net/wireless/ath/ath9k/init.c:MODULE_PARM_DESC(btcoex_enable, "Enable 
wifi-BT coexistence");

drivers/net/wireless/broadcom/b43/main.c:module_param_named(btcoex, 
modparam_btcoex, int, 0444);
drivers/net/wireless/broadcom/b43/main.c:MODULE_PARM_DESC(btcoex, "Enable 
Bluetooth coexistence (default on)");

But looking closely in ath9k cases it doesn't actually sound doable as
IIRC the ath9k module parameters are off by default. In b43 it's enabled
by default, though.

> Now I do recall a thread between you and Marcel. Looked it up and it
> was this thread [1], but did not see a follow-up on it. I suspect it
> involves more than just an enable/disable state. That may be fine for
> devices in which BT and WLAN are integrated and coordination of RF use
> is done on the device. The "btcoex subsystem" thread seems to aim for
> more like providing the coordination logic so independent BT device
> and WLAN device can still use the same RF. So before adopting the api
> in nl80211 it would be good to revive that thread.

I think that "btcoex subsystem" is a holy grail which will be never
implemented :) But who knows, miracles can happen...

-- 
Kalle Valo

Re: [PATCH] firmware: add a function to load optional firmware v2

[Kalle Valo]

"Luis R. Rodriguez"  writes:

> On Mon, Mar 12, 2018 at 12:10:47AM +0100, Arend van Spriel wrote:
>> On 3/11/2018 5:05 PM, Andres Rodriguez wrote:
>> > > Your patch series then should also have the driver callers who you
>> > > want to modify to use this new API. Collect from the 802.11 folks the
>> > > other drivers which I think they wanted changed as well.
>> > 
>> > Arend, Kalle, would love to hear your feedback.
>> 
>> I am not sure if it was ath10k, but Kalle will surely know. The other driver
>> firing a whole batch of firmware requests is iwlwifi. These basically try to
>> get latest firmware version and if not there try an older one.
>
> Ah I recall now. At least for iwlwifi its that it requests firmware with a
> range of api files, and we don't need information about files in the middle
> not found, given all we need to know if is if at lest one file was found
> or not.
>
> I have future code to also enable use of a range request which would replace
> the recursive nature of iwlwifi's firmware API request, so it simplifies it
> considerably.
>
> Once we get this flag to be silent in, this can be used later. Ie, the new
> API I'd add would replace the complex api revision thing for an internal set.

TBH I doubt we would use this kind of "range" request in ath10k, the
current code works just fine only if we can get rid of the annoying
warning from request_firmware(). Unless if it's significantly faster or
something.

-- 
Kalle Valo

Re: [PATCH] firmware: add a function to load optional firmware v2

[Kalle Valo]

Arend van Spriel  writes:

> On 3/11/2018 5:05 PM, Andres Rodriguez wrote:
>>> Your patch series then should also have the driver callers who you
>>> want to modify to use this new API. Collect from the 802.11 folks the
>>> other drivers which I think they wanted changed as well.
>>
>> Arend, Kalle, would love to hear your feedback.
>
> I am not sure if it was ath10k, but Kalle will surely know. The other
> driver firing a whole batch of firmware requests is iwlwifi. These
> basically try to get latest firmware version and if not there try an
> older one.

Oh yeah, ath10k definitely needs this! It tries different firmware API
versions from latest to older (firmware-6.bin, firmware-5.bin,
firmware-4.bin and so on) to find a compatible firmware and the error
messages from request_firmware() are constantly confusing the users, I
think the latest query about these errors from last week on IRC. So
having request_firmware_nowarn() (or similar) would help users a lot.

We tried to workaround this by using request_firmware_direct() (which
oddly doesn't print anything) but that caused issues with OpenWRT/LEDE:

https://git.kernel.org/linus/c0cc00f250e1

And iwlwifi has a similar problem, adding Luca to the loop.

-- 
Kalle Valo

Re: [PATCH] firmware: add a function to load optional firmware v2

[Kalle Valo]

"Luis R. Rodriguez"  writes:

>> +/**
>> + * request_firmware_optional: - request for an optional fw module
>> + * @firmware_p: pointer to firmware image
>> + * @name: name of firmware file
>> + * @device: device for which firmware is being loaded
>> + *
>> + * This function is similar in behaviour to request_firmware(), except
>> + * it doesn't produce warning messages when the file is not found.
>> + **/
>> +int
>> +request_firmware_optional(const struct firmware **firmware_p, const char 
>> *name,
>> + struct device *device)
>> +{
>> +   int ret;
>> +
>> +   /* Need to pin this module until return */
>> +   __module_get(THIS_MODULE);
>> +   ret = _request_firmware(firmware_p, name, device, NULL, 0,
>> +   FW_OPT_UEVENT | FW_OPT_NO_WARN );
>> +   module_put(THIS_MODULE);
>> +   return ret;
>> +}
>> +EXPORT_SYMBOL(request_firmware_optional);
>
> New exported symbols for the firmware API should be EXPORT_SYMBOL_GPL().

To me the word optional feels weird to me. For example, in ath10k I
suspect we would be only calling request_firmware_optional() with all
firmware and not request_firmware() at all.

How about request_firmware_nowarn()? That would even match the
documentation above.

-- 
Kalle Valo

Re: [2/3] mwifiex: support sysfs initiated device coredump

[Kalle Valo]

Arend van Spriel  writes:

> On 3/12/2018 10:41 AM, Kalle Valo wrote:
>> Arend Van Spriel  wrote:
>>
>>> Since commit 3c47d19ff4dc ("drivers: base: add coredump driver ops")
>>> it is possible to initiate a device coredump from user-space. This
>>> patch adds support for it adding the .coredump() driver callback.
>>> As there is no longer a need to initiate it through debugfs remove
>>> that code.
>>>
>>> Signed-off-by: Arend van Spriel 
>>
>> Based on the discussion I assume this is ok to take to w-d-next. If that's 
>> not
>> the case, please let me know ASAP.
>
> It is up to the mwifiex maintainers to decide, I guess. The ABI
> documentation need to be revised and change the callback to void
> return type. I am not sure what the best approach is. 1) apply this
> and fix return type later, or 2) fix return type and resubmit this.
> What is your opinion?

I guess the callback change will go through Greg's tree? Then I suspect
it's easier that you submit the callback change to Greg first and wait
for it to trickle down to wireless-drivers-next (after the next merge
window) and then I can apply the driver patches. Otherwise there might
be a conflict between my and Greg's tree.

-- 
Kalle Valo

RE: [PATCH] drivers: net: wireless: ath: ath9: dfs: remove VLA usage

[David Laight]

From: Daniel Micay
> Sent: 10 March 2018 23:45
> 
> > Just wondering. Is this actually a VLA. FFT_NUM_SAMPLES was static const so
> > not really going to show a lot of variation. This array will always have the
> > same size on the stack.
> 
> The issue is that unlike in C++, a `static const` can't be used in a
> constant expression in C. It's unclear why C is defined that way but
> it's how it is and there isn't currently a GCC extension making more
> things into constant expressions like C++.

'static' and 'const' are both just qualifiers to a 'variable'
You can still take it's address.
The language allows you to cast away the 'const' and write to
the variable - the effect is probably 'implementation defined'.

It is probably required to be valid for 'static const' items
to be patchable.

David

[PATCH] staging: wilc1000: use kmemdup to replace kmalloc/memcpy

[hariprasath . elango]

From: HariPrasath Elango 

kmalloc followed by memcpy can be replaced by kmemdup.Also added the
related error handling part

Signed-off-by: HariPrasath Elango 
---
 drivers/staging/wilc1000/host_interface.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index 0fac8e1..4ae2da6 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -992,9 +992,13 @@ static s32 handle_connect(struct wilc_vif *vif,
 
if (memcmp("DIRECT-", pstrHostIFconnectAttr->ssid, 7)) {
info_element_size = hif_drv->usr_conn_req.ies_len;
-   info_element = kmalloc(info_element_size, GFP_KERNEL);
-   memcpy(info_element, hif_drv->usr_conn_req.ies,
-   info_element_size);
+   info_element = kmemdup(hif_drv->usr_conn_req.ies,
+  info_element_size,
+  GFP_KERNEL);
+   if (!info_element) {
+   result = -ENOMEM;
+   goto ERRORHANDLER;
+   }
}
 
wid_list[wid_cnt].id = (u16)WID_11I_MODE;
-- 
2.10.0.GIT

Re: [PATCHv2] staging: wilc1000: use pre-defined macro is_broadcast_ether_addr

[Ajay Singh]

On Tue, 13 Mar 2018 10:58:16 +0100
Greg Kroah-Hartman  wrote:

> On Tue, Mar 13, 2018 at 01:00:51PM +0530, Ajay Singh wrote:
> > 
> > Reviewed-by: Ajay Singh 
> > 
> > On Mon, 12 Mar 2018 15:09:03 +0530
> >  wrote:
> >   
> > > From: HariPrasath Elango 
> > >  
> > 
> > Please avoid use of 'From' tag specially when there is only one
> > 'Signed-off-by' tag and its same.  
> 
> But the email client isn't putting the correct info in the email header,
> so this line is needed.
> 
> And if it's redundant, not a problem, git handles it just fine, better
> to be safe.


Thanks Greg, for giving the information.
In that case, we can keep the 'From:' tag to be on safe side.


Regards,
Ajay

[PATCH] staging: wilc1000: remove unwanted braces and correct code alignment

[hariprasath . elango]

From: HariPrasath Elango 

Remove the unwated brace and corrected the code block alignment
accordingly

Signed-off-by: HariPrasath Elango 
---
 drivers/staging/wilc1000/host_interface.c | 25 -
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index a4ee175..0fac8e1 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -984,20 +984,19 @@ static s32 handle_connect(struct wilc_vif *vif,
wid_list[wid_cnt].val = (s8 *)(&(dummyval));
wid_cnt++;
 
-   {
-   wid_list[wid_cnt].id = WID_INFO_ELEMENT_ASSOCIATE;
-   wid_list[wid_cnt].type = WID_BIN_DATA;
-   wid_list[wid_cnt].val = hif_drv->usr_conn_req.ies;
-   wid_list[wid_cnt].size = hif_drv->usr_conn_req.ies_len;
-   wid_cnt++;
-
-   if (memcmp("DIRECT-", pstrHostIFconnectAttr->ssid, 7)) {
-   info_element_size = hif_drv->usr_conn_req.ies_len;
-   info_element = kmalloc(info_element_size, GFP_KERNEL);
-   memcpy(info_element, hif_drv->usr_conn_req.ies,
-  info_element_size);
-   }
+   wid_list[wid_cnt].id = WID_INFO_ELEMENT_ASSOCIATE;
+   wid_list[wid_cnt].type = WID_BIN_DATA;
+   wid_list[wid_cnt].val = hif_drv->usr_conn_req.ies;
+   wid_list[wid_cnt].size = hif_drv->usr_conn_req.ies_len;
+   wid_cnt++;
+
+   if (memcmp("DIRECT-", pstrHostIFconnectAttr->ssid, 7)) {
+   info_element_size = hif_drv->usr_conn_req.ies_len;
+   info_element = kmalloc(info_element_size, GFP_KERNEL);
+   memcpy(info_element, hif_drv->usr_conn_req.ies,
+   info_element_size);
}
+
wid_list[wid_cnt].id = (u16)WID_11I_MODE;
wid_list[wid_cnt].type = WID_CHAR;
wid_list[wid_cnt].size = sizeof(char);
-- 
2.10.0.GIT

[PATCH] staging: wilc1000: replace switch statement by simple if condition

[hariprasath . elango]

From: HariPrasath Elango 

In this case,there is only a single switch case statement.So replacing
by a simple if condition.

Signed-off-by: HariPrasath Elango 
---
 drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c 
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index c901108..17bd762 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -772,14 +772,8 @@ static int connect(struct wiphy *wiphy, struct net_device 
*dev,
}
 
if (sme->crypto.n_akm_suites) {
-   switch (sme->crypto.akm_suites[0]) {
-   case WLAN_AKM_SUITE_8021X:
+   if (sme->crypto.akm_suites[0] == WLAN_AKM_SUITE_8021X)
auth_type = IEEE8021;
-   break;
-
-   default:
-   break;
-   }
}
 
curr_channel = pstrNetworkInfo->ch;
-- 
2.10.0.GIT

Re: [PATCHv2] staging: wilc1000: use pre-defined macro is_broadcast_ether_addr

[Greg Kroah-Hartman]

On Tue, Mar 13, 2018 at 01:00:51PM +0530, Ajay Singh wrote:
> 
> Reviewed-by: Ajay Singh 
> 
> On Mon, 12 Mar 2018 15:09:03 +0530
>  wrote:
> 
> > From: HariPrasath Elango 
> >
> 
> Please avoid use of 'From' tag specially when there is only one
> 'Signed-off-by' tag and its same.

But the email client isn't putting the correct info in the email header,
so this line is needed.

And if it's redundant, not a problem, git handles it just fine, better
to be safe.

thanks,

greg k-h

[PATCH] staging: wilc1000: destroy initialized mutex object

[hariprasath . elango]

From: HariPrasath Elango 

A mutex object that is initialized but not destroyed.This patch destroys
the mutex object

Signed-off-by: HariPrasath Elango 
---
 drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c 
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index 205304c..c901108 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -2303,6 +2303,7 @@ int wilc_deinit_host_int(struct net_device *net)
 
op_ifcs--;
 
+   mutex_destroy(>scan_req_lock);
ret = wilc_deinit(vif);
 
clear_shadow_scan();
-- 
2.10.0.GIT

Re: Problem with bridge (mcast-to-ucast + hairpin) and Broadcom's 802.11f in their FullMAC fw

[Arend van Spriel]

On 3/13/2018 8:20 AM, Felix Fietkau wrote:

[resent with fixed typo in linux-wireless address]

On 2018-02-27 11:08, Rafał Miłecki wrote:

I've problem when using OpenWrt/LEDE on a home router with Broadcom's
FullMAC WiFi chipset.


First of all OpenWrt/LEDE uses bridge interface for LAN network with:
1) IFLA_BRPORT_MCAST_TO_UCAST
2) Clients isolation in hostapd
3) Hairpin mode enabled

For more details please see Linus's patch description:
https://patchwork.kernel.org/patch/9530669/
and maybe hairpin mode patch:
https://lwn.net/Articles/347344/

Short version: in that setup packets received from a bridged wireless
interface can be handled back to it for transmission.


Now, Broadcom's firmware for their FullMAC chipsets in AP mode
supports an obsoleted 802.11f AKA IAPP standard. It's a roaming
standard that was replaced by 802.11r.

Whenever a new station associates, firmware generates a packet like:
ff ff ff ff  ff ff ec 10  7b 5f ?? ??  00 06 00 01  af 81 01 00
(just masked 2 bytes of my MAC)

For mode details you can see discussion in my brcmfmac patch thread:
https://patchwork.kernel.org/patch/10191451/


The problem is that bridge (in setup as above) handles such a packet
back to the device.

That makes Broadcom's FullMAC firmware believe that a given station
just connected to another AP in a network (which doesn't even exist).
As a result firmware immediately disassociates that station. It's
simply impossible to connect to the router. Every association is
followed by immediate disassociation.


Can you see any solution for this problem? Is that an option to stop
multicast-to-unicast from touching 802.11f packets? Some other ideas?
Obviously I can't modify Broadcom's firmware and drop that obsoleted
standard.

Let's look at it from a different angle: Since these packets are
forwarded as normal packets by the bridge, and the Broadcom firmware
reacts to them in this nasty way, that's basically local DoS security
issue. In my opinion that matters a lot more than having support for an
obsolete feature that almost nobody will ever want to use.

I think the right approach to deal with this issue is to drop these
garbage packets in both the receive and transmit path of brcmfmac.


My approach was to get rid of it in firmware as this never made it into 
the 802.11 spec. So I asked internally whether it was still used. Turns 
out that we still rely on it for some customers. I am fine with dropping 
these "garbage" packets, but given that there is still use for it I 
would like to see that under a Kconfig flag. Dropping it may be the default.


Regards,
Arend

Re: [PATCH] staging: wilc1000: use kmemdup instead of kmalloc and memcpy

[Ajay Singh]

On Tue, 13 Mar 2018 11:50:48 +0530
 wrote:

> From: HariPrasath Elango 
> 
> Kmalloc followed by memcpy can be replaced by kmemdup.
> 
> Signed-off-by: HariPrasath Elango 

Reviewed-by: Ajay Singh 


Regards,
Ajay

Re: [PATCH] staging: wilc1000: Destroy mutex object in deinitialization

[Ajay Singh]

On Mon, 12 Mar 2018 18:49:49 +0530
 wrote:

> From: HariPrasath Elango 
> 
> Destroy the mutex object that is initialized in wlan_init_locks()
> 
> Signed-off-by: HariPrasath Elango 

Reviewed-by: Ajay Singh 


Regards,
Ajay

Re: [PATCH] staging: wilc1000: Fix code block alignment

[Ajay Singh]

On Mon, 12 Mar 2018 18:30:44 +0530
 wrote:

> From: HariPrasath Elango 
> 
> Fix the code alignment for a block of code to adhere to coding
> guidelines
> 
> Signed-off-by: HariPrasath Elango 

Reviewed-by: Ajay Singh 

Regards,
Ajay

Re: [PATCHv2] staging: wilc1000: use pre-defined macro is_broadcast_ether_addr

[Ajay Singh]

Reviewed-by: Ajay Singh 

On Mon, 12 Mar 2018 15:09:03 +0530
 wrote:

> From: HariPrasath Elango 
>

Please avoid use of 'From' tag specially when there is only one
'Signed-off-by' tag and its same.

> Use the kernel pre-defined macro is_broadcast_ether_addr() instead of
> doing a memcmp here.
> 
> Signed-off-by: HariPrasath Elango 
> ---

Regards,
Ajay

Re: Problem with bridge (mcast-to-ucast + hairpin) and Broadcom's 802.11f in their FullMAC fw

[Felix Fietkau]

[resent with fixed typo in linux-wireless address]

On 2018-02-27 11:08, Rafał Miłecki wrote:
> I've problem when using OpenWrt/LEDE on a home router with Broadcom's
> FullMAC WiFi chipset.
> 
> 
> First of all OpenWrt/LEDE uses bridge interface for LAN network with:
> 1) IFLA_BRPORT_MCAST_TO_UCAST
> 2) Clients isolation in hostapd
> 3) Hairpin mode enabled
> 
> For more details please see Linus's patch description:
> https://patchwork.kernel.org/patch/9530669/
> and maybe hairpin mode patch:
> https://lwn.net/Articles/347344/
> 
> Short version: in that setup packets received from a bridged wireless
> interface can be handled back to it for transmission.
> 
> 
> Now, Broadcom's firmware for their FullMAC chipsets in AP mode
> supports an obsoleted 802.11f AKA IAPP standard. It's a roaming
> standard that was replaced by 802.11r.
> 
> Whenever a new station associates, firmware generates a packet like:
> ff ff ff ff  ff ff ec 10  7b 5f ?? ??  00 06 00 01  af 81 01 00
> (just masked 2 bytes of my MAC)
> 
> For mode details you can see discussion in my brcmfmac patch thread:
> https://patchwork.kernel.org/patch/10191451/
> 
> 
> The problem is that bridge (in setup as above) handles such a packet
> back to the device.
> 
> That makes Broadcom's FullMAC firmware believe that a given station
> just connected to another AP in a network (which doesn't even exist).
> As a result firmware immediately disassociates that station. It's
> simply impossible to connect to the router. Every association is
> followed by immediate disassociation.
> 
> 
> Can you see any solution for this problem? Is that an option to stop
> multicast-to-unicast from touching 802.11f packets? Some other ideas?
> Obviously I can't modify Broadcom's firmware and drop that obsoleted
> standard.
Let's look at it from a different angle: Since these packets are
forwarded as normal packets by the bridge, and the Broadcom firmware
reacts to them in this nasty way, that's basically local DoS security
issue. In my opinion that matters a lot more than having support for an
obsolete feature that almost nobody will ever want to use.

I think the right approach to deal with this issue is to drop these
garbage packets in both the receive and transmit path of brcmfmac.

- Felix

Re: Problem with bridge (mcast-to-ucast + hairpin) and Broadcom's 802.11f in their FullMAC fw

[Rafał Miłecki]

On 03/13/2018 12:01 AM, Stephen Hemminger wrote:

On Mon, 12 Mar 2018 23:42:48 +0100
Rafał Miłecki  wrote:


2) Blame bridge + mcast-to-ucast + hairpin for 802.11f incompatibility

If we agree that 802.11f support in FullMAC firmware is acceptable, then
we have to make sure Linux's bridge doesn't break it by passing 802.11f
(broadcast) frames back to the source interface. That would require a
check like in below diff + proper code for handling such packets. I'm
afraid I'm not familiar with bridge code enough to complete that.

diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index edae702..9e5d6ea 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -126,6 +126,27 @@ static void br_do_proxy_arp(struct sk_buff *skb, struct 
net_bridge *br,
}
  }

+static bool br_skb_is_iapp_add_packet(struct sk_buff *skb)
+{
+   const u8 iapp_add_packet[6] __aligned(2) = {
+   0x00, 0x01, 0xaf, 0x81, 0x01, 0x00,
+   };
+#if !defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
+   const u16 *a = (const u16 *)skb->data;
+   const u16 *b = (const u16 *)iapp_add_packet;
+#endif
+
+   if (skb->len != 6)
+   return false;
+
+#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
+   return !(((*(const u32 *)skb->data) ^ (*(const u32 *)iapp_add_packet)) |
+((*(const u16 *)(skb->data + 4)) ^ (*(const u16 
*)(iapp_add_packet + 4;
+#else
+   return !((a[0] ^ b[0]) | (a[1] ^ b[1]) | (a[2] ^ b[2]));
+#endif
+}
+
  /* note: already called with rcu_read_lock */
  int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff 
*skb)
  {
@@ -155,6 +176,8 @@ int br_handle_frame_finish(struct net *net, struct sock 
*sk, struct sk_buff *skb
if (is_multicast_ether_addr(dest)) {
/* by definition the broadcast is also a multicast address */
if (is_broadcast_ether_addr(dest)) {
+   if (br_skb_is_iapp_add_packet(skb))
+   pr_warn("This packet should not be passed back to 
the source interface!\n");
pkt_type = BR_PKT_BROADCAST;
local_rcv = true;
} else {



Don't like bridge doing special case code for magic received values directly in 
input path.
Really needs to be generic which is why I suggested ebtables.


We need in-bridge solution only if we decide to support FullMAC
firmwares with 802.11f implementation.

In that case is this possible to use ebtables as a workaround at all?
Can I really use ebtables to set switch to don't pass 802.11f ADD frames
back to the original interface?

[PATCH] staging: wilc1000: use kmemdup instead of kmalloc and memcpy

[hariprasath . elango]

From: HariPrasath Elango 

Kmalloc followed by memcpy can be replaced by kmemdup.

Signed-off-by: HariPrasath Elango 
---
 drivers/staging/wilc1000/linux_mon.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/wilc1000/linux_mon.c 
b/drivers/staging/wilc1000/linux_mon.c
index f93f411..c6fd6b3 100644
--- a/drivers/staging/wilc1000/linux_mon.c
+++ b/drivers/staging/wilc1000/linux_mon.c
@@ -146,7 +146,7 @@ static int mon_mgmt_tx(struct net_device *dev, const u8 
*buf, size_t len)
if (!mgmt_tx)
return -ENOMEM;
 
-   mgmt_tx->buff = kmalloc(len, GFP_ATOMIC);
+   mgmt_tx->buff = kmemdup(buf, len, GFP_ATOMIC);
if (!mgmt_tx->buff) {
kfree(mgmt_tx);
return -ENOMEM;
@@ -154,7 +154,6 @@ static int mon_mgmt_tx(struct net_device *dev, const u8 
*buf, size_t len)
 
mgmt_tx->size = len;
 
-   memcpy(mgmt_tx->buff, buf, len);
wilc_wlan_txq_add_mgmt_pkt(dev, mgmt_tx, mgmt_tx->buff, mgmt_tx->size,
   mgmt_tx_complete);
 
-- 
2.10.0.GIT