2016-05-03 07:00:41

by Martin Willi

[permalink] [raw]
Subject: [PATCH 0/2] wireless: Allow wiphy/hwsim management from user namespaces

This patch set enables user namespaces having CAP_NET_ADMIN to manage
wiphy devices and create/destroy hwsim radios.

The first patch allows a caller from a non-initial user namespace to run
privileged nl80211 phy/dev operations. The second patch enables hwsim
radio management over Netlink from such namespaces. Together, with these
patches an unprivileged test environment can create user/network
namespaces and set up abitrary simulated wireless networks.

Martin Willi (2):
nl80211: Allow privileged operations from user namespaces
mac80211_hwsim: Allow managing radios from non-initial namespaces

drivers/net/wireless/mac80211_hwsim.c | 88 +++++++++++++++++++-
net/wireless/nl80211.c | 150 +++++++++++++++++-----------------
2 files changed, 160 insertions(+), 78 deletions(-)

--
2.7.4


2016-05-04 08:33:23

by Martin Willi

[permalink] [raw]
Subject: Re: [PATCH 2/2] mac80211_hwsim: Allow managing radios from non-initial namespaces

 
> > +static __net_init int hwsim_init_net(struct net *net)
> > +{
> > + struct mac80211_hwsim_data *data;
> > + bool exists = true;
> > + int netgroup = 0;
> > +
> > + spin_lock_bh(&hwsim_radio_lock);
> > + while (exists) {
> > + exists = false;
> > + list_for_each_entry(data, &hwsim_radios, list) {
> > + if (netgroup == data->netgroup) {
> > + exists = true;
> > + netgroup++;
> > + break;
> > + }
> > + }
> > + }
> > + spin_unlock_bh(&hwsim_radio_lock);
> > +
> > + *(int *)net_generic(net, hwsim_net_id) = netgroup;
>
> This seems somewhat awkward. Why not just take the maximum of all the
> netgroup IDs + 1? We'd run out of memory and radio IDs long before
> netgroup IDs even that way

My intention was to reuse netgroups for the case many namespaces come
and go, but I agree that it is not optimal.

> consider a new netns that doesn't have any hwsim radios yet.
> Now you create *another* one, but it would get the same netgroup.

Correct, that is indeed broken if there are no radios.

> IOW, you should simply use a global counter. Surprising (net)
> namespaces don't have an index like that already, but I don't see
> one.

Ok, will do that in a v2.

> > +static void __net_exit hwsim_exit_net(struct net *net)
> > +{
> > + struct mac80211_hwsim_data *entry, *tmp;
> > +
> > + spin_lock_bh(&hwsim_radio_lock);
> > + list_for_each_entry_safe(entry, tmp, &hwsim_radios, list) {
> > + if (net_eq(wiphy_net(entry->hw->wiphy), net)) {
> > + list_del(&entry->list);
> > + INIT_WORK(&entry->destroy_work, destroy_radio);
> > + schedule_work(&entry->destroy_work);
> > + }
> > + }
> > + spin_unlock_bh(&hwsim_radio_lock);
> > +}

> This changes today's default behaviour of moving the wiphys to the
> default namespace. Did you intend to destroy them based on the
> netgroup, i.e. based on the namespace that created them? Actually,
> maybe they should move back to the namespace that created them, if
> the namespace they are in is destroyed? But that's difficult, I don't
> mind this behaviour, but I'm not sure it's what we want by default
> for radios created in the init_net.

With the proposed approach I destroy all radios if the owning namespace
gets deleted, because we probably don't want them landing in init_net
if they are created from a (unprivileged) userns process. I think this
is what other "virtual" interfaces do (gre tunnels, veth etc.). If we
think of hwsim radios as such a "virtual" device, that makes IMO sense
to delete them.

If we want to keep the existing behavior, we could move radios
belonging to the init_net-associated netgroup back to init_net, that
shouldn't be too difficult.

Moving the radio back to the creators namespace would be the most
consistent behavior, so I'll check how difficult such a reverse lookup
is. We then would delete the radio only if it is in the creators
namespace, or if the creators namespace is gone. Does that make sense?

Martin

2016-05-09 07:31:33

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 2/2] mac80211_hwsim: Allow managing radios from non-initial namespaces


