diff mbox

[3/4] batman-adv: Handle parent interfaces in a different netns

Message ID 1453312110-32683-4-git-send-email-andrew@lunn.ch (mailing list archive)
State Superseded, archived
Delegated to: Marek Lindner
Headers

Commit Message

Andrew Lunn Jan. 20, 2016, 5:48 p.m. UTC 
  batman-adv tries to prevent the user from placing a batX soft
interface into another batman mesh as a hard interface. It does this
by walking up the devices list of parents and ensures they are all
none batX interfaces. iflink can point to an interface in a different
nameplace, so also retrieve the parents name space when finding the
parent.

Additionally, veth devices come in pairs and they are each others
parents. Don't infinitely recurse in this situation.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 net/batman-adv/hard-interface.c | 44 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 42 insertions(+), 2 deletions(-)

Comments

Antonio Quartulli Jan. 26, 2016, 12:53 p.m. UTC | #1 
On Wed, Jan 20, 2016 at 06:48:29PM +0100, Andrew Lunn wrote:
>  /**
> + * batadv_mutual_parents - check if two devices are each others parent
> + * @net_dev - 1st parent
> + * @parent_dev - 2nd parent
> + *
> + * veth devices come in pairs and each is the parent of the other!
> + *
> + * Return true if the devices are each others parent, otherwise false

Andrew, minor style issue: you should put the ':' after the word Return.
Please check the other comments in this file as example. Sven recently changed
all the comments to be coherent with this style.

Other than this, the patch looks good to me.
You can add my

Acked-by: Antonio Quartulli <a@untable.cc>
Sven Eckelmann Jan. 26, 2016, 1:25 p.m. UTC | #2 
On Tuesday 26 January 2016 20:53:14 Antonio Quartulli wrote:
> On Wed, Jan 20, 2016 at 06:48:29PM +0100, Andrew Lunn wrote:
> >  /**
> > + * batadv_mutual_parents - check if two devices are each others parent
> > + * @net_dev - 1st parent
> > + * @parent_dev - 2nd parent
> > + *
> > + * veth devices come in pairs and each is the parent of the other!
> > + *
> > + * Return true if the devices are each others parent, otherwise false
> 
> Andrew, minor style issue: you should put the ':' after the word Return.
> Please check the other comments in this file as example. Sven recently changed
> all the comments to be coherent with this style.

It is not about being coherent but about kernel-doc being able to parse it.

Kind regards,
	Sven
Sven Eckelmann Jan. 27, 2016, 10:13 a.m. UTC | #3 
On Wednesday 20 January 2016 18:48:29 Andrew Lunn wrote:
> nameplace, so also retrieve the parents name space when finding the

s/nameplace/namespace/ ?

> Additionally, veth devices come in pairs and they are each others
> parents. Don't infinitely recurse in this situation.

Can we have this part as extra patch so Antonio can ask David for a stable 
backport?

Thanks,
	Sven
Antonio Quartulli Jan. 27, 2016, 10:44 a.m. UTC | #4 
On Wed, Jan 27, 2016 at 11:13:53AM +0100, Sven Eckelmann wrote:
> > Additionally, veth devices come in pairs and they are each others
> > parents. Don't infinitely recurse in this situation.
> 
> Can we have this part as extra patch so Antonio can ask David for a stable 
> backport?

good idea!
Sven Eckelmann Jan. 31, 2016, 1:26 p.m. UTC | #5 
On Wednesday 20 January 2016 18:48:29 Andrew Lunn wrote:
> batman-adv tries to prevent the user from placing a batX soft
> interface into another batman mesh as a hard interface. It does this
> by walking up the devices list of parents and ensures they are all
> none batX interfaces. iflink can point to an interface in a different
> nameplace, so also retrieve the parents name space when finding the
> parent.
> 
> Additionally, veth devices come in pairs and they are each others
> parents. Don't infinitely recurse in this situation.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  net/batman-adv/hard-interface.c | 44
> +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 42 insertions(+),
> 2 deletions(-)
> 
> diff --git a/net/batman-adv/hard-interface.c
> b/net/batman-adv/hard-interface.c index a5ce58a..8d51a4a 100644
> --- a/net/batman-adv/hard-interface.c
> +++ b/net/batman-adv/hard-interface.c

Include missing:

#include <net/rtnetlink.h>

> @@ -75,6 +75,39 @@ out:
>  }
> 
>  /**
> + * batadv_mutual_parents - check if two devices are each others parent
> + * @net_dev - 1st parent
> + * @parent_dev - 2nd parent
> + *
> + * veth devices come in pairs and each is the parent of the other!
> + *
> + * Return true if the devices are each others parent, otherwise false
> + */
> +static bool batadv_mutual_parents(const struct net_device *dev1,
> +				  const struct net *dev1_net,
> +				  const struct net_device *dev2,
> +				  const struct net *dev2_net)


 * What is net_dev and parent_dev?
 * Missing parameter docs are:
   - dev1
   - dev1_net
   - dev2
   - dev2_net.
 * Return -> Return:

