| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 13 Nov 2017 15:05:41 +0100
From: Jiri Pirko <jiri@...nulli.us>
To: netdev@...r.kernel.org
Cc: davem@...emloft.net, jhs@...atatu.com, xiyou.wangcong@...il.com,
mlxsw@...lanox.com, idosch@...lanox.com, edumazet@...gle.com,
willemb@...gle.com, me@...in.cc, john.fastabend@...il.com,
jakub.kicinski@...ronome.com, daniel@...earbox.net
Subject: [patch net] net: forbid netdev used by mirred tc act from being moved to another netns
From: Jiri Pirko <jiri@...lanox.com>
Currently, user may choose to move device that is used by mirred action
to another network namespace. That is wrong as the action still remains
in the original namespace and references non-existing ifindex.
See an example to illustrate this:
$ ip link add dummyx1 type dummy
$ ip link add dummyx2 type dummy
$ tc qdisc add dev dummyx1 clsact
$ tc filter add dev dummyx1 ingress protocol all matchall action mirred egress mirror dev dummyx2
$ tc -s filter show dev dummyx1 ingress
filter protocol all pref 49152 matchall
filter protocol all pref 49152 matchall handle 0x1
action order 1: mirred (Egress Mirror to device dummyx2) pipe
index 1 ref 1 bind 1 installed 16 sec used 16 sec
Action statistics:
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
$ ip netns add testx1
$ ip link set dummyx2 netns testx1
$ tc -s filter show dev dummyx1 ingress
filter protocol all pref 49152 matchall
filter protocol all pref 49152 matchall handle 0x1
action order 1: mirred (Egress Mirror to device if13) pipe
^^^^
index 1 ref 1 bind 1 installed 56 sec used 56 sec
Action statistics:
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
It may even happen that this ifindex is used by another totally
unrelated netdevice. So fix this by disallowing the netdevice used by
mirred action to move to another network namespace.
Fixes: ce286d327341 ("[NET]: Implement network device movement between namespaces")
Signed-off-by: Jiri Pirko <jiri@...lanox.com>
---
include/linux/netdevice.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
net/core/dev.c | 2 +-
net/sched/act_mirred.c | 8 ++++----
3 files changed, 49 insertions(+), 5 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 2eaac7d..35672e6 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1898,6 +1898,7 @@ struct net_device {
struct lock_class_key *qdisc_tx_busylock;
struct lock_class_key *qdisc_running_key;
bool proto_down;
+ unsigned int netns_blocker_count;
};
#define to_net_dev(d) container_of(d, struct net_device, dev)
@@ -3345,6 +3346,49 @@ static inline void dev_hold(struct net_device *dev)
this_cpu_inc(*dev->pcpu_refcnt);
}
+static inline void dev_netns_blocker_inc(struct net_device *dev)
+{
+ dev->netns_blocker_count++;
+}
+
+static inline void dev_netns_blocker_dec(struct net_device *dev)
+{
+ WARN_ON(dev->netns_blocker_count-- == 0);
+}
+
+/**
+ * dev_put - release reference to device, allow netns change
+ * @dev: network device
+ *
+ * Release reference to device to allow it to be freed.
+ * Also, release netns blocker reference and allow it
+ * to be moved to another network namespace.
+ */
+static inline void dev_put_netns(struct net_device *dev)
+{
+ dev_netns_blocker_dec(dev);
+ dev_put(dev);
+}
+
+/**
+ * dev_hold_netns - get reference to device, disallow netns change
+ * @dev: network device
+ *
+ * Hold reference to device to keep it from being freed.
+ * Also, hold netns blocker reference to keep it from
+ * being moved to another network namespace.
+ */
+static inline void dev_hold_netns(struct net_device *dev)
+{
+ dev_hold(dev);
+ dev_netns_blocker_inc(dev);
+}
+
+static inline bool dev_netns_blocked(struct net_device *dev)
+{
+ return dev->netns_blocker_count;
+}
+
/* Carrier loss detection, dial on demand. The functions netif_carrier_on
* and _off may be called from IRQ context, but it is caller
* who is responsible for serialization of these calls.
diff --git a/net/core/dev.c b/net/core/dev.c
index 11596a3..877979f 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -8250,7 +8250,7 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char
/* Don't allow namespace local devices to be moved. */
err = -EINVAL;
- if (dev->features & NETIF_F_NETNS_LOCAL)
+ if (dev->features & NETIF_F_NETNS_LOCAL || dev_netns_blocked(dev))
goto out;
/* Ensure the device has been registrered */
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 416627c..5a7b949 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -60,7 +60,7 @@ static void tcf_mirred_release(struct tc_action *a, int bind)
list_del(&m->tcfm_list);
dev = rcu_dereference_protected(m->tcfm_dev, 1);
if (dev)
- dev_put(dev);
+ dev_put_netns(dev);
spin_unlock_bh(&mirred_list_lock);
}
@@ -141,8 +141,8 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
if (dev != NULL) {
m->tcfm_ifindex = parm->ifindex;
if (ret != ACT_P_CREATED)
- dev_put(rcu_dereference_protected(m->tcfm_dev, 1));
- dev_hold(dev);
+ dev_put_netns(rcu_dereference_protected(m->tcfm_dev, 1));
+ dev_hold_netns(dev);
rcu_assign_pointer(m->tcfm_dev, dev);
m->tcfm_mac_header_xmit = mac_header_xmit;
}
@@ -296,7 +296,7 @@ static int mirred_device_event(struct notifier_block *unused,
spin_lock_bh(&mirred_list_lock);
list_for_each_entry(m, &mirred_list, tcfm_list) {
if (rcu_access_pointer(m->tcfm_dev) == dev) {
- dev_put(dev);
+ dev_put_netns(dev);
/* Note : no rcu grace period necessary, as
* net_device are already rcu protected.
*/
--
2.9.5
Powered by blists - more mailing lists