> >
> > + data->netgroup = *(int *)net_generic(net, hwsim_net_id);
> Anything doing *(integer_type *) rings alarm bells.
>
> I suspect you should be defining a structure that currently contains
> one integer member.
> Something (maybe a compile time assert) needs to check that buffer
> space you are accessing (where ever it is) is large enough.
>

It does look a bit awkward, but there's no value in having a struct -
you still have an opaque pointer here and cast it to something whose
size you assume to be present... it really makes no difference.

johannes

2016-05-03 07:00:42

by Martin Willi

[permalink] [raw]
Subject: [PATCH 1/2] nl80211: Allow privileged operations from user namespaces

While a wiphy can be transferred to network namespaces, a process having
CAP_NET_ADMIN in a non-initial user namespace can not administrate such
devices due to the genetlink GENL_ADMIN_PERM restrictions.

For openvswitch having the same issue, a new GENL_UNS_ADMIN_PERM flag has
been introduced, commit 4a92602aa1cd ("openvswitch: allow management from
inside user namespaces"). This patch changes all privileged operations
operating on a wiphy, dev or wdev to allow their administration using the
same mechanism. All operations use either NEED_WIPHY, NEED_WDEV or
NEED_NETDEV, which implies a namespace aware lookup of the device. The only
exception is NL80211_CMD_SET_WIPHY, which explicitly uses a namespace aware
lookup.

Signed-off-by: Martin Willi <[email protected]>
---
net/wireless/nl80211.c | 150 ++++++++++++++++++++++++-------------------------
1 file changed, 75 insertions(+), 75 deletions(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 9bc84a2..df4c897 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -10945,7 +10945,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_SET_WIPHY,
.doit = nl80211_set_wiphy,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_RTNL,
},
{
@@ -10961,7 +10961,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_SET_INTERFACE,
.doit = nl80211_set_interface,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV |
NL80211_FLAG_NEED_RTNL,
},
@@ -10969,7 +10969,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_NEW_INTERFACE,
.doit = nl80211_new_interface,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_WIPHY |
NL80211_FLAG_NEED_RTNL,
},
@@ -10977,7 +10977,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_DEL_INTERFACE,
.doit = nl80211_del_interface,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_WDEV |
NL80211_FLAG_NEED_RTNL,
},
@@ -10985,7 +10985,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_GET_KEY,
.doit = nl80211_get_key,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -10993,7 +10993,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_SET_KEY,
.doit = nl80211_set_key,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL |
NL80211_FLAG_CLEAR_SKB,
@@ -11002,7 +11002,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_NEW_KEY,
.doit = nl80211_new_key,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL |
NL80211_FLAG_CLEAR_SKB,
@@ -11011,14 +11011,14 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_DEL_KEY,
.doit = nl80211_del_key,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
{
.cmd = NL80211_CMD_SET_BEACON,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.doit = nl80211_set_beacon,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
@@ -11026,7 +11026,7 @@ static const struct genl_ops nl80211_ops[] = {
{
.cmd = NL80211_CMD_START_AP,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.doit = nl80211_start_ap,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
@@ -11034,7 +11034,7 @@ static const struct genl_ops nl80211_ops[] = {
{
.cmd = NL80211_CMD_STOP_AP,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.doit = nl80211_stop_ap,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
@@ -11051,7 +11051,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_SET_STATION,
.doit = nl80211_set_station,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11059,7 +11059,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_NEW_STATION,
.doit = nl80211_new_station,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11067,7 +11067,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_DEL_STATION,
.doit = nl80211_del_station,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11076,7 +11076,7 @@ static const struct genl_ops nl80211_ops[] = {
.doit = nl80211_get_mpath,
.dumpit = nl80211_dump_mpath,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11085,7 +11085,7 @@ static const struct genl_ops nl80211_ops[] = {
.doit = nl80211_get_mpp,
.dumpit = nl80211_dump_mpp,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11093,7 +11093,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_SET_MPATH,
.doit = nl80211_set_mpath,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11101,7 +11101,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_NEW_MPATH,
.doit = nl80211_new_mpath,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11109,7 +11109,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_DEL_MPATH,
.doit = nl80211_del_mpath,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11117,7 +11117,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_SET_BSS,
.doit = nl80211_set_bss,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11156,7 +11156,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_SET_MESH_CONFIG,
.doit = nl80211_update_mesh_config,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11164,7 +11164,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_TRIGGER_SCAN,
.doit = nl80211_trigger_scan,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_WDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11185,7 +11185,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_START_SCHED_SCAN,
.doit = nl80211_start_sched_scan,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11193,7 +11193,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_STOP_SCHED_SCAN,
.doit = nl80211_stop_sched_scan,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11201,7 +11201,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_AUTHENTICATE,
.doit = nl80211_authenticate,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL |
NL80211_FLAG_CLEAR_SKB,
@@ -11210,7 +11210,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_ASSOCIATE,
.doit = nl80211_associate,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11218,7 +11218,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_DEAUTHENTICATE,
.doit = nl80211_deauthenticate,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11226,7 +11226,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_DISASSOCIATE,
.doit = nl80211_disassociate,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11234,7 +11234,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_JOIN_IBSS,
.doit = nl80211_join_ibss,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11242,7 +11242,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_LEAVE_IBSS,
.doit = nl80211_leave_ibss,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11252,7 +11252,7 @@ static const struct genl_ops nl80211_ops[] = {
.doit = nl80211_testmode_do,
.dumpit = nl80211_testmode_dump,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_WIPHY |
NL80211_FLAG_NEED_RTNL,
},
@@ -11261,7 +11261,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_CONNECT,
.doit = nl80211_connect,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11269,7 +11269,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_DISCONNECT,
.doit = nl80211_disconnect,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11277,7 +11277,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_SET_WIPHY_NETNS,
.doit = nl80211_wiphy_netns,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_WIPHY |
NL80211_FLAG_NEED_RTNL,
},
@@ -11290,7 +11290,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_SET_PMKSA,
.doit = nl80211_setdel_pmksa,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11298,7 +11298,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_DEL_PMKSA,
.doit = nl80211_setdel_pmksa,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11306,7 +11306,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_FLUSH_PMKSA,
.doit = nl80211_flush_pmksa,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11314,7 +11314,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_REMAIN_ON_CHANNEL,
.doit = nl80211_remain_on_channel,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_WDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11322,7 +11322,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_CANCEL_REMAIN_ON_CHANNEL,
.doit = nl80211_cancel_remain_on_channel,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_WDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11330,7 +11330,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_SET_TX_BITRATE_MASK,
.doit = nl80211_set_tx_bitrate_mask,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV |
NL80211_FLAG_NEED_RTNL,
},
@@ -11338,7 +11338,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_REGISTER_FRAME,
.doit = nl80211_register_mgmt,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_WDEV |
NL80211_FLAG_NEED_RTNL,
},
@@ -11346,7 +11346,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_FRAME,
.doit = nl80211_tx_mgmt,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_WDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11354,7 +11354,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_FRAME_WAIT_CANCEL,
.doit = nl80211_tx_mgmt_cancel_wait,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_WDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11362,7 +11362,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_SET_POWER_SAVE,
.doit = nl80211_set_power_save,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV |
NL80211_FLAG_NEED_RTNL,
},
@@ -11378,7 +11378,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_SET_CQM,
.doit = nl80211_set_cqm,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV |
NL80211_FLAG_NEED_RTNL,
},
@@ -11386,7 +11386,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_SET_CHANNEL,
.doit = nl80211_set_channel,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV |
NL80211_FLAG_NEED_RTNL,
},
@@ -11394,7 +11394,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_SET_WDS_PEER,
.doit = nl80211_set_wds_peer,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV |
NL80211_FLAG_NEED_RTNL,
},
@@ -11402,7 +11402,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_JOIN_MESH,
.doit = nl80211_join_mesh,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11410,7 +11410,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_LEAVE_MESH,
.doit = nl80211_leave_mesh,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11418,7 +11418,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_JOIN_OCB,
.doit = nl80211_join_ocb,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11426,7 +11426,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_LEAVE_OCB,
.doit = nl80211_leave_ocb,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11443,7 +11443,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_SET_WOWLAN,
.doit = nl80211_set_wowlan,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_WIPHY |
NL80211_FLAG_NEED_RTNL,
},
@@ -11452,7 +11452,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_SET_REKEY_OFFLOAD,
.doit = nl80211_set_rekey_data,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL |
NL80211_FLAG_CLEAR_SKB,
@@ -11461,7 +11461,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_TDLS_MGMT,
.doit = nl80211_tdls_mgmt,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11469,7 +11469,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_TDLS_OPER,
.doit = nl80211_tdls_oper,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11477,7 +11477,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_UNEXPECTED_FRAME,
.doit = nl80211_register_unexpected_frame,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV |
NL80211_FLAG_NEED_RTNL,
},
@@ -11485,7 +11485,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_PROBE_CLIENT,
.doit = nl80211_probe_client,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11493,7 +11493,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_REGISTER_BEACONS,
.doit = nl80211_register_beacons,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_WIPHY |
NL80211_FLAG_NEED_RTNL,
},
@@ -11501,7 +11501,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_SET_NOACK_MAP,
.doit = nl80211_set_noack_map,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV |
NL80211_FLAG_NEED_RTNL,
},
@@ -11509,7 +11509,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_START_P2P_DEVICE,
.doit = nl80211_start_p2p_device,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_WDEV |
NL80211_FLAG_NEED_RTNL,
},
@@ -11517,7 +11517,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_STOP_P2P_DEVICE,
.doit = nl80211_stop_p2p_device,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_WDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11525,7 +11525,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_SET_MCAST_RATE,
.doit = nl80211_set_mcast_rate,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV |
NL80211_FLAG_NEED_RTNL,
},
@@ -11533,7 +11533,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_SET_MAC_ACL,
.doit = nl80211_set_mac_acl,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV |
NL80211_FLAG_NEED_RTNL,
},
@@ -11541,7 +11541,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_RADAR_DETECT,
.doit = nl80211_start_radar_detection,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11554,7 +11554,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_UPDATE_FT_IES,
.doit = nl80211_update_ft_ies,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11562,7 +11562,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_CRIT_PROTOCOL_START,
.doit = nl80211_crit_protocol_start,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_WDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11570,7 +11570,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_CRIT_PROTOCOL_STOP,
.doit = nl80211_crit_protocol_stop,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_WDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11585,7 +11585,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_SET_COALESCE,
.doit = nl80211_set_coalesce,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_WIPHY |
NL80211_FLAG_NEED_RTNL,
},
@@ -11593,7 +11593,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_CHANNEL_SWITCH,
.doit = nl80211_channel_switch,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11602,7 +11602,7 @@ static const struct genl_ops nl80211_ops[] = {
.doit = nl80211_vendor_cmd,
.dumpit = nl80211_vendor_cmd_dump,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_WIPHY |
NL80211_FLAG_NEED_RTNL,
},
@@ -11610,7 +11610,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_SET_QOS_MAP,
.doit = nl80211_set_qos_map,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11618,7 +11618,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_ADD_TX_TS,
.doit = nl80211_add_tx_ts,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11626,7 +11626,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_DEL_TX_TS,
.doit = nl80211_del_tx_ts,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11634,7 +11634,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_TDLS_CHANNEL_SWITCH,
.doit = nl80211_tdls_channel_switch,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
@@ -11642,7 +11642,7 @@ static const struct genl_ops nl80211_ops[] = {
.cmd = NL80211_CMD_TDLS_CANCEL_CHANNEL_SWITCH,
.doit = nl80211_tdls_cancel_channel_switch,
.policy = nl80211_policy,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
--
2.7.4


2016-05-09 07:30:49

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 2/2] mac80211_hwsim: Allow managing radios from non-initial namespaces