Kind regards,
	Sven
Antonio Quartulli Feb. 1, 2016, 2:57 a.m. UTC | #6 
On Wed, Jan 27, 2016 at 11:13:53AM +0100, Sven Eckelmann wrote:
> On Wednesday 20 January 2016 18:48:29 Andrew Lunn wrote:
> > nameplace, so also retrieve the parents name space when finding the
> 
> s/nameplace/namespace/ ?
> 
> > Additionally, veth devices come in pairs and they are each others
> > parents. Don't infinitely recurse in this situation.
> 
> Can we have this part as extra patch so Antonio can ask David for a stable 
> backport?

Andrew,

do you have any plan about resending this fix ?
Sven also sent a pair of fixes and it would be nice I could send everything in
on pull request.

Cheers,
Andrew Lunn Feb. 2, 2016, 2:11 a.m. UTC | #7 
On Mon, Feb 01, 2016 at 10:57:55AM +0800, Antonio Quartulli wrote:
> On Wed, Jan 27, 2016 at 11:13:53AM +0100, Sven Eckelmann wrote:
> > On Wednesday 20 January 2016 18:48:29 Andrew Lunn wrote:
> > > nameplace, so also retrieve the parents name space when finding the
> > 
> > s/nameplace/namespace/ ?
> > 
> > > Additionally, veth devices come in pairs and they are each others
> > > parents. Don't infinitely recurse in this situation.
> > 
> > Can we have this part as extra patch so Antonio can ask David for a stable 
> > backport?
> 
> Andrew,
> 
> do you have any plan about resending this fix ?
> Sven also sent a pair of fixes and it would be nice I could send everything in
> on pull request.

Sorry, been in the wilderness the last few days.

I will try to resend tomorrow. If not, it will be Thursday.  If you
want it sooner, feel free to fix it up yourself and add your own
Signed-off by, next to mine.

	   Andrew
diff mbox

Patch

diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index a5ce58a..8d51a4a 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -75,6 +75,39 @@  out:
 }
 
 /**
+ * batadv_mutual_parents - check if two devices are each others parent
+ * @net_dev - 1st parent
+ * @parent_dev - 2nd parent
+ *
+ * veth devices come in pairs and each is the parent of the other!
+ *
+ * Return true if the devices are each others parent, otherwise false
+ */
+static bool batadv_mutual_parents(const struct net_device *dev1,
+				  const struct net *dev1_net,
+				  const struct net_device *dev2,
+				  const struct net *dev2_net)
+{
+	int dev1_parent_iflink = dev_get_iflink(dev1);
+	int dev2_parent_iflink = dev_get_iflink(dev2);
+	const struct net *dev1_parent_net = dev1_net;
+	const struct net *dev2_parent_net = dev2_net;
+
+	if (dev1->rtnl_link_ops && dev1->rtnl_link_ops->get_link_net)
+		dev1_parent_net = dev1->rtnl_link_ops->get_link_net(dev1);
+	if (dev2->rtnl_link_ops && dev2->rtnl_link_ops->get_link_net)
+		dev2_parent_net = dev2->rtnl_link_ops->get_link_net(dev2);
+
+	if (!dev1_parent_iflink || !dev2_parent_iflink)
+		return false;
+
+	return (dev1_parent_iflink == dev2->ifindex) &&
+		(dev2_parent_iflink == dev1->ifindex) &&
+		net_eq(dev1_parent_net, dev2_net) &&
+		net_eq(dev2_parent_net, dev1_net);
+}
+
+/**
  * batadv_is_on_batman_iface - check if a device is a batman iface descendant
  * @net_dev: the device to check
  *
@@ -89,8 +122,9 @@  out:
  */
 static bool batadv_is_on_batman_iface(const struct net_device *net_dev)
 {
-	struct net_device *parent_dev;
 	struct net *net = dev_net(net_dev);
+	struct net_device *parent_dev;
+	struct net *parent_net = net;
 	bool ret;
 
 	/* check if this is a batman-adv mesh interface */
@@ -102,12 +136,18 @@  static bool batadv_is_on_batman_iface(const struct net_device *net_dev)
 	    dev_get_iflink(net_dev) == net_dev->ifindex)
 		return false;
 
+	if (net_dev->rtnl_link_ops && net_dev->rtnl_link_ops->get_link_net)
+		parent_net = net_dev->rtnl_link_ops->get_link_net(net_dev);
+
 	/* recurse over the parent device */
-	parent_dev = __dev_get_by_index(net, dev_get_iflink(net_dev));
+	parent_dev = __dev_get_by_index(parent_net, dev_get_iflink(net_dev));
 	/* if we got a NULL parent_dev there is something broken.. */
 	if (WARN(!parent_dev, "Cannot find parent device"))
 		return false;
 
+	if (batadv_mutual_parents(net_dev, net, parent_dev, parent_net))
+		return false;
+
 	ret = batadv_is_on_batman_iface(parent_dev);
 
 	return ret;