[PATCH 2/3] staging: Dynamically allocate struct station_info

[PATCH 2/3] staging: Dynamically allocate struct station_info

[Toke Høiland-Jørgensen]

Since the addition of the TXQ stats to cfg80211, the station_info struct
has grown to be quite large, which results in warnings when allocated on
the stack. Fix the affected places to do dynamic allocations instead.
WARN_ON is used where the function has no way to signal errors to the
caller.

This patch applies the fix to the rtl8723bs driver in staging while a
separate patch fixes the drivers in the main tree.

Fixes: 52539ca89f36 cfg80211: Expose TXQ stats and parameters to userspace
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
---
 drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c |   16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
index 46bc2e512557..c76c9a8066c4 100644
--- a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
+++ b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
@@ -2431,17 +2431,23 @@ void rtw_cfg80211_indicate_sta_assoc(struct adapter *padapter, u8 *pmgmt_frame,
 	DBG_871X(FUNC_ADPT_FMT"\n", FUNC_ADPT_ARG(padapter));
 
 	{
-		struct station_info sinfo;
+		struct station_info *sinfo;
 		u8 ie_offset;
 		if (GetFrameSubType(pmgmt_frame) == WIFI_ASSOCREQ)
 			ie_offset = _ASOCREQ_IE_OFFSET_;
 		else /*  WIFI_REASSOCREQ */
 			ie_offset = _REASOCREQ_IE_OFFSET_;
 
-		sinfo.filled = 0;
-		sinfo.assoc_req_ies = pmgmt_frame + WLAN_HDR_A3_LEN + ie_offset;
-		sinfo.assoc_req_ies_len = frame_len - WLAN_HDR_A3_LEN - ie_offset;
-		cfg80211_new_sta(ndev, GetAddr2Ptr(pmgmt_frame), &sinfo, GFP_ATOMIC);
+		sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
+		if (WARN_ON(!sinfo))
+			return;
+
+		sinfo->filled = 0;
+		sinfo->assoc_req_ies = pmgmt_frame + WLAN_HDR_A3_LEN + ie_offset;
+		sinfo->assoc_req_ies_len = frame_len - WLAN_HDR_A3_LEN - ie_offset;
+		cfg80211_new_sta(ndev, GetAddr2Ptr(pmgmt_frame), sinfo, GFP_ATOMIC);
+
+		kfree(sinfo);
 	}
 }
 

[PATCH 3/3] net: Dynamically allocate struct station_info

[Toke Høiland-Jørgensen]

Since the addition of the TXQ stats to cfg80211, the station_info struct
has grown to be quite large, which results in warnings when allocated on
the stack. Fix the affected places to do dynamic allocations instead.
WARN_ON is used where the function has no way to signal errors to the
caller.

This patch applies the fix to batman-adv and wext-compat, while a
separate patch fixes up the drivers.

Fixes: 52539ca89f36 cfg80211: Expose TXQ stats and parameters to userspace
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
---
 net/batman-adv/bat_v_elp.c |   21 +++++++++++++++------
 net/wireless/wext-compat.c |   29 +++++++++++++++++------------
 2 files changed, 32 insertions(+), 18 deletions(-)

diff --git a/net/batman-adv/bat_v_elp.c b/net/batman-adv/bat_v_elp.c
index 28687493599f..d2393ebc6af4 100644
--- a/net/batman-adv/bat_v_elp.c
+++ b/net/batman-adv/bat_v_elp.c
@@ -79,8 +79,9 @@ static u32 batadv_v_elp_get_throughput(struct batadv_hardif_neigh_node *neigh)
 	struct batadv_hard_iface *hard_iface = neigh->if_incoming;
 	struct ethtool_link_ksettings link_settings;
 	struct net_device *real_netdev;
-	struct station_info sinfo;
+	struct station_info *sinfo;
 	u32 throughput;
+	bool filled;
 	int ret;
 
 	/* if the user specified a customised value for this interface, then
@@ -102,7 +103,17 @@ static u32 batadv_v_elp_get_throughput(struct batadv_hardif_neigh_node *neigh)
 		if (!real_netdev)
 			goto default_throughput;
 
-		ret = cfg80211_get_station(real_netdev, neigh->addr, &sinfo);
+		sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
+		if (!sinfo)
+			goto default_throughput;
+
+		ret = cfg80211_get_station(real_netdev, neigh->addr, sinfo);
+
+		/* just save these here instead of having complex free logic below */
+		throughput = sinfo.expected_throughput / 100;
+		filled = !!(sinfo.filled & BIT(NL80211_STA_INFO_EXPECTED_THROUGHPUT));
+
+		kfree(sinfo);
 
 		dev_put(real_netdev);
 		if (ret == -ENOENT) {
@@ -112,12 +123,10 @@ static u32 batadv_v_elp_get_throughput(struct batadv_hardif_neigh_node *neigh)
 			 */
 			return 0;
 		}
-		if (ret)
-			goto default_throughput;
-		if (!(sinfo.filled & BIT(NL80211_STA_INFO_EXPECTED_THROUGHPUT)))
+		if (ret || !filled)
 			goto default_throughput;
 
-		return sinfo.expected_throughput / 100;
+		return throughput;
 	}
 
 	/* if not a wifi interface, check if this device provides data via
diff --git a/net/wireless/wext-compat.c b/net/wireless/wext-compat.c
index 9e002df0f8d8..2038e3fb25fa 100644
--- a/net/wireless/wext-compat.c
+++ b/net/wireless/wext-compat.c
@@ -1300,7 +1300,7 @@ static struct iw_statistics *cfg80211_wireless_stats(struct net_device *dev)
 	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
 	/* we are under RTNL - globally locked - so can use static structs */
 	static struct iw_statistics wstats;