On Wed, 2016-05-04 at 10:33 +0200, Martin Willi wrote:

> > This changes today's default behaviour of moving the wiphys to the
> > default namespace. Did you intend to destroy them based on the
> > netgroup, i.e. based on the namespace that created them? Actually,
> > maybe they should move back to the namespace that created them, if
> > the namespace they are in is destroyed? But that's difficult, I
> > don't
> > mind this behaviour, but I'm not sure it's what we want by default
> > for radios created in the init_net.
> With the proposed approach I destroy all radios if the owning
> namespace gets deleted, because we probably don't want them landing
> in init_net if they are created from a (unprivileged) userns process.

I agree they shouldn't land in init_net.

> I think this is what other "virtual" interfaces do (gre tunnels, veth
> etc.). If we think of hwsim radios as such a "virtual" device, that
> makes IMO sense to delete them.

Ok, I have no idea what happens there.

> If we want to keep the existing behavior, we could move radios
> belonging to the init_net-associated netgroup back to init_net, that
> shouldn't be too difficult.
>
> Moving the radio back to the creators namespace would be the most
> consistent behavior, so I'll check how difficult such a reverse
> lookup is. We then would delete the radio only if it is in the
> creators namespace, or if the creators namespace is gone. Does that
> make sense?

It does make sense, but it does also feel a bit complicated. Perhaps
just special-case the init_net case for consistency with the existing
behaviour, and reserve netgroup 0 for that so we can easily check for
it?

