[PATCH 1/3] mac80211: fix some snprintf misuses

[PATCH 1/3] mac80211: fix some snprintf misuses

[Johannes Berg]

From: Eliad Peller <eliad@wizery.com>

In some debugfs related functions snprintf was used
while scnprintf should have been used instead.

(blindly adding the return value of snprintf and supplying
it to the next snprintf might result in buffer overflow when
the input is too big)

Signed-off-by: Eliad Peller <eliad@wizery.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/debugfs.c             | 55 ++++++++++++++++++++------------------
 net/mac80211/rc80211_pid_debugfs.c | 26 +++++++++---------
 net/wireless/debugfs.c             | 24 +++++++++--------
 3 files changed, 55 insertions(+), 50 deletions(-)

diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c
index b0e32d6..5c090e4 100644
--- a/net/mac80211/debugfs.c
+++ b/net/mac80211/debugfs.c
@@ -103,54 +103,57 @@ static ssize_t hwflags_read(struct file *file, char __user *user_buf,
 	if (!buf)
 		return 0;
 
-	sf += snprintf(buf, mxln - sf, "0x%x\n", local->hw.flags);
+	sf += scnprintf(buf, mxln - sf, "0x%x\n", local->hw.flags);
 	if (local->hw.flags & IEEE80211_HW_HAS_RATE_CONTROL)
-		sf += snprintf(buf + sf, mxln - sf, "HAS_RATE_CONTROL\n");
+		sf += scnprintf(buf + sf, mxln - sf, "HAS_RATE_CONTROL\n");
 	if (local->hw.flags & IEEE80211_HW_RX_INCLUDES_FCS)
-		sf += snprintf(buf + sf, mxln - sf, "RX_INCLUDES_FCS\n");
+		sf += scnprintf(buf + sf, mxln - sf, "RX_INCLUDES_FCS\n");
 	if (local->hw.flags & IEEE80211_HW_HOST_BROADCAST_PS_BUFFERING)
-		sf += snprintf(buf + sf, mxln - sf,
-			       "HOST_BCAST_PS_BUFFERING\n");
+		sf += scnprintf(buf + sf, mxln - sf,
+				"HOST_BCAST_PS_BUFFERING\n");
 	if (local->hw.flags & IEEE80211_HW_2GHZ_SHORT_SLOT_INCAPABLE)
-		sf += snprintf(buf + sf, mxln - sf,
-			       "2GHZ_SHORT_SLOT_INCAPABLE\n");
+		sf += scnprintf(buf + sf, mxln - sf,
+				"2GHZ_SHORT_SLOT_INCAPABLE\n");
 	if (local->hw.flags & IEEE80211_HW_2GHZ_SHORT_PREAMBLE_INCAPABLE)
-		sf += snprintf(buf + sf, mxln - sf,
-			       "2GHZ_SHORT_PREAMBLE_INCAPABLE\n");
+		sf += scnprintf(buf + sf, mxln - sf,
+				"2GHZ_SHORT_PREAMBLE_INCAPABLE\n");
 	if (local->hw.flags & IEEE80211_HW_SIGNAL_UNSPEC)
-		sf += snprintf(buf + sf, mxln - sf, "SIGNAL_UNSPEC\n");
+		sf += scnprintf(buf + sf, mxln - sf, "SIGNAL_UNSPEC\n");
 	if (local->hw.flags & IEEE80211_HW_SIGNAL_DBM)
-		sf += snprintf(buf + sf, mxln - sf, "SIGNAL_DBM\n");
+		sf += scnprintf(buf + sf, mxln - sf, "SIGNAL_DBM\n");
 	if (local->hw.flags & IEEE80211_HW_NEED_DTIM_BEFORE_ASSOC)
-		sf += snprintf(buf + sf, mxln - sf, "NEED_DTIM_BEFORE_ASSOC\n");
+		sf += scnprintf(buf + sf, mxln - sf,
+				"NEED_DTIM_BEFORE_ASSOC\n");
 	if (local->hw.flags & IEEE80211_HW_SPECTRUM_MGMT)
-		sf += snprintf(buf + sf, mxln - sf, "SPECTRUM_MGMT\n");
+		sf += scnprintf(buf + sf, mxln - sf, "SPECTRUM_MGMT\n");
 	if (local->hw.flags & IEEE80211_HW_AMPDU_AGGREGATION)
-		sf += snprintf(buf + sf, mxln - sf, "AMPDU_AGGREGATION\n");
+		sf += scnprintf(buf + sf, mxln - sf, "AMPDU_AGGREGATION\n");
 	if (local->hw.flags & IEEE80211_HW_SUPPORTS_PS)
-		sf += snprintf(buf + sf, mxln - sf, "SUPPORTS_PS\n");
+		sf += scnprintf(buf + sf, mxln - sf, "SUPPORTS_PS\n");
 	if (local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK)
-		sf += snprintf(buf + sf, mxln - sf, "PS_NULLFUNC_STACK\n");
+		sf += scnprintf(buf + sf, mxln - sf, "PS_NULLFUNC_STACK\n");
 	if (local->hw.flags & IEEE80211_HW_SUPPORTS_DYNAMIC_PS)
-		sf += snprintf(buf + sf, mxln - sf, "SUPPORTS_DYNAMIC_PS\n");
+		sf += scnprintf(buf + sf, mxln - sf, "SUPPORTS_DYNAMIC_PS\n");
 	if (local->hw.flags & IEEE80211_HW_MFP_CAPABLE)
-		sf += snprintf(buf + sf, mxln - sf, "MFP_CAPABLE\n");
+		sf += scnprintf(buf + sf, mxln - sf, "MFP_CAPABLE\n");
 	if (local->hw.flags & IEEE80211_HW_SUPPORTS_STATIC_SMPS)
-		sf += snprintf(buf + sf, mxln - sf, "SUPPORTS_STATIC_SMPS\n");
+		sf += scnprintf(buf + sf, mxln - sf, "SUPPORTS_STATIC_SMPS\n");
 	if (local->hw.flags & IEEE80211_HW_SUPPORTS_DYNAMIC_SMPS)
-		sf += snprintf(buf + sf, mxln - sf, "SUPPORTS_DYNAMIC_SMPS\n");
+		sf += scnprintf(buf + sf, mxln - sf,
+				"SUPPORTS_DYNAMIC_SMPS\n");
 	if (local->hw.flags & IEEE80211_HW_SUPPORTS_UAPSD)
-		sf += snprintf(buf + sf, mxln - sf, "SUPPORTS_UAPSD\n");
+		sf += scnprintf(buf + sf, mxln - sf, "SUPPORTS_UAPSD\n");
 	if (local->hw.flags & IEEE80211_HW_REPORTS_TX_ACK_STATUS)
-		sf += snprintf(buf + sf, mxln - sf, "REPORTS_TX_ACK_STATUS\n");
+		sf += scnprintf(buf + sf, mxln - sf,
+				"REPORTS_TX_ACK_STATUS\n");
 	if (local->hw.flags & IEEE80211_HW_CONNECTION_MONITOR)
-		sf += snprintf(buf + sf, mxln - sf, "CONNECTION_MONITOR\n");
+		sf += scnprintf(buf + sf, mxln - sf, "CONNECTION_MONITOR\n");
 	if (local->hw.flags & IEEE80211_HW_SUPPORTS_PER_STA_GTK)
-		sf += snprintf(buf + sf, mxln - sf, "SUPPORTS_PER_STA_GTK\n");
+		sf += scnprintf(buf + sf, mxln - sf, "SUPPORTS_PER_STA_GTK\n");
 	if (local->hw.flags & IEEE80211_HW_AP_LINK_PS)
-		sf += snprintf(buf + sf, mxln - sf, "AP_LINK_PS\n");
+		sf += scnprintf(buf + sf, mxln - sf, "AP_LINK_PS\n");
 	if (local->hw.flags & IEEE80211_HW_TX_AMPDU_SETUP_IN_HW)
-		sf += snprintf(buf + sf, mxln - sf, "TX_AMPDU_SETUP_IN_HW\n");
+		sf += scnprintf(buf + sf, mxln - sf, "TX_AMPDU_SETUP_IN_HW\n");
 
 	rv = simple_read_from_buffer(user_buf, count, ppos, buf, strlen(buf));
 	kfree(buf);
diff --git a/net/mac80211/rc80211_pid_debugfs.c b/net/mac80211/rc80211_pid_debugfs.c
index c97a065..6ff1346 100644
--- a/net/mac80211/rc80211_pid_debugfs.c
+++ b/net/mac80211/rc80211_pid_debugfs.c
@@ -167,29 +167,29 @@ static ssize_t rate_control_pid_events_read(struct file *file, char __user *buf,
 	 * provide large enough buffers. */
 	length = length < RC_PID_PRINT_BUF_SIZE ?
 		 length : RC_PID_PRINT_BUF_SIZE;
-	p = snprintf(pb, length, "%u %lu ", ev->id, ev->timestamp);
+	p = scnprintf(pb, length, "%u %lu ", ev->id, ev->timestamp);
 	switch (ev->type) {
 	case RC_PID_EVENT_TYPE_TX_STATUS:
-		p += snprintf(pb + p, length - p, "tx_status %u %u",
-			      !(ev->data.flags & IEEE80211_TX_STAT_ACK),
-			      ev->data.tx_status.status.rates[0].idx);
+		p += scnprintf(pb + p, length - p, "tx_status %u %u",
+			       !(ev->data.flags & IEEE80211_TX_STAT_ACK),
+			       ev->data.tx_status.status.rates[0].idx);
 		break;
 	case RC_PID_EVENT_TYPE_RATE_CHANGE:
-		p += snprintf(pb + p, length - p, "rate_change %d %d",
-			      ev->data.index, ev->data.rate);
+		p += scnprintf(pb + p, length - p, "rate_change %d %d",
+			       ev->data.index, ev->data.rate);
 		break;
 	case RC_PID_EVENT_TYPE_TX_RATE:
-		p += snprintf(pb + p, length - p, "tx_rate %d %d",
-			      ev->data.index, ev->data.rate);
+		p += scnprintf(pb + p, length - p, "tx_rate %d %d",
+			       ev->data.index, ev->data.rate);
 		break;
 	case RC_PID_EVENT_TYPE_PF_SAMPLE:
-		p += snprintf(pb + p, length - p,
-			      "pf_sample %d %d %d %d",
-			      ev->data.pf_sample, ev->data.prop_err,
-			      ev->data.int_err, ev->data.der_err);
+		p += scnprintf(pb + p, length - p,
+			       "pf_sample %d %d %d %d",
+			       ev->data.pf_sample, ev->data.prop_err,
+			       ev->data.int_err, ev->data.der_err);
 		break;
 	}
-	p += snprintf(pb + p, length - p, "\n");
+	p += scnprintf(pb + p, length - p, "\n");
 
 	spin_unlock_irqrestore(&events->lock, status);
 
diff --git a/net/wireless/debugfs.c b/net/wireless/debugfs.c
index 90d0500..4541577 100644
--- a/net/wireless/debugfs.c
+++ b/net/wireless/debugfs.c
@@ -47,17 +47,19 @@ static int ht_print_chan(struct ieee80211_channel *chan,
 		return 0;
 
 	if (chan->flags & IEEE80211_CHAN_DISABLED)
-		return snprintf(buf + offset,
-				buf_size - offset,
-				"%d Disabled\n",
-				chan->center_freq);
-
-	return snprintf(buf + offset,
-			buf_size - offset,
-			"%d HT40 %c%c\n",
-			chan->center_freq,
-			(chan->flags & IEEE80211_CHAN_NO_HT40MINUS) ? ' ' : '-',
-			(chan->flags & IEEE80211_CHAN_NO_HT40PLUS)  ? ' ' : '+');
+		return scnprintf(buf + offset,
+				 buf_size - offset,
+				 "%d Disabled\n",
+				 chan->center_freq);
+
+	return scnprintf(buf + offset,
+			 buf_size - offset,
+			 "%d HT40 %c%c\n",
+			 chan->center_freq,
+			 (chan->flags & IEEE80211_CHAN_NO_HT40MINUS) ?
+				' ' : '-',
+			 (chan->flags & IEEE80211_CHAN_NO_HT40PLUS) ?
+				' ' : '+');
 }
 
 static ssize_t ht40allow_map_read(struct file *file,
-- 
1.8.4.rc3

[PATCH 2/3] ieee80211: fix vht cap definitions

[Johannes Berg]

From: Eliad Peller <eliad@wizery.com>

VHT_CAP_BEAMFORMER_ANTENNAS cap is actually defined in the draft as
VHT_CAP_BEAMFORMEE_STS_MAX, and its size is 3 bits long.

VHT_CAP_SOUNDING_DIMENSIONS is also 3 bits long.

Fix the definitions and change the cap masking accordingly.

Signed-off-by: Eliad Peller <eliad@wizery.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/linux/ieee80211.h | 4 ++--
 net/mac80211/vht.c        | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index 23a8877..57b699e 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -1390,8 +1390,8 @@ struct ieee80211_vht_operation {
 #define IEEE80211_VHT_CAP_RXSTBC_MASK				0x00000700
 #define IEEE80211_VHT_CAP_SU_BEAMFORMER_CAPABLE			0x00000800
 #define IEEE80211_VHT_CAP_SU_BEAMFORMEE_CAPABLE			0x00001000
-#define IEEE80211_VHT_CAP_BEAMFORMER_ANTENNAS_MAX		0x00006000
-#define IEEE80211_VHT_CAP_SOUNDING_DIMENSIONS_MAX		0x00030000
+#define IEEE80211_VHT_CAP_BEAMFORMEE_STS_MAX			0x0000e000
+#define IEEE80211_VHT_CAP_SOUNDING_DIMENSIONS_MAX		0x00070000
 #define IEEE80211_VHT_CAP_MU_BEAMFORMER_CAPABLE			0x00080000
 #define IEEE80211_VHT_CAP_MU_BEAMFORMEE_CAPABLE			0x00100000
 #define IEEE80211_VHT_CAP_VHT_TXOP_PS				0x00200000
diff --git a/net/mac80211/vht.c b/net/mac80211/vht.c
index 97c28941..de01127 100644
--- a/net/mac80211/vht.c
+++ b/net/mac80211/vht.c
@@ -185,13 +185,13 @@ ieee80211_vht_cap_ie_to_sta_vht_cap(struct ieee80211_sub_if_data *sdata,
 	if (own_cap.cap & IEEE80211_VHT_CAP_SU_BEAMFORMEE_CAPABLE) {
 		vht_cap->cap |= cap_info &
 				(IEEE80211_VHT_CAP_SU_BEAMFORMER_CAPABLE |
-				 IEEE80211_VHT_CAP_BEAMFORMER_ANTENNAS_MAX |
 				 IEEE80211_VHT_CAP_SOUNDING_DIMENSIONS_MAX);
 	}
 
 	if (own_cap.cap & IEEE80211_VHT_CAP_SU_BEAMFORMER_CAPABLE)
 		vht_cap->cap |= cap_info &
-				IEEE80211_VHT_CAP_SU_BEAMFORMEE_CAPABLE;
+				(IEEE80211_VHT_CAP_SU_BEAMFORMEE_CAPABLE |
+				 IEEE80211_VHT_CAP_BEAMFORMEE_STS_MAX);
 
 	if (own_cap.cap & IEEE80211_VHT_CAP_MU_BEAMFORMER_CAPABLE)
 		vht_cap->cap |= cap_info &
-- 
1.8.4.rc3

Re: [PATCH 2/3] ieee80211: fix vht cap definitions

[Johannes Berg]

On Fri, 2013-08-30 at 10:49 +0200, Johannes Berg wrote:
> From: Eliad Peller <eliad@wizery.com>
> 
> VHT_CAP_BEAMFORMER_ANTENNAS cap is actually defined in the draft as
> VHT_CAP_BEAMFORMEE_STS_MAX, and its size is 3 bits long.
> 
> VHT_CAP_SOUNDING_DIMENSIONS is also 3 bits long.
> 
> Fix the definitions and change the cap masking accordingly.

Applied.

johannes

[PATCH 3/3] mac80211: allow configuring smps_mode_in_ps

[Johannes Berg]

From: Eliad Peller <eliad@wizery.com>

When in automatic smps mode (default for station mode),
mac80211 configures dynamic smps when the station enters
powersave.

However, drivers might prefer using different modes in this
case (e.g. due to major throughput degradation).

Add a new hw->smps_mode_in_ps field that can be configured
by the driver in order to override the default smps dynamic mode.

Signed-off-by: Eliad Peller <eliad@wizery.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/net/mac80211.h        |  5 +++++
 net/mac80211/cfg.c            |  2 +-
 net/mac80211/debugfs_netdev.c | 11 +++++++----
 net/mac80211/main.c           |  1 +
 net/mac80211/mlme.c           |  7 ++++++-
 5 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 3ecf635..637b797 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1615,6 +1615,10 @@ enum ieee80211_hw_flags {
  * @uapsd_max_sp_len: maximum number of total buffered frames the WMM AP may
  *	deliver to a WMM STA during any Service Period triggered by the WMM STA.
  *	Use IEEE80211_WMM_IE_STA_QOSINFO_SP_* for correct values.
+ *
+ * @smps_mode_in_ps: smps mode to use when automatic smps mode is configured.
+ *	The default is _DYNAMIC.
+ *	Use the %IEEE80211_SMPS_* values.
  */
 struct ieee80211_hw {
 	struct ieee80211_conf conf;
@@ -1642,6 +1646,7 @@ struct ieee80211_hw {
 	netdev_features_t netdev_features;
 	u8 uapsd_queues;
 	u8 uapsd_max_sp_len;
+	enum ieee80211_smps_mode smps_mode_in_ps;
 };
 
 /**
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 7aa38ce..1932126 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2364,7 +2364,7 @@ int __ieee80211_request_smps(struct ieee80211_sub_if_data *sdata,
 
 	if (smps_mode == IEEE80211_SMPS_AUTOMATIC) {
 		if (sdata->u.mgd.powersave)
-			smps_mode = IEEE80211_SMPS_DYNAMIC;
+			smps_mode = sdata->local->hw.smps_mode_in_ps;
 		else
 			smps_mode = IEEE80211_SMPS_OFF;
 	}
diff --git a/net/mac80211/debugfs_netdev.c b/net/mac80211/debugfs_netdev.c
index cafe614..66dff46 100644
--- a/net/mac80211/debugfs_netdev.c
+++ b/net/mac80211/debugfs_netdev.c
@@ -212,16 +212,19 @@ static int ieee80211_set_smps(struct ieee80211_sub_if_data *sdata,
 			      enum ieee80211_smps_mode smps_mode)
 {
 	struct ieee80211_local *local = sdata->local;
+	enum ieee80211_smps_mode target_mode = smps_mode;
 	int err;
 
+	/* in case of auto, hw.smps_mode_in_ps will be configured when in psm */
+	if (smps_mode == IEEE80211_SMPS_AUTOMATIC)
+		target_mode = local->hw.smps_mode_in_ps;
+
 	if (!(local->hw.flags & IEEE80211_HW_SUPPORTS_STATIC_SMPS) &&
-	    smps_mode == IEEE80211_SMPS_STATIC)
+	    target_mode == IEEE80211_SMPS_STATIC)
 		return -EINVAL;
 
-	/* auto should be dynamic if in PS mode */
 	if (!(local->hw.flags & IEEE80211_HW_SUPPORTS_DYNAMIC_SMPS) &&
-	    (smps_mode == IEEE80211_SMPS_DYNAMIC ||
-	     smps_mode == IEEE80211_SMPS_AUTOMATIC))
+	    target_mode == IEEE80211_SMPS_DYNAMIC)
 		return -EINVAL;
 
 	/* supported only on managed interfaces for now */
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 21d5d44..885c8e3 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -585,6 +585,7 @@ struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len,
 					 IEEE80211_RADIOTAP_VHT_KNOWN_BANDWIDTH;
 	local->hw.uapsd_queues = IEEE80211_DEFAULT_UAPSD_QUEUES;
 	local->hw.uapsd_max_sp_len = IEEE80211_DEFAULT_MAX_SP_LEN;
+	local->hw.smps_mode_in_ps = IEEE80211_SMPS_DYNAMIC;
 	local->user_power_level = IEEE80211_UNSET_POWER_LEVEL;
 	wiphy->ht_capa_mod_mask = &mac80211_ht_capa_mod_mask;
 	wiphy->vht_capa_mod_mask = &mac80211_vht_capa_mod_mask;
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 45a87ee..007cc4d 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -3700,7 +3700,12 @@ void ieee80211_sta_setup_sdata(struct ieee80211_sub_if_data *sdata)
 	ifmgd->uapsd_max_sp_len = sdata->local->hw.uapsd_max_sp_len;
 	ifmgd->p2p_noa_index = -1;
 
-	if (sdata->local->hw.flags & IEEE80211_HW_SUPPORTS_DYNAMIC_SMPS)
+	/* set automatic smps by default, if hw.smps_mode_in_ps is supported */
+	if (sdata->local->hw.flags & IEEE80211_HW_SUPPORTS_DYNAMIC_SMPS &&
+	    sdata->local->hw.smps_mode_in_ps == IEEE80211_SMPS_DYNAMIC)
+		ifmgd->req_smps = IEEE80211_SMPS_AUTOMATIC;
+	else if (sdata->local->hw.flags & IEEE80211_HW_SUPPORTS_STATIC_SMPS &&
+		 sdata->local->hw.smps_mode_in_ps == IEEE80211_SMPS_STATIC)
 		ifmgd->req_smps = IEEE80211_SMPS_AUTOMATIC;
 	else
 		ifmgd->req_smps = IEEE80211_SMPS_OFF;
-- 
1.8.4.rc3

Re: [PATCH 3/3] mac80211: allow configuring smps_mode_in_ps

[Johannes Berg]

On Fri, 2013-08-30 at 10:49 +0200, Johannes Berg wrote:
> From: Eliad Peller <eliad@wizery.com>
> 
> When in automatic smps mode (default for station mode),
> mac80211 configures dynamic smps when the station enters
> powersave.
> 
> However, drivers might prefer using different modes in this
> case (e.g. due to major throughput degradation).
> 
> Add a new hw->smps_mode_in_ps field that can be configured
> by the driver in order to override the default smps dynamic mode.

Dropping this, we ended up not wanting/needing it.

johannes

Re: [PATCH 1/3] mac80211: fix some snprintf misuses

[Johannes Berg]

On Fri, 2013-08-30 at 10:49 +0200, Johannes Berg wrote:
> From: Eliad Peller <eliad@wizery.com>
> 
> In some debugfs related functions snprintf was used
> while scnprintf should have been used instead.
> 
> (blindly adding the return value of snprintf and supplying
> it to the next snprintf might result in buffer overflow when
> the input is too big)

Applied.

johannes