-	static struct station_info sinfo;
+	static struct station_info *sinfo;
 	u8 bssid[ETH_ALEN];
 
 	if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_STATION)
@@ -1318,17 +1318,21 @@ static struct iw_statistics *cfg80211_wireless_stats(struct net_device *dev)
 	memcpy(bssid, wdev->current_bss->pub.bssid, ETH_ALEN);
 	wdev_unlock(wdev);
 
-	memset(&sinfo, 0, sizeof(sinfo));
+	sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
+	if (!sinfo)
+		return NULL;
 
-	if (rdev_get_station(rdev, dev, bssid, &sinfo))
+	if (rdev_get_station(rdev, dev, bssid, sinfo)) {
+		kfree(sinfo);
 		return NULL;
+	}
 
 	memset(&wstats, 0, sizeof(wstats));
 
 	switch (rdev->wiphy.signal_type) {
 	case CFG80211_SIGNAL_TYPE_MBM:
-		if (sinfo.filled & BIT(NL80211_STA_INFO_SIGNAL)) {
-			int sig = sinfo.signal;
+		if (sinfo->filled & BIT(NL80211_STA_INFO_SIGNAL)) {
+			int sig = sinfo->signal;
 			wstats.qual.updated |= IW_QUAL_LEVEL_UPDATED;
 			wstats.qual.updated |= IW_QUAL_QUAL_UPDATED;
 			wstats.qual.updated |= IW_QUAL_DBM;
@@ -1341,11 +1345,11 @@ static struct iw_statistics *cfg80211_wireless_stats(struct net_device *dev)
 			break;
 		}
 	case CFG80211_SIGNAL_TYPE_UNSPEC:
-		if (sinfo.filled & BIT(NL80211_STA_INFO_SIGNAL)) {
+		if (sinfo->filled & BIT(NL80211_STA_INFO_SIGNAL)) {
 			wstats.qual.updated |= IW_QUAL_LEVEL_UPDATED;
 			wstats.qual.updated |= IW_QUAL_QUAL_UPDATED;
-			wstats.qual.level = sinfo.signal;
-			wstats.qual.qual = sinfo.signal;
+			wstats.qual.level = sinfo->signal;
+			wstats.qual.qual = sinfo->signal;
 			break;
 		}
 	default:
@@ -1354,11 +1358,12 @@ static struct iw_statistics *cfg80211_wireless_stats(struct net_device *dev)
 	}
 
 	wstats.qual.updated |= IW_QUAL_NOISE_INVALID;
-	if (sinfo.filled & BIT(NL80211_STA_INFO_RX_DROP_MISC))
-		wstats.discard.misc = sinfo.rx_dropped_misc;
-	if (sinfo.filled & BIT(NL80211_STA_INFO_TX_FAILED))
-		wstats.discard.retries = sinfo.tx_failed;
+	if (sinfo->filled & BIT(NL80211_STA_INFO_RX_DROP_MISC))
+		wstats.discard.misc = sinfo->rx_dropped_misc;
+	if (sinfo->filled & BIT(NL80211_STA_INFO_TX_FAILED))
+		wstats.discard.retries = sinfo->tx_failed;
 
+	kfree(sinfo);
 	return &wstats;
 }
 

[PATCH 1/3] wireless-drivers: Dynamically allocate struct station_info

[Toke Høiland-Jørgensen]

Since the addition of the TXQ stats to cfg80211, the station_info struct
has grown to be quite large, which results in warnings when allocated on
the stack. Fix the affected places to do dynamic allocations instead.
WARN_ON is used where the function has no way to signal errors to the
caller.

Fixes: 52539ca89f36 cfg80211: Expose TXQ stats and parameters to userspace
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
---
 drivers/net/wireless/ath/ath6kl/main.c             |   14 ++++++----
 drivers/net/wireless/ath/wil6210/debugfs.c         |   22 ++++++++++-----
 drivers/net/wireless/ath/wil6210/wmi.c             |   19 ++++++++-----
 .../broadcom/brcm80211/brcmfmac/cfg80211.c         |   18 ++++++++----
 drivers/net/wireless/marvell/mwifiex/uap_event.c   |   25 +++++++++++------
 drivers/net/wireless/quantenna/qtnfmac/event.c     |   29 +++++++++++++-------
 6 files changed, 82 insertions(+), 45 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/main.c b/drivers/net/wireless/ath/ath6kl/main.c
index db95f85751e3..cf5e4ed5c8fc 100644
--- a/drivers/net/wireless/ath/ath6kl/main.c
+++ b/drivers/net/wireless/ath/ath6kl/main.c
@@ -426,7 +426,7 @@ void ath6kl_connect_ap_mode_sta(struct ath6kl_vif *vif, u16 aid, u8 *mac_addr,
 {
 	u8 *ies = NULL, *wpa_ie = NULL, *pos;
 	size_t ies_len = 0;
-	struct station_info sinfo;
+	struct station_info *sinfo;
 
 	ath6kl_dbg(ATH6KL_DBG_TRC, "new station %pM aid=%d\n", mac_addr, aid);
 
@@ -482,16 +482,20 @@ void ath6kl_connect_ap_mode_sta(struct ath6kl_vif *vif, u16 aid, u8 *mac_addr,
 			   keymgmt, ucipher, auth, apsd_info);
 
 	/* send event to application */
-	memset(&sinfo, 0, sizeof(sinfo));
+	sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
+	if (WARN_ON(!sinfo))
+		return;
 
 	/* TODO: sinfo.generation */
 
-	sinfo.assoc_req_ies = ies;
-	sinfo.assoc_req_ies_len = ies_len;
+	sinfo->assoc_req_ies = ies;
+	sinfo->assoc_req_ies_len = ies_len;
 
-	cfg80211_new_sta(vif->ndev, mac_addr, &sinfo, GFP_KERNEL);
+	cfg80211_new_sta(vif->ndev, mac_addr, sinfo, GFP_KERNEL);
 
 	netif_wake_queue(vif->ndev);
+
+	kfree(sinfo);
 }
 
 void disconnect_timer_handler(struct timer_list *t)
diff --git a/drivers/net/wireless/ath/wil6210/debugfs.c b/drivers/net/wireless/ath/wil6210/debugfs.c
index 8c90b3111f0b..11e46e44381e 100644
--- a/drivers/net/wireless/ath/wil6210/debugfs.c
+++ b/drivers/net/wireless/ath/wil6210/debugfs.c
@@ -1200,8 +1200,12 @@ static const struct file_operations fops_freq = {
 static int wil_link_debugfs_show(struct seq_file *s, void *data)
 {
 	struct wil6210_priv *wil = s->private;
-	struct station_info sinfo;
-	int i, rc;
+	struct station_info *sinfo;
+	int i, rc = 0;
+
+	sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
+	if (!sinfo)
+		return -ENOMEM;
 
 	for (i = 0; i < ARRAY_SIZE(wil->sta); i++) {
 		struct wil_sta_info *p = &wil->sta[i];
@@ -1229,19 +1233,21 @@ static int wil_link_debugfs_show(struct seq_file *s, void *data)
 
 		vif = (mid < wil->max_vifs) ? wil->vifs[mid] : NULL;
 		if (vif) {
-			rc = wil_cid_fill_sinfo(vif, i, &sinfo);
+			rc = wil_cid_fill_sinfo(vif, i, sinfo);
 			if (rc)
-				return rc;
+				goto out;
 
-			seq_printf(s, "  Tx_mcs = %d\n", sinfo.txrate.mcs);
-			seq_printf(s, "  Rx_mcs = %d\n", sinfo.rxrate.mcs);
-			seq_printf(s, "  SQ     = %d\n", sinfo.signal);
+			seq_printf(s, "  Tx_mcs = %d\n", sinfo->txrate.mcs);
+			seq_printf(s, "  Rx_mcs = %d\n", sinfo->rxrate.mcs);
+			seq_printf(s, "  SQ     = %d\n", sinfo->signal);
 		} else {
 			seq_puts(s, "  INVALID MID\n");
 		}
 	}
 
-	return 0;
+out:
+	kfree(sinfo);
+	return rc;
 }
 
 static int wil_link_seq_open(struct inode *inode, struct file *file)
diff --git a/drivers/net/wireless/ath/wil6210/wmi.c b/drivers/net/wireless/ath/wil6210/wmi.c
index a3dda9a97c1f..21124af06bdd 100644
--- a/drivers/net/wireless/ath/wil6210/wmi.c
+++ b/drivers/net/wireless/ath/wil6210/wmi.c
@@ -824,7 +824,7 @@ static void wmi_evt_connect(struct wil6210_vif *vif, int id, void *d, int len)
 	struct wireless_dev *wdev = vif_to_wdev(vif);
 	struct wmi_connect_event *evt = d;
 	int ch; /* channel number */
-	struct station_info sinfo;
+	struct station_info *sinfo;
 	u8 *assoc_req_ie, *assoc_resp_ie;
 	size_t assoc_req_ielen, assoc_resp_ielen;
 	/* capinfo(u16) + listen_interval(u16) + IEs */
@@ -940,6 +940,11 @@ static void wmi_evt_connect(struct wil6210_vif *vif, int id, void *d, int len)
 		vif->bss = NULL;
 	} else if ((wdev->iftype == NL80211_IFTYPE_AP) ||
 		   (wdev->iftype == NL80211_IFTYPE_P2P_GO)) {
+
+		sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
+		if (!sinfo)
+			rc = -ENOMEM;
+
 		if (rc) {
 			if (disable_ap_sme)
 				/* notify new_sta has failed */
@@ -947,16 +952,16 @@ static void wmi_evt_connect(struct wil6210_vif *vif, int id, void *d, int len)
 			goto out;
 		}
 
-		memset(&sinfo, 0, sizeof(sinfo));
-
-		sinfo.generation = wil->sinfo_gen++;
+		sinfo->generation = wil->sinfo_gen++;
 
 		if (assoc_req_ie) {
-			sinfo.assoc_req_ies = assoc_req_ie;
-			sinfo.assoc_req_ies_len = assoc_req_ielen;
+			sinfo->assoc_req_ies = assoc_req_ie;
+			sinfo->assoc_req_ies_len = assoc_req_ielen;
 		}
 
-		cfg80211_new_sta(ndev, evt->bssid, &sinfo, GFP_KERNEL);
+		cfg80211_new_sta(ndev, evt->bssid, sinfo, GFP_KERNEL);
+
+		kfree(sinfo);
 	} else {
 		wil_err(wil, "unhandled iftype %d for CID %d\n", wdev->iftype,
 			evt->cid);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 89b86251910e..f29f9ef521ab 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -5498,7 +5498,7 @@ brcmf_notify_connect_status_ap(struct brcmf_cfg80211_info *cfg,
 	static int generation;
 	u32 event = e->event_code;
 	u32 reason = e->reason;
-	struct station_info sinfo;
+	struct station_info *sinfo;
 
 	brcmf_dbg(CONN, "event %s (%u), reason %d\n",
 		  brcmf_fweh_event_name(event), event, reason);
@@ -5511,16 +5511,22 @@ brcmf_notify_connect_status_ap(struct brcmf_cfg80211_info *cfg,
 
 	if (((event == BRCMF_E_ASSOC_IND) || (event == BRCMF_E_REASSOC_IND)) &&
 	    (reason == BRCMF_E_STATUS_SUCCESS)) {
-		memset(&sinfo, 0, sizeof(sinfo));
 		if (!data) {
 			brcmf_err("No IEs present in ASSOC/REASSOC_IND");
 			return -EINVAL;
 		}
-		sinfo.assoc_req_ies = data;
-		sinfo.assoc_req_ies_len = e->datalen;
+
+		sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
+		if (!sinfo)
+			return -ENOMEM;
+
+		sinfo->assoc_req_ies = data;
+		sinfo->assoc_req_ies_len = e->datalen;
 		generation++;
-		sinfo.generation = generation;
-		cfg80211_new_sta(ndev, e->addr, &sinfo, GFP_KERNEL);
+		sinfo->generation = generation;
+		cfg80211_new_sta(ndev, e->addr, sinfo, GFP_KERNEL);
+
+		kfree(sinfo);
 	} else if ((event == BRCMF_E_DISASSOC_IND) ||
 		   (event == BRCMF_E_DEAUTH_IND) ||
 		   (event == BRCMF_E_DEAUTH)) {
diff --git a/drivers/net/wireless/marvell/mwifiex/uap_event.c b/drivers/net/wireless/marvell/mwifiex/uap_event.c
index e8c8728db15a..e86217a6b9ca 100644
--- a/drivers/net/wireless/marvell/mwifiex/uap_event.c
+++ b/drivers/net/wireless/marvell/mwifiex/uap_event.c
@@ -108,7 +108,7 @@ int mwifiex_process_uap_event(struct mwifiex_private *priv)
 	struct mwifiex_adapter *adapter = priv->adapter;
 	int len, i;
 	u32 eventcause = adapter->event_cause;
-	struct station_info sinfo;
+	struct station_info *sinfo;
 	struct mwifiex_assoc_event *event;
 	struct mwifiex_sta_node *node;
 	u8 *deauth_mac;
@@ -117,7 +117,10 @@ int mwifiex_process_uap_event(struct mwifiex_private *priv)
 
 	switch (eventcause) {
 	case EVENT_UAP_STA_ASSOC:
-		memset(&sinfo, 0, sizeof(sinfo));
+		sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
+		if (!sinfo)
+			return -ENOMEM;
+
 		event = (struct mwifiex_assoc_event *)
 			(adapter->event_body + MWIFIEX_UAP_EVENT_EXTRA_HEADER);
 		if (le16_to_cpu(event->type) == TLV_TYPE_UAP_MGMT_FRAME) {
@@ -132,28 +135,31 @@ int mwifiex_process_uap_event(struct mwifiex_private *priv)
 				len = ETH_ALEN;
 
 			if (len != -1) {
-				sinfo.assoc_req_ies = &event->data[len];
-				len = (u8 *)sinfo.assoc_req_ies -
+				sinfo->assoc_req_ies = &event->data[len];
+				len = (u8 *)sinfo->assoc_req_ies -
 				      (u8 *)&event->frame_control;
-				sinfo.assoc_req_ies_len =
+				sinfo->assoc_req_ies_len =
 					le16_to_cpu(event->len) - (u16)len;
 			}
 		}
-		cfg80211_new_sta(priv->netdev, event->sta_addr, &sinfo,
+		cfg80211_new_sta(priv->netdev, event->sta_addr, sinfo,
 				 GFP_KERNEL);
 
 		node = mwifiex_add_sta_entry(priv, event->sta_addr);
 		if (!node) {
 			mwifiex_dbg(adapter, ERROR,
 				    "could not create station entry!\n");
+			kfree(sinfo);
 			return -1;
 		}
 
-		if (!priv->ap_11n_enabled)
+		if (!priv->ap_11n_enabled) {
+			kfree(sinfo);
 			break;
+		}
 
-		mwifiex_set_sta_ht_cap(priv, sinfo.assoc_req_ies,
-				       sinfo.assoc_req_ies_len, node);
+		mwifiex_set_sta_ht_cap(priv, sinfo->assoc_req_ies,
+				       sinfo->assoc_req_ies_len, node);
 
 		for (i = 0; i < MAX_NUM_TID; i++) {
 			if (node->is_11n_enabled)
@@ -163,6 +169,7 @@ int mwifiex_process_uap_event(struct mwifiex_private *priv)
 				node->ampdu_sta[i] = BA_STREAM_NOT_ALLOWED;
 		}
 		memset(node->rx_seq, 0xff, sizeof(node->rx_seq));
+		kfree(sinfo);
 		break;
 	case EVENT_UAP_STA_DEAUTH:
 		deauth_mac = adapter->event_body +
diff --git a/drivers/net/wireless/quantenna/qtnfmac/event.c b/drivers/net/wireless/quantenna/qtnfmac/event.c
index bcd415f96412..e9231bd498f7 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/event.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/event.c
@@ -34,12 +34,13 @@ qtnf_event_handle_sta_assoc(struct qtnf_wmac *mac, struct qtnf_vif *vif,
 {
 	const u8 *sta_addr;
 	u16 frame_control;
-	struct station_info sinfo = { 0 };
+	struct station_info *sinfo;
 	size_t payload_len;
 	u16 tlv_type;
 	u16 tlv_value_len;
 	size_t tlv_full_len;
 	const struct qlink_tlv_hdr *tlv;
+	int ret = 0;
 
 	if (unlikely(len < sizeof(*sta_assoc))) {
 		pr_err("VIF%u.%u: payload is too short (%u < %zu)\n",
@@ -53,6 +54,10 @@ qtnf_event_handle_sta_assoc(struct qtnf_wmac *mac, struct qtnf_vif *vif,
 		return -EPROTO;
 	}
 
+	sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
+	if (!sinfo)
+		return -ENOMEM;
+
 	sta_addr = sta_assoc->sta_addr;
 	frame_control = le16_to_cpu(sta_assoc->frame_control);
 
@@ -61,9 +66,9 @@ qtnf_event_handle_sta_assoc(struct qtnf_wmac *mac, struct qtnf_vif *vif,
 
 	qtnf_sta_list_add(vif, sta_addr);
 
-	sinfo.assoc_req_ies = NULL;
-	sinfo.assoc_req_ies_len = 0;
-	sinfo.generation = vif->generation;
+	sinfo->assoc_req_ies = NULL;
+	sinfo->assoc_req_ies_len = 0;
+	sinfo->generation = vif->generation;
 
 	payload_len = len - sizeof(*sta_assoc);
 	tlv = (const struct qlink_tlv_hdr *)sta_assoc->ies;
@@ -88,8 +93,8 @@ qtnf_event_handle_sta_assoc(struct qtnf_wmac *mac, struct qtnf_vif *vif,
 				(sizeof(*ie_set) - sizeof(ie_set->hdr));
 
 			if (ie_set->type == QLINK_IE_SET_ASSOC_REQ && ie_len) {
-				sinfo.assoc_req_ies = ie_set->ie_data;
-				sinfo.assoc_req_ies_len = ie_len;
+				sinfo->assoc_req_ies = ie_set->ie_data;
+				sinfo->assoc_req_ies_len = ie_len;
 			}
 		}
 
@@ -97,13 +102,17 @@ qtnf_event_handle_sta_assoc(struct qtnf_wmac *mac, struct qtnf_vif *vif,
 		tlv = (struct qlink_tlv_hdr *)(tlv->val + tlv_value_len);
 	}
 
-	if (payload_len)
-		return -EINVAL;
+	if (payload_len) {
+		ret = -EINVAL;
+		goto out;
+	}
 
-	cfg80211_new_sta(vif->netdev, sta_assoc->sta_addr, &sinfo,
+	cfg80211_new_sta(vif->netdev, sta_assoc->sta_addr, sinfo,
 			 GFP_KERNEL);
 
-	return 0;
+out:
+	kfree(sinfo);
+	return ret;
 }
 
 static int

Re: [PATCH 3/3] net: Dynamically allocate struct station_info

[Johannes Berg]

> WARN_ON is used where the function has no way to signal errors to the
> caller.

There isn't really much point in that - failing allocations are already
*very* noisy. Please respin with that removed (and then I guess you can
fix the Fixes: too)

You didn't actually do that in this patch though :-)

johannes

> +	bool filled;
>  	int ret;
>  
>  	/* if the user specified a customised value for this interface, then
> @@ -102,7 +103,17 @@ static u32 batadv_v_elp_get_throughput(struct batadv_hardif_neigh_node *neigh)
>  		if (!real_netdev)
>  			goto default_throughput;
>  
> -		ret = cfg80211_get_station(real_netdev, neigh->addr, &sinfo);
> +		sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
> +		if (!sinfo)
> +			goto default_throughput;
> +
> +		ret = cfg80211_get_station(real_netdev, neigh->addr, sinfo);
> +
> +		/* just save these here instead of having complex free logic below */
> +		throughput = sinfo.expected_throughput / 100;
> +		filled = !!(sinfo.filled & BIT(NL80211_STA_INFO_EXPECTED_THROUGHPUT));

It's bool so you don't need the !!(...) :-)

johannes

Re: [PATCH 3/3] net: Dynamically allocate struct station_info

[Toke Høiland-Jørgensen]

Johannes Berg <johannes@sipsolutions.net> writes:

>> WARN_ON is used where the function has no way to signal errors to the
>> caller.
>
> There isn't really much point in that - failing allocations are already
> *very* noisy. Please respin with that removed (and then I guess you can
> fix the Fixes: too)

OK, will do.

> You didn't actually do that in this patch though :-)

Well, I may have copy-pasted that from the commit message of the
previous patch ;)

> It's bool so you don't need the !!(...) :-)

But what if I want it to be extra super duper bool? ;)

-Toke

Re: [PATCH 3/3] net: Dynamically allocate struct station_info

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 5180 bytes --]

Hi Toke,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on wireless-drivers-next/master]
[also build test ERROR on v4.17-rc4 next-20180509]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Toke-H-iland-J-rgensen/wireless-drivers-Dynamically-allocate-struct-station_info/20180510-034416
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master
config: i386-randconfig-x000-201818 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   net/batman-adv/bat_v_elp.c: In function 'batadv_v_elp_get_throughput':
>> net/batman-adv/bat_v_elp.c:113:21: error: 'sinfo' is a pointer; did you mean to use '->'?
      throughput = sinfo.expected_throughput / 100;
                        ^
                        ->
   net/batman-adv/bat_v_elp.c:114:20: error: 'sinfo' is a pointer; did you mean to use '->'?
      filled = !!(sinfo.filled & BIT(NL80211_STA_INFO_EXPECTED_THROUGHPUT));
                       ^
                       ->

vim +113 net/batman-adv/bat_v_elp.c

    69	
    70	/**
    71	 * batadv_v_elp_get_throughput() - get the throughput towards a neighbour
    72	 * @neigh: the neighbour for which the throughput has to be obtained
    73	 *
    74	 * Return: The throughput towards the given neighbour in multiples of 100kpbs
    75	 *         (a value of '1' equals to 0.1Mbps, '10' equals 1Mbps, etc).
    76	 */
    77	static u32 batadv_v_elp_get_throughput(struct batadv_hardif_neigh_node *neigh)
    78	{
    79		struct batadv_hard_iface *hard_iface = neigh->if_incoming;
    80		struct ethtool_link_ksettings link_settings;
    81		struct net_device *real_netdev;
    82		struct station_info *sinfo;
    83		u32 throughput;
    84		bool filled;
    85		int ret;
    86	
    87		/* if the user specified a customised value for this interface, then
    88		 * return it directly
    89		 */
    90		throughput =  atomic_read(&hard_iface->bat_v.throughput_override);
    91		if (throughput != 0)
    92			return throughput;
    93	
    94		/* if this is a wireless device, then ask its throughput through
    95		 * cfg80211 API
    96		 */
    97		if (batadv_is_wifi_hardif(hard_iface)) {
    98			if (!batadv_is_cfg80211_hardif(hard_iface))
    99				/* unsupported WiFi driver version */
   100				goto default_throughput;
   101	
   102			real_netdev = batadv_get_real_netdev(hard_iface->net_dev);
   103			if (!real_netdev)
   104				goto default_throughput;
   105	
   106			sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
   107			if (!sinfo)
   108				goto default_throughput;
   109	
   110			ret = cfg80211_get_station(real_netdev, neigh->addr, sinfo);
   111	
   112			/* just save these here instead of having complex free logic below */
 > 113			throughput = sinfo.expected_throughput / 100;
   114			filled = !!(sinfo.filled & BIT(NL80211_STA_INFO_EXPECTED_THROUGHPUT));
   115	
   116			kfree(sinfo);
   117	
   118			dev_put(real_netdev);
   119			if (ret == -ENOENT) {
   120				/* Node is not associated anymore! It would be
   121				 * possible to delete this neighbor. For now set
   122				 * the throughput metric to 0.
   123				 */
   124				return 0;
   125			}
   126			if (ret || !filled)
   127				goto default_throughput;
   128	
   129			return throughput;
   130		}
   131	
   132		/* if not a wifi interface, check if this device provides data via
   133		 * ethtool (e.g. an Ethernet adapter)
   134		 */
   135		memset(&link_settings, 0, sizeof(link_settings));
   136		rtnl_lock();
   137		ret = __ethtool_get_link_ksettings(hard_iface->net_dev, &link_settings);
   138		rtnl_unlock();
   139		if (ret == 0) {
   140			/* link characteristics might change over time */
   141			if (link_settings.base.duplex == DUPLEX_FULL)
   142				hard_iface->bat_v.flags |= BATADV_FULL_DUPLEX;
   143			else
   144				hard_iface->bat_v.flags &= ~BATADV_FULL_DUPLEX;
   145	
   146			throughput = link_settings.base.speed;
   147			if (throughput && throughput != SPEED_UNKNOWN)
   148				return throughput * 10;
   149		}
   150	
   151	default_throughput:
   152		if (!(hard_iface->bat_v.flags & BATADV_WARNING_DEFAULT)) {
   153			batadv_info(hard_iface->soft_iface,
   154				    "WiFi driver or ethtool info does not provide information about link speeds on interface %s, therefore defaulting to hardcoded throughput values of %u.%1u Mbps. Consider overriding the throughput manually or checking your driver.\n",
   155				    hard_iface->net_dev->name,
   156				    BATADV_THROUGHPUT_DEFAULT_VALUE / 10,
   157				    BATADV_THROUGHPUT_DEFAULT_VALUE % 10);
   158			hard_iface->bat_v.flags |= BATADV_WARNING_DEFAULT;
   159		}
   160	
   161		/* if none of the above cases apply, return the base_throughput */
   162		return BATADV_THROUGHPUT_DEFAULT_VALUE;
   163	}
   164	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31325 bytes --]

Re: [PATCH 3/3] net: Dynamically allocate struct station_info

[Toke Høiland-Jørgensen]

On 10 May 2018 00:55:01 CEST, kbuild test robot <lkp@intel=2Ecom> wrote:
>Hi Toke,
>
>Thank you for the patch! Yet something to improve:
>
>[auto build test ERROR on wireless-drivers-next/master]
>[also build test ERROR on v4=2E17-rc4 next-20180509]
>[if your patch is applied to the wrong git tree, please drop us a note
>to help improve the system]
>
>url:  =20
>https://github=2Ecom/0day-ci/linux/commits/Toke-H-iland-J-rgensen/wireles=
s-drivers-Dynamically-allocate-struct-station_info/20180510-034416
>base: =20
>https://git=2Ekernel=2Eorg/pub/scm/linux/kernel/git/kvalo/wireless-driver=
s-next=2Egit
>master
>config: i386-randconfig-x000-201818 (attached as =2Econfig)
>compiler: gcc-7 (Debian 7=2E3=2E0-16) 7=2E3=2E0
>reproduce:
>        # save the attached =2Econfig to linux build tree
>        make ARCH=3Di386=20
>
>All errors (new ones prefixed by >>):
>
> net/batman-adv/bat_v_elp=2Ec: In function 'batadv_v_elp_get_throughput':
>>> net/batman-adv/bat_v_elp=2Ec:113:21: error: 'sinfo' is a pointer; did
>you mean to use '->'?
>      throughput =3D sinfo=2Eexpected_throughput / 100;
>                        ^
>                        ->
>net/batman-adv/bat_v_elp=2Ec:114:20: error: 'sinfo' is a pointer; did you
>mean to use '->'?
> filled =3D !!(sinfo=2Efilled & BIT(NL80211_STA_INFO_EXPECTED_THROUGHPUT)=
);
>                       ^
>                       ->

Huh, I could swear I compile tested this=2E Will send another version tomo=
rrow, sorry about that :/

-Toke

>
>vim +113 net/batman-adv/bat_v_elp=2Ec
>
>    69=09
>    70	/**
>71	 * batadv_v_elp_get_throughput() - get the throughput towards a
>neighbour
>72	 * @neigh: the neighbour for which the throughput has to be obtained
>    73	 *
>74	 * Return: The throughput towards the given neighbour in multiples
>of 100kpbs
>75	 *         (a value of '1' equals to 0=2E1Mbps, '10' equals 1Mbps,
>etc)=2E
>    76	 */
>77	static u32 batadv_v_elp_get_throughput(struct
>batadv_hardif_neigh_node *neigh)
>    78	{
>    79		struct batadv_hard_iface *hard_iface =3D neigh->if_incoming;
>    80		struct ethtool_link_ksettings link_settings;
>    81		struct net_device *real_netdev;
>    82		struct station_info *sinfo;
>    83		u32 throughput;
>    84		bool filled;
>    85		int ret;
>    86=09
>87		/* if the user specified a customised value for this interface,
>then
>    88		 * return it directly
>    89		 */
> 90		throughput =3D  atomic_read(&hard_iface->bat_v=2Ethroughput_override=
);
>    91		if (throughput !=3D 0)
>    92			return throughput;
>    93=09
>   94		/* if this is a wireless device, then ask its throughput through
>    95		 * cfg80211 API
>    96		 */
>    97		if (batadv_is_wifi_hardif(hard_iface)) {
>    98			if (!batadv_is_cfg80211_hardif(hard_iface))
>    99				/* unsupported WiFi driver version */
>   100				goto default_throughput;
>   101=09
>   102			real_netdev =3D batadv_get_real_netdev(hard_iface->net_dev);
>   103			if (!real_netdev)
>   104				goto default_throughput;
>   105=09
>   106			sinfo =3D kzalloc(sizeof(*sinfo), GFP_KERNEL);
>   107			if (!sinfo)
>   108				goto default_throughput;
>   109=09
>   110			ret =3D cfg80211_get_station(real_netdev, neigh->addr, sinfo);
>   111=09
>112			/* just save these here instead of having complex free logic
>below */
> > 113			throughput =3D sinfo=2Eexpected_throughput / 100;
>114			filled =3D !!(sinfo=2Efilled &
>BIT(NL80211_STA_INFO_EXPECTED_THROUGHPUT));
>   115=09
>   116			kfree(sinfo);
>   117=09
>   118			dev_put(real_netdev);
>   119			if (ret =3D=3D -ENOENT) {
>   120				/* Node is not associated anymore! It would be
>   121				 * possible to delete this neighbor=2E For now set
>   122				 * the throughput metric to 0=2E
>   123				 */
>   124				return 0;
>   125			}
>   126			if (ret || !filled)
>   127				goto default_throughput;
>   128=09
>   129			return throughput;
>   130		}
>   131=09
>132		/* if not a wifi interface, check if this device provides data via
>   133		 * ethtool (e=2Eg=2E an Ethernet adapter)
>   134		 */
>   135		memset(&link_settings, 0, sizeof(link_settings));
>   136		rtnl_lock();
>137		ret =3D __ethtool_get_link_ksettings(hard_iface->net_dev,
>&link_settings);
>   138		rtnl_unlock();
>   139		if (ret =3D=3D 0) {
>   140			/* link characteristics might change over time */
>   141			if (link_settings=2Ebase=2Eduplex =3D=3D DUPLEX_FULL)
>   142				hard_iface->bat_v=2Eflags |=3D BATADV_FULL_DUPLEX;
>   143			else
>   144				hard_iface->bat_v=2Eflags &=3D ~BATADV_FULL_DUPLEX;
>   145=09
>   146			throughput =3D link_settings=2Ebase=2Espeed;
>   147			if (throughput && throughput !=3D SPEED_UNKNOWN)
>   148				return throughput * 10;
>   149		}
>   150=09
>   151	default_throughput:
>   152		if (!(hard_iface->bat_v=2Eflags & BATADV_WARNING_DEFAULT)) {
>   153			batadv_info(hard_iface->soft_iface,
>154				    "WiFi driver or ethtool info does not provide information
>about link speeds on interface %s, therefore defaulting to hardcoded
>throughput values of %u=2E%1u Mbps=2E Consider overriding the throughput
>manually or checking your driver=2E\n",
>   155				    hard_iface->net_dev->name,
>   156				    BATADV_THROUGHPUT_DEFAULT_VALUE / 10,
>   157				    BATADV_THROUGHPUT_DEFAULT_VALUE % 10);
>   158			hard_iface->bat_v=2Eflags |=3D BATADV_WARNING_DEFAULT;
>   159		}
>   160=09
>161		/* if none of the above cases apply, return the base_throughput */
>   162		return BATADV_THROUGHPUT_DEFAULT_VALUE;
>   163	}
>   164=09
>
>---
>0-DAY kernel test infrastructure                Open Source Technology
>Center
>https://lists=2E01=2Eorg/pipermail/kbuild-all                   Intel
>Corporation

Re: [PATCH 3/3] net: Dynamically allocate struct station_info

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 5210 bytes --]

Hi Toke,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on wireless-drivers-next/master]
[also build test ERROR on v4.17-rc4 next-20180509]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Toke-H-iland-J-rgensen/wireless-drivers-Dynamically-allocate-struct-station_info/20180510-034416
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master
config: i386-randconfig-x0-05100327 (attached as .config)
compiler: gcc-5 (Debian 5.5.0-3) 5.4.1 20171010
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   net//batman-adv/bat_v_elp.c: In function 'batadv_v_elp_get_throughput':
>> net//batman-adv/bat_v_elp.c:113:21: error: request for member 'expected_throughput' in something not a structure or union
      throughput = sinfo.expected_throughput / 100;
                        ^
>> net//batman-adv/bat_v_elp.c:114:20: error: request for member 'filled' in something not a structure or union
      filled = !!(sinfo.filled & BIT(NL80211_STA_INFO_EXPECTED_THROUGHPUT));
                       ^

vim +/expected_throughput +113 net//batman-adv/bat_v_elp.c

    69	
    70	/**
    71	 * batadv_v_elp_get_throughput() - get the throughput towards a neighbour
    72	 * @neigh: the neighbour for which the throughput has to be obtained
    73	 *
    74	 * Return: The throughput towards the given neighbour in multiples of 100kpbs
    75	 *         (a value of '1' equals to 0.1Mbps, '10' equals 1Mbps, etc).
    76	 */
    77	static u32 batadv_v_elp_get_throughput(struct batadv_hardif_neigh_node *neigh)
    78	{
    79		struct batadv_hard_iface *hard_iface = neigh->if_incoming;
    80		struct ethtool_link_ksettings link_settings;
    81		struct net_device *real_netdev;
    82		struct station_info *sinfo;
    83		u32 throughput;
    84		bool filled;
    85		int ret;
    86	
    87		/* if the user specified a customised value for this interface, then
    88		 * return it directly
    89		 */
    90		throughput =  atomic_read(&hard_iface->bat_v.throughput_override);
    91		if (throughput != 0)
    92			return throughput;
    93	
    94		/* if this is a wireless device, then ask its throughput through
    95		 * cfg80211 API
    96		 */
    97		if (batadv_is_wifi_hardif(hard_iface)) {
    98			if (!batadv_is_cfg80211_hardif(hard_iface))
    99				/* unsupported WiFi driver version */
   100				goto default_throughput;
   101	
   102			real_netdev = batadv_get_real_netdev(hard_iface->net_dev);
   103			if (!real_netdev)
   104				goto default_throughput;
   105	
   106			sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
   107			if (!sinfo)
   108				goto default_throughput;
   109	
   110			ret = cfg80211_get_station(real_netdev, neigh->addr, sinfo);
   111	
   112			/* just save these here instead of having complex free logic below */
 > 113			throughput = sinfo.expected_throughput / 100;
 > 114			filled = !!(sinfo.filled & BIT(NL80211_STA_INFO_EXPECTED_THROUGHPUT));
   115	
   116			kfree(sinfo);
   117	
   118			dev_put(real_netdev);
   119			if (ret == -ENOENT) {
   120				/* Node is not associated anymore! It would be
   121				 * possible to delete this neighbor. For now set
   122				 * the throughput metric to 0.
   123				 */
   124				return 0;
   125			}
   126			if (ret || !filled)
   127				goto default_throughput;
   128	
   129			return throughput;
   130		}
   131	
   132		/* if not a wifi interface, check if this device provides data via
   133		 * ethtool (e.g. an Ethernet adapter)
   134		 */
   135		memset(&link_settings, 0, sizeof(link_settings));
   136		rtnl_lock();
   137		ret = __ethtool_get_link_ksettings(hard_iface->net_dev, &link_settings);
   138		rtnl_unlock();
   139		if (ret == 0) {
   140			/* link characteristics might change over time */
   141			if (link_settings.base.duplex == DUPLEX_FULL)
   142				hard_iface->bat_v.flags |= BATADV_FULL_DUPLEX;
   143			else
   144				hard_iface->bat_v.flags &= ~BATADV_FULL_DUPLEX;
   145	
   146			throughput = link_settings.base.speed;
   147			if (throughput && throughput != SPEED_UNKNOWN)
   148				return throughput * 10;
   149		}
   150	
   151	default_throughput:
   152		if (!(hard_iface->bat_v.flags & BATADV_WARNING_DEFAULT)) {
   153			batadv_info(hard_iface->soft_iface,
   154				    "WiFi driver or ethtool info does not provide information about link speeds on interface %s, therefore defaulting to hardcoded throughput values of %u.%1u Mbps. Consider overriding the throughput manually or checking your driver.\n",
   155				    hard_iface->net_dev->name,
   156				    BATADV_THROUGHPUT_DEFAULT_VALUE / 10,
   157				    BATADV_THROUGHPUT_DEFAULT_VALUE % 10);
   158			hard_iface->bat_v.flags |= BATADV_WARNING_DEFAULT;
   159		}
   160	
   161		/* if none of the above cases apply, return the base_throughput */
   162		return BATADV_THROUGHPUT_DEFAULT_VALUE;
   163	}
   164	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29664 bytes --]