johannes

2016-05-03 06:56:34

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 2/2] mac80211_hwsim: Allow managing radios from non-initial namespaces

On Tue, 2016-05-03 at 08:53 +0200, Martin Willi wrote:
> While wiphys can be moved into network namespaces over nl80211, the
> creation and removal of hwsim radios is currently limited to the
> initial namespace. This patch allows management of namespaced radios
> from the owning namespace by setting genetlink netnsok.

Interesting.

> To prevent two arbitrary namespaces from communicating over the
> simulated
> shared medium, radios are separated by netgroups. Each radio created
> in
> the same namespace lives in the same netgroup and hence can
> communicate
> with other radios in that group. When moving radios to other
> namespaces,
> the netgroup is preserved, so two radios having the same netgroup can
> communicate even if not in the same namespace; This allows a
> controlling
> namespace to create radios and move them to other namespaces for
> communication.

Neat.

I'm curious what the use case is?

I'll need some time to review it, it'll likely have to wait until next
week given our holiday here on Thursday (and I'm also off work
tomorrow/Friday)

johannes


2016-05-05 11:03:00

by David Laight

[permalink] [raw]
Subject: RE: [PATCH 2/2] mac80211_hwsim: Allow managing radios from non-initial namespaces

From: Martin Willi
> Sent: 03 May 2016 07:53
> While wiphys can be moved into network namespaces over nl80211, the
> creation and removal of hwsim radios is currently limited to the initial
> namespace. This patch allows management of namespaced radios from the
> owning namespace by setting genetlink netnsok.
>
> To prevent two arbitrary namespaces from communicating over the simulated
> shared medium, radios are separated by netgroups. Each radio created in
> the same namespace lives in the same netgroup and hence can communicate
> with other radios in that group. When moving radios to other namespaces,
> the netgroup is preserved, so two radios having the same netgroup can
> communicate even if not in the same namespace; This allows a controlling
> namespace to create radios and move them to other namespaces for
> communication.
>
...
> + data->netgroup = *(int *)net_generic(net, hwsim_net_id);

