diff mbox series

[V3,2/9] mac80211: add multiple bssid support to interface handling

Message ID 20200812150050.2683396-3-john@phrozen.org
State New
Headers show
Series None | expand

Commit Message

John Crispin Aug. 12, 2020, 3 p.m. UTC 
When bringing up multi bssid APs we need to track the parent-child relation
of non-transmitting and transmitting VAPs. This patch checks the above by
using a linked list to track the relations. The patch also ensures that
when a non-transmitting interface is closed the transmitting one is also
closed.

Signed-off-by: John Crispin <john@phrozen.org>
---
 include/net/mac80211.h | 13 +++++++++-
 net/mac80211/cfg.c     | 54 ++++++++++++++++++++++++++++++++++++++++++
 net/mac80211/debugfs.c |  1 +
 net/mac80211/iface.c   |  7 ++++++
 4 files changed, 74 insertions(+), 1 deletion(-)

Comments

Pradeep Kumar Chitrapu Oct. 8, 2020, 12:33 a.m. UTC | #1 
> 
>  static int ieee80211_del_iface(struct wiphy *wiphy, struct 
> wireless_dev *wdev)
>  {
> +	struct ieee80211_sub_if_data *sdata;
> +	struct ieee80211_vif *child, *tmp;
> +
> +	sdata = IEEE80211_WDEV_TO_SUB_IF(wdev);
> +	if (sdata->vif.type == NL80211_IFTYPE_AP) {
Hi John, Observed a NULL ptr dereference error here..

Thanks
Pradeep
John Crispin Oct. 8, 2020, 8:06 a.m. UTC | #2 
On 08.10.20 02:33, Pradeep Kumar Chitrapu wrote:
>>
>>  static int ieee80211_del_iface(struct wiphy *wiphy, struct 
>> wireless_dev *wdev)
>>  {
>> +    struct ieee80211_sub_if_data *sdata;
>> +    struct ieee80211_vif *child, *tmp;
>> +
>> +    sdata = IEEE80211_WDEV_TO_SUB_IF(wdev);
>> +    if (sdata->vif.type == NL80211_IFTYPE_AP) {
> Hi John, Observed a NULL ptr dereference error here..
>
> Thanks
> Pradeep


how did you trigger it ?

     John
Pradeep Kumar Chitrapu Oct. 8, 2020, 5:21 p.m. UTC | #3 
On 2020-10-08 01:06, John Crispin wrote:
> On 08.10.20 02:33, Pradeep Kumar Chitrapu wrote:
>>> 
>>>  static int ieee80211_del_iface(struct wiphy *wiphy, struct 
>>> wireless_dev *wdev)
>>>  {
>>> +    struct ieee80211_sub_if_data *sdata;
>>> +    struct ieee80211_vif *child, *tmp;
>>> +
>>> +    sdata = IEEE80211_WDEV_TO_SUB_IF(wdev);
>>> +    if (sdata->vif.type == NL80211_IFTYPE_AP) {
>> Hi John, Observed a NULL ptr dereference error here..
>> 
>> Thanks
>> Pradeep
> 
> 
> how did you trigger it ?
> 
>     John
Hi

Deleted the interface and did rmmod and insmod of cfg80211/mac80211/ath 
modules.

[  883.565933] Unable to handle kernel NULL pointer dereference at 
virtual address 00000000
[  883.565970] pgd = b311c000
[  883.573357] [00000000] *pgd=00000000
[  883.579021] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
[  883.848257] task: bd1ac600 ti: b027a000 task.ti: b027a000
[  883.852904] PC is at ieee80211_del_iface+0x34/0x90 [mac80211]
[  883.858333] LR is at extack_doit+0x20/0x6c [compat]
[  884.092936] [<c751fbd8>] (ieee80211_del_iface [mac80211]) from 
[<7f56181c>] (extack_doit+0x20/0x6c [compat])
[  884.100991] [<7f56181c>] (extack_doit [compat]) from [<8076a340>] 
(genl_rcv_msg+0x27c/0x300)
[  884.110854] [<8076a340>] (genl_rcv_msg) from [<807696c0>] 
(netlink_rcv_skb+0x58/0xb4)

Thanks
Pradeep
John Crispin Oct. 8, 2020, 7:42 p.m. UTC | #4 
On 08.10.20 19:21, Pradeep Kumar Chitrapu wrote:
> On 2020-10-08 01:06, John Crispin wrote:

>> On 08.10.20 02:33, Pradeep Kumar Chitrapu wrote:

>>>>

>>>>  static int ieee80211_del_iface(struct wiphy *wiphy, struct 

>>>> wireless_dev *wdev)

>>>>  {

>>>> +    struct ieee80211_sub_if_data *sdata;

>>>> +    struct ieee80211_vif *child, *tmp;

>>>> +

>>>> +    sdata = IEEE80211_WDEV_TO_SUB_IF(wdev);

>>>> +    if (sdata->vif.type == NL80211_IFTYPE_AP) {

>>> Hi John, Observed a NULL ptr dereference error here..

>>>

>>> Thanks

>>> Pradeep

>>

>>

>> how did you trigger it ?

>>

>>     John

> Hi

>

> Deleted the interface and did rmmod and insmod of 

> cfg80211/mac80211/ath modules.

>

> [  883.565933] Unable to handle kernel NULL pointer dereference at 

> virtual address 00000000

> [  883.565970] pgd = b311c000

> [  883.573357] [00000000] *pgd=00000000

> [  883.579021] Internal error: Oops: 5 [#1] PREEMPT SMP ARM

> [  883.848257] task: bd1ac600 ti: b027a000 task.ti: b027a000

> [  883.852904] PC is at ieee80211_del_iface+0x34/0x90 [mac80211]

> [  883.858333] LR is at extack_doit+0x20/0x6c [compat]

> [  884.092936] [<c751fbd8>] (ieee80211_del_iface [mac80211]) from 

> [<7f56181c>] (extack_doit+0x20/0x6c [compat])

> [  884.100991] [<7f56181c>] (extack_doit [compat]) from [<8076a340>] 

> (genl_rcv_msg+0x27c/0x300)

> [  884.110854] [<8076a340>] (genl_rcv_msg) from [<807696c0>] 

> (netlink_rcv_skb+0x58/0xb4)

>

> Thanks

> Pradeep

>

last I tested rmmod/insmod will always crash the kernel when using ath11k

however i'll ass !NULL guard in the next series

     John
diff mbox series

Patch

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index f0ae718633d2..b409a5f1026c 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1649,6 +1649,9 @@  enum ieee80211_vif_flags {
  *	write-protected by sdata_lock and local->mtx so holding either is fine
  *	for read access.
  * @cca_color: the color that we will have after the change.
+ * @multiple_bssid.parent: a non-transmitted bssid has a transmitted parent.
+ * @multiple_bssid.list: linked list for tracking parent - child relations.
+ * @multiple_bssid.non_transmitted: Is this a non-transmitted bssi
  */
 struct ieee80211_vif {
 	enum nl80211_iftype type;
@@ -1675,6 +1678,11 @@  struct ieee80211_vif {
 	bool rx_mcast_action_reg;
 
 	bool txqs_stopped[IEEE80211_NUM_ACS];
+	struct {
+		struct ieee80211_vif *parent;
+		struct list_head list;
+		bool non_transmitted;
+	} multiple_bssid;
 
 	bool cca_active;
 	u8 cca_color;
@@ -2326,7 +2334,7 @@  struct ieee80211_txq {
  * @IEEE80211_HW_TX_STATUS_NO_AMPDU_LEN: Driver does not report accurate A-MPDU
  *	length in tx status information
  *
- * @IEEE80211_HW_SUPPORTS_MULTI_BSSID: Hardware supports multi BSSID
+ * @IEEE80211_HW_SUPPORTS_MULTI_BSSID: Hardware supports multi BSSID in STA mode
  *
  * @IEEE80211_HW_SUPPORTS_ONLY_HE_MULTI_BSSID: Hardware supports multi BSSID
  *	only for HE APs. Applies if @IEEE80211_HW_SUPPORTS_MULTI_BSSID is set.
@@ -2335,6 +2343,8 @@  struct ieee80211_txq {
  *	aggregating MPDUs with the same keyid, allowing mac80211 to keep Tx
  *	A-MPDU sessions active while rekeying with Extended Key ID.
  *
+ * @IEEE80211_HW_SUPPORTS_MULTI_BSSID_AP: Hardware supports multi BSSID in AP mode
+ *
  * @NUM_IEEE80211_HW_FLAGS: number of hardware flags, used for sizing arrays
  */
 enum ieee80211_hw_flags {
@@ -2387,6 +2397,7 @@  enum ieee80211_hw_flags {
 	IEEE80211_HW_SUPPORTS_MULTI_BSSID,
 	IEEE80211_HW_SUPPORTS_ONLY_HE_MULTI_BSSID,
 	IEEE80211_HW_AMPDU_KEYBORDER_SUPPORT,
+	IEEE80211_HW_SUPPORTS_MULTI_BSSID_AP,
 
 	/* keep last, obviously */
 	NUM_IEEE80211_HW_FLAGS
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 37a218b89c9a..50a219d8a2cc 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -111,6 +111,36 @@  static int ieee80211_set_mon_options(struct ieee80211_sub_if_data *sdata,
 	return 0;
 }
 
+static int ieee80211_set_multiple_bssid_options(struct ieee80211_sub_if_data *sdata,
+						struct vif_params *params)
+{
+	struct ieee80211_local *local = sdata->local;
+	struct wiphy *wiphy = local->hw.wiphy;
+	struct net_device *parent;
+	struct ieee80211_sub_if_data *psdata;
+
+	if (!ieee80211_hw_check(&local->hw, SUPPORTS_MULTI_BSSID_AP))
+		return 0;
+
+	if (params->multiple_bssid.non_transmitted) {
+		parent = __dev_get_by_index(wiphy_net(wiphy),
+					    params->multiple_bssid.parent);
+		if (!parent || !parent->ieee80211_ptr)
+			return -EINVAL;
+		psdata = IEEE80211_WDEV_TO_SUB_IF(parent->ieee80211_ptr);
+		if (psdata->vif.multiple_bssid.non_transmitted)
+			return -EINVAL;
+		sdata->vif.multiple_bssid.parent = &psdata->vif;
+		list_add(&sdata->vif.multiple_bssid.list,
+			 &psdata->vif.multiple_bssid.list);
+		sdata->vif.multiple_bssid.non_transmitted = true;
+	} else {
+		INIT_LIST_HEAD(&sdata->vif.multiple_bssid.list);
+	}
+
+	return 0;
+}
+
 static struct wireless_dev *ieee80211_add_iface(struct wiphy *wiphy,
 						const char *name,
 						unsigned char name_assign_type,
@@ -136,11 +166,35 @@  static struct wireless_dev *ieee80211_add_iface(struct wiphy *wiphy,
 		}
 	}
 
+	if (type == NL80211_IFTYPE_AP) {
+		err = ieee80211_set_multiple_bssid_options(sdata, params);
+		if (err) {
+			ieee80211_if_remove(sdata);
+			return NULL;
+		}
+	}
+
 	return wdev;
 }
 
 static int ieee80211_del_iface(struct wiphy *wiphy, struct wireless_dev *wdev)
 {
+	struct ieee80211_sub_if_data *sdata;
+	struct ieee80211_vif *child, *tmp;
+
+	sdata = IEEE80211_WDEV_TO_SUB_IF(wdev);
+	if (sdata->vif.type == NL80211_IFTYPE_AP) {
+		if (!sdata->vif.multiple_bssid.non_transmitted) {
+			if (!list_empty(&sdata->vif.multiple_bssid.list))
+				list_for_each_entry_safe(child, tmp,
+							 &sdata->vif.multiple_bssid.list,
+							 multiple_bssid.list)
+					dev_close(vif_to_sdata(child)->wdev.netdev);
+		} else {
+			list_del(&sdata->vif.multiple_bssid.list);
+		}
+	}
+
 	ieee80211_if_remove(IEEE80211_WDEV_TO_SUB_IF(wdev));
 
 	return 0;
diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c
index 54080290d6e2..5d5c9185755a 100644
--- a/net/mac80211/debugfs.c
+++ b/net/mac80211/debugfs.c
@@ -408,6 +408,7 @@  static const char *hw_flag_names[] = {
 	FLAG(SUPPORTS_MULTI_BSSID),
 	FLAG(SUPPORTS_ONLY_HE_MULTI_BSSID),
 	FLAG(AMPDU_KEYBORDER_SUPPORT),
+	FLAG(SUPPORTS_MULTI_BSSID_AP),
 #undef FLAG
 };
 
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 8060cdc102d4..e6397d0c788d 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -810,6 +810,13 @@  static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
 	bool cancel_scan;
 	struct cfg80211_nan_func *func;
 
+	if (sdata->vif.type == NL80211_IFTYPE_AP &&
+	    sdata->vif.multiple_bssid.non_transmitted)
+		/* make sure the parent is already down */
+		if (sdata->vif.multiple_bssid.parent &&
+		    ieee80211_sdata_running(vif_to_sdata(sdata->vif.multiple_bssid.parent)))
+			dev_close(vif_to_sdata(sdata->vif.multiple_bssid.parent)->wdev.netdev);
+
 	clear_bit(SDATA_STATE_RUNNING, &sdata->state);
 
 	cancel_scan = rcu_access_pointer(local->scan_sdata) == sdata;