nl80211/mac80211: allow non-linear skb in rx_control_port

The current implementation of cfg80211_rx_control_port assumed that the
caller could provide a contiguous region of memory for the control port
frame to be sent up to userspace.  Unfortunately, many drivers produce
non-linear skbs, especially for data frames.  This resulted in userspace
getting notified of control port frames with correct metadata (from
address, port, etc) yet garbage / nonsense contents, resulting in bad
handshakes, disconnections, etc.

mac80211 linearizes skbs containing management frames.  But it didn't
seem worthwhile to do this for control port frames.  Thus the signature
of cfg80211_rx_control_port was changed to take the skb directly.
nl80211 then takes care of obtaining control port frame data directly
from the (linear | non-linear) skb.

The caller is still responsible for freeing the skb,
cfg80211_rx_control_port does not take ownership of it.

Fixes: 6a671a50f819 ("nl80211: Add CMD_CONTROL_PORT_FRAME API")
Signed-off-by: Denis Kenzior <denkenz@gmail.com>
[fix some kernel-doc formatting, add fixes tag]
Signed-off-by: Johannes Berg <johannes.berg@intel.com>

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 5fbfe61f41c67f19713bf0e307ae0612428d68a6..1beb3ead038561d84c618757919871393f8c80c4 100644 (file)
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -5835,10 +5835,11 @@ 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
+ * @skb: The skbuf with the control port frame.  It is assumed that the skbuf
+ *     is 802.3 formatted (with 802.3 header).  The skb can be non-linear.
+ *     This function does not take ownership of the skb, so the caller is
+ *     responsible for any cleanup.  The caller must also ensure that
+ *     skb->protocol is set appropriately.
  * @unencrypted: Whether the frame was received unencrypted
  *
  * This function is used to inform userspace about a received control port
@@ -5851,8 +5852,7 @@ void cfg80211_mgmt_tx_status(struct wireless_dev *wdev, u64 cookie,
  * 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);
+                             struct sk_buff *skb, bool unencrypted);
 
 /**
  * cfg80211_cqm_rssi_notify - connection quality monitoring rssi event

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 0a38cc1cbebcee97ed7e8779ab487e2e0943e84c..932985ca4e66829ffa559fac1a10243e93043101 100644 (file)
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -2254,11 +2254,8 @@ static void ieee80211_deliver_skb_to_local_stack(struct sk_buff *skb,
                     sdata->control_port_over_nl80211)) {
                struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
                bool noencrypt = status->flag & RX_FLAG_DECRYPTED;
-               struct ethhdr *ehdr = eth_hdr(skb);
 
-               cfg80211_rx_control_port(dev, skb->data, skb->len,
-                                        ehdr->h_source,
-                                        be16_to_cpu(skb->protocol), noencrypt);
+               cfg80211_rx_control_port(dev, skb, noencrypt);
                dev_kfree_skb(skb);
        } else {
                /* deliver to local stack */

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 4eece06be1e734c0a0c8de50cdeff2475dd20a59..b6c700572755143cc5775a191dc1520e625fed9e 100644 (file)
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -14923,20 +14923,24 @@ 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,
+                                    struct sk_buff *skb,
                                     bool unencrypted, gfp_t gfp)
 {
        struct wireless_dev *wdev = dev->ieee80211_ptr;
        struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
+       struct ethhdr *ehdr = eth_hdr(skb);
+       const u8 *addr = ehdr->h_source;
+       u16 proto = be16_to_cpu(skb->protocol);
        struct sk_buff *msg;
        void *hdr;
+       struct nlattr *frame;
+
        u32 nlportid = READ_ONCE(wdev->conn_owner_nlportid);
 
        if (!nlportid)
                return -ENOENT;
 
-       msg = nlmsg_new(100 + len, gfp);
+       msg = nlmsg_new(100 + skb->len, gfp);
        if (!msg)
                return -ENOMEM;
 
@@ -14950,13 +14954,17 @@ static int __nl80211_rx_control_port(struct net_device *dev,
            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, ETH_ALEN, addr) ||
            nla_put_u16(msg, NL80211_ATTR_CONTROL_PORT_ETHERTYPE, proto) ||
            (unencrypted && nla_put_flag(msg,
                                         NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT)))
                goto nla_put_failure;
 
+       frame = nla_reserve(msg, NL80211_ATTR_FRAME, skb->len);
+       if (!frame)
+               goto nla_put_failure;
+
+       skb_copy_bits(skb, 0, nla_data(frame), skb->len);
        genlmsg_end(msg, hdr);
 
        return genlmsg_unicast(wiphy_net(&rdev->wiphy), msg, nlportid);
@@ -14967,14 +14975,12 @@ static int __nl80211_rx_control_port(struct net_device *dev,
 }
 
 bool cfg80211_rx_control_port(struct net_device *dev,
-                             const u8 *buf, size_t len,
-                             const u8 *addr, u16 proto, bool unencrypted)
+                             struct sk_buff *skb, bool unencrypted)
 {
        int ret;
 
-       trace_cfg80211_rx_control_port(dev, buf, len, addr, proto, unencrypted);
-       ret = __nl80211_rx_control_port(dev, buf, len, addr, proto,
-                                       unencrypted, GFP_ATOMIC);
+       trace_cfg80211_rx_control_port(dev, skb, unencrypted);
+       ret = __nl80211_rx_control_port(dev, skb, unencrypted, GFP_ATOMIC);
        trace_cfg80211_return_bool(ret == 0);
        return ret == 0;
 }

diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index 2b417a2fe63ffb564b744e8e159c3bdbcf43a4ed..7c73510b161f3b84ce3d7d24dd3b9aa3b472823e 100644 (file)
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -2627,23 +2627,25 @@ TRACE_EVENT(cfg80211_mgmt_tx_status,
 );
 
 TRACE_EVENT(cfg80211_rx_control_port,
-       TP_PROTO(struct net_device *netdev, const u8 *buf, size_t len,
-                const u8 *addr, u16 proto, bool unencrypted),
-       TP_ARGS(netdev, buf, len, addr, proto, unencrypted),
+       TP_PROTO(struct net_device *netdev, struct sk_buff *skb,
+                bool unencrypted),
+       TP_ARGS(netdev, skb, unencrypted),
        TP_STRUCT__entry(
                NETDEV_ENTRY
-               MAC_ENTRY(addr)
+               __field(int, len)
+               MAC_ENTRY(from)
                __field(u16, proto)
                __field(bool, unencrypted)
        ),
        TP_fast_assign(
                NETDEV_ASSIGN;
-               MAC_ASSIGN(addr, addr);
-               __entry->proto = proto;
+               __entry->len = skb->len;
+               MAC_ASSIGN(from, eth_hdr(skb)->h_source);
+               __entry->proto = be16_to_cpu(skb->protocol);
                __entry->unencrypted = unencrypted;
        ),
-       TP_printk(NETDEV_PR_FMT ", " MAC_PR_FMT " proto: 0x%x, unencrypted: %s",
-                 NETDEV_PR_ARG, MAC_PR_ARG(addr),
+       TP_printk(NETDEV_PR_FMT ", len=%d, " MAC_PR_FMT ", proto: 0x%x, unencrypted: %s",
+                 NETDEV_PR_ARG, __entry->len, MAC_PR_ARG(from),
                  __entry->proto, BOOL_TO_STR(__entry->unencrypted))
 );