Anything doing *(integer_type *) rings alarm bells.

I suspect you should be defining a structure that currently contains
one integer member.
Something (maybe a compile time assert) needs to check that buffer
space you are accessing (where ever it is) is large enough.

David


2016-05-03 19:16:36

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 2/2] mac80211_hwsim: Allow managing radios from non-initial namespaces

On Tue, 2016-05-03 at 08:53 +0200, Martin Willi wrote:

> +static __net_init int hwsim_init_net(struct net *net)
> +{
> + struct mac80211_hwsim_data *data;
> + bool exists = true;
> + int netgroup = 0;
> +
> + spin_lock_bh(&hwsim_radio_lock);
> + while (exists) {
> + exists = false;
> + list_for_each_entry(data, &hwsim_radios, list) {
> + if (netgroup == data->netgroup) {
> + exists = true;
> + netgroup++;
> + break;
> + }
> + }
> + }
> + spin_unlock_bh(&hwsim_radio_lock);
> +
> + *(int *)net_generic(net, hwsim_net_id) = netgroup;


This seems somewhat awkward. Why not just take the maximum of all the
netgroup IDs + 1? We'd run out of memory and radio IDs long before
netgroup IDs even that way, and they're not actually visible anywhere
so it doesn't matter.

Actually though, *both* your approach and my suggestion don't seem
safe: consider a new netns that doesn't have any hwsim radios yet. Now
you create *another* one, but it would get the same netgroup.

IOW, you should simply use a global counter. Surprising (net)
namespaces don't have an index like that already, but I don't see one.

> +static void __net_exit hwsim_exit_net(struct net *net)
> +{
> + struct mac80211_hwsim_data *entry, *tmp;
> +
> + spin_lock_bh(&hwsim_radio_lock);
> + list_for_each_entry_safe(entry, tmp, &hwsim_radios, list) {
> + if (net_eq(wiphy_net(entry->hw->wiphy), net)) {
> + list_del(&entry->list);
> + INIT_WORK(&entry->destroy_work,
> destroy_radio);
> + schedule_work(&entry->destroy_work);
> + }
> + }
> + spin_unlock_bh(&hwsim_radio_lock);
> +}

This changes today's default behaviour of moving the wiphys to the
default namespace. Did you intend to destroy them based on the
netgroup, i.e. based on the namespace that created them? Actually,
maybe they should move back to the namespace that created them, if the
namespace they are in is destroyed? But that's difficult, I don't mind
this behaviour, but I'm not sure it's what we want by default for
radios created in the init_net.

johannes

2016-05-03 07:00:41

by Martin Willi

[permalink] [raw]
Subject: [PATCH 2/2] mac80211_hwsim: Allow managing radios from non-initial namespaces

While wiphys can be moved into network namespaces over nl80211, the
creation and removal of hwsim radios is currently limited to the initial
namespace. This patch allows management of namespaced radios from the
owning namespace by setting genetlink netnsok.

To prevent two arbitrary namespaces from communicating over the simulated
shared medium, radios are separated by netgroups. Each radio created in
the same namespace lives in the same netgroup and hence can communicate
with other radios in that group. When moving radios to other namespaces,
the netgroup is preserved, so two radios having the same netgroup can
communicate even if not in the same namespace; This allows a controlling
namespace to create radios and move them to other namespaces for
communication.

Signed-off-by: Martin Willi <[email protected]>
---
drivers/net/wireless/mac80211_hwsim.c | 88 +++++++++++++++++++++++++++++++++--
1 file changed, 85 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index c757f14..91bd440 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -30,6 +30,8 @@
#include <linux/module.h>
#include <linux/ktime.h>
#include <net/genetlink.h>
+#include <net/net_namespace.h>
+#include <net/netns/generic.h>
#include "mac80211_hwsim.h"

#define WARN_QUEUE 100
@@ -39,6 +41,7 @@ MODULE_AUTHOR("Jouni Malinen");
MODULE_DESCRIPTION("Software simulator of 802.11 radio(s) for mac80211");
MODULE_LICENSE("GPL");

+static unsigned int hwsim_net_id;
static u32 wmediumd_portid;

static int radios = 2;
@@ -526,6 +529,9 @@ struct mac80211_hwsim_data {
*/
u64 group;

+ /* group shared by radios created in the same netns */
+ int netgroup;
+
int power_level;

/* difference between this hw's clock and the real clock, in usecs */
@@ -568,6 +574,7 @@ static struct genl_family hwsim_genl_family = {
.name = "MAC80211_HWSIM",
.version = 1,
.maxattr = HWSIM_ATTR_MAX,
+ .netnsok = true,
};

enum hwsim_multicast_groups {
@@ -1202,6 +1209,9 @@ static bool mac80211_hwsim_tx_frame_no_nl(struct ieee80211_hw *hw,
if (!(data->group & data2->group))
continue;

+ if (data->netgroup != data2->netgroup)
+ continue;
+
if (!hwsim_chans_compat(chan, data2->tmp_chan) &&
!hwsim_chans_compat(chan, data2->channel)) {
ieee80211_iterate_active_interfaces_atomic(
@@ -2348,6 +2358,7 @@ static int mac80211_hwsim_new_radio(struct genl_info *info,
struct mac80211_hwsim_data *data;
struct ieee80211_hw *hw;
enum nl80211_band band;
+ struct net *net;
const struct ieee80211_ops *ops = &mac80211_hwsim_ops;
int idx;

@@ -2366,6 +2377,13 @@ static int mac80211_hwsim_new_radio(struct genl_info *info,
err = -ENOMEM;
goto failed;
}
+
+ if (info)
+ net = genl_info_net(info);
+ else
+ net = &init_net;
+ wiphy_net_set(hw->wiphy, net);
+
data = hw->priv;
data->hw = hw;

@@ -2541,6 +2559,8 @@ static int mac80211_hwsim_new_radio(struct genl_info *info,
data->group = 1;
mutex_init(&data->mutex);

+ data->netgroup = *(int *)net_generic(net, hwsim_net_id);
+
/* Enable frame retransmissions for lossy channels */
hw->max_rates = 4;
hw->max_rate_tries = 11;
@@ -3013,6 +3033,9 @@ static int hwsim_del_radio_nl(struct sk_buff *msg, struct genl_info *info)
continue;
}

+ if (!net_eq(wiphy_net(data->hw->wiphy), genl_info_net(info)))
+ continue;
+
list_del(&data->list);
spin_unlock_bh(&hwsim_radio_lock);
mac80211_hwsim_del_radio(data, wiphy_name(data->hw->wiphy),
@@ -3039,6 +3062,9 @@ static int hwsim_get_radio_nl(struct sk_buff *msg, struct genl_info *info)
if (data->idx != idx)
continue;

+ if (!net_eq(wiphy_net(data->hw->wiphy), genl_info_net(info)))
+ continue;
+
skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
if (!skb) {
res = -ENOMEM;
@@ -3078,6 +3104,9 @@ static int hwsim_dump_radio_nl(struct sk_buff *skb,
if (data->idx < idx)
continue;

+ if (!net_eq(wiphy_net(data->hw->wiphy), sock_net(skb->sk)))
+ continue;
+
res = mac80211_hwsim_get_radio(skb, data,
NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq, cb,
@@ -3117,13 +3146,13 @@ static const struct genl_ops hwsim_ops[] = {
.cmd = HWSIM_CMD_NEW_RADIO,
.policy = hwsim_genl_policy,
.doit = hwsim_new_radio_nl,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
},
{
.cmd = HWSIM_CMD_DEL_RADIO,
.policy = hwsim_genl_policy,
.doit = hwsim_del_radio_nl,
- .flags = GENL_ADMIN_PERM,
+ .flags = GENL_UNS_ADMIN_PERM,
},
{
.cmd = HWSIM_CMD_GET_RADIO,
@@ -3205,6 +3234,52 @@ failure:
return -EINVAL;
}

+static __net_init int hwsim_init_net(struct net *net)
+{
+ struct mac80211_hwsim_data *data;
+ bool exists = true;
+ int netgroup = 0;
+
+ spin_lock_bh(&hwsim_radio_lock);
+ while (exists) {
+ exists = false;
+ list_for_each_entry(data, &hwsim_radios, list) {
+ if (netgroup == data->netgroup) {
+ exists = true;
+ netgroup++;
+ break;
+ }
+ }
+ }
+ spin_unlock_bh(&hwsim_radio_lock);
+
+ *(int *)net_generic(net, hwsim_net_id) = netgroup;
+
+ return 0;
+}
+
+static void __net_exit hwsim_exit_net(struct net *net)
+{
+ struct mac80211_hwsim_data *entry, *tmp;
+
+ spin_lock_bh(&hwsim_radio_lock);
+ list_for_each_entry_safe(entry, tmp, &hwsim_radios, list) {
+ if (net_eq(wiphy_net(entry->hw->wiphy), net)) {
+ list_del(&entry->list);
+ INIT_WORK(&entry->destroy_work, destroy_radio);
+ schedule_work(&entry->destroy_work);
+ }
+ }
+ spin_unlock_bh(&hwsim_radio_lock);
+}
+
+static struct pernet_operations hwsim_net_ops = {
+ .init = hwsim_init_net,
+ .exit = hwsim_exit_net,
+ .id = &hwsim_net_id,
+ .size = sizeof(int),
+};
+
static void hwsim_exit_netlink(void)
{
/* unregister the notifier */
@@ -3241,10 +3316,14 @@ static int __init init_mac80211_hwsim(void)
spin_lock_init(&hwsim_radio_lock);
INIT_LIST_HEAD(&hwsim_radios);

- err = platform_driver_register(&mac80211_hwsim_driver);
+ err = register_pernet_device(&hwsim_net_ops);
if (err)
return err;

+ err = platform_driver_register(&mac80211_hwsim_driver);
+ if (err)
+ goto out_unregister_pernet;
+
hwsim_class = class_create(THIS_MODULE, "mac80211_hwsim");
if (IS_ERR(hwsim_class)) {
err = PTR_ERR(hwsim_class);
@@ -3362,6 +3441,8 @@ out_free_radios:
mac80211_hwsim_free();
out_unregister_driver:
platform_driver_unregister(&mac80211_hwsim_driver);
+out_unregister_pernet:
+ unregister_pernet_device(&hwsim_net_ops);
return err;
}
module_init(init_mac80211_hwsim);
@@ -3375,5 +3456,6 @@ static void __exit exit_mac80211_hwsim(void)
mac80211_hwsim_free();
unregister_netdev(hwsim_mon);
platform_driver_unregister(&mac80211_hwsim_driver);
+ unregister_pernet_device(&hwsim_net_ops);
}
module_exit(exit_mac80211_hwsim);
--
2.7.4


2016-05-03 07:50:56

by Martin Willi

[permalink] [raw]
Subject: Re: [PATCH 2/2] mac80211_hwsim: Allow managing radios from non-initial namespaces


> > This allows a controlling namespace to create radios and move them
> > to other namespaces for communication.

> Neat.
>
> I'm curious what the use case is?

We use a test environment for integration and regression testing, which
allows us to run our networked applications in a simulated environment
using many hosts. Something like [1], but based on the full set of
(nested) Linux Namespaces.

This has been working great to simulate wired networks (using veth and
bridges). But as it runs unprivileged using user namespaces, we have
hit these permission issues when we tried to integrate support for
wireless radios using (the very cool) mac80211_hwsim. hwsim allows us
to simulate complex wireless networks using an unmodified userspace
software stack.

> I'll need some time to review it, it'll likely have to wait until
> next week given our holiday here on Thursday (and I'm also off work
> tomorrow/Friday)

Ok, great, looking forward to your feedback.

Best regardsMartin

[1]https://wiki.strongswan.org/projects/strongswan/wiki/TestingEnvironment

2016-05-09 16:33:08

by Martin Willi

[permalink] [raw]
Subject: Re: [PATCH 2/2] mac80211_hwsim: Allow managing radios from non-initial namespaces


> > Moving the radio back to the creators namespace would be the most
> > consistent behavior, so I'll check how difficult such a reverse
> > lookup is. We then would delete the radio only if it is in the
> > creators namespace, or if the creators namespace is gone. Does that
> > make sense?

> It does make sense, but it does also feel a bit complicated.

Yes, and proper locking gets difficult when using cfg80211_set_netns(),
which can sleep. This would probably need larger changes in the locking
code, but that's IMO not worth the effort.

> Perhaps just special-case the init_net case for consistency with the
> existing behaviour, and reserve netgroup 0 for that so we can easily
> check for it?

I'll do that in a v2, following immediately.

> > I suspect you should be defining a structure that currently
> > contains one integer member. Something (maybe a compile time
> > assert) needs to check that buffer space you are accessing (where
> > ever it is) is large enough.
>
> It does look a bit awkward, but there's no value in having a struct -
> you still have an opaque pointer here and cast it to something whose
> size you assume to be present... it really makes no difference.

I agree there is no added safety when using a struct; Nonetheless have
I added one in v2, and is it just for any future member additions.

Best regards
Martin