Date:   Thu, 28 Sep 2017 15:17:28 +0100
From:   Tom Parkin <tparkin@...alix.com>
To:     Guillaume Nault <g.nault@...halink.fr>
Cc:     netdev@...r.kernel.org, James Chapman <jchapman@...alix.com>
Subject: Re: [PATCH net] l2tp: fix l2tp_eth module loading

On Thu, Sep 28, 2017 at 03:44:38PM +0200, Guillaume Nault wrote:
> The l2tp_eth module crashes if its netlink callbacks are run when the
> pernet data aren't initialised.
> 
> We should normally register_pernet_device() before the genl callbacks.
> However, the pernet data only maintain a list of l2tpeth interfaces,
> and this list is never used. So let's just drop pernet handling
> instead.
> 
> Fixes: d9e31d17ceba ("l2tp: Add L2TP ethernet pseudowire support")
> Signed-off-by: Guillaume Nault <g.nault@...halink.fr>

Whoops.  I think this was intended to clear up the devices in the net
namespace, but since l2tp_core.c already deletes tunnels on namespace
exit I don't think it's necessary for l2tp_eth.c to do anything more.

> ---
>  net/l2tp/l2tp_eth.c | 51 ++-------------------------------------------------
>  1 file changed, 2 insertions(+), 49 deletions(-)
> 
> diff --git a/net/l2tp/l2tp_eth.c b/net/l2tp/l2tp_eth.c
> index 87da9ef61860..014a7bc2a872 100644
> --- a/net/l2tp/l2tp_eth.c
> +++ b/net/l2tp/l2tp_eth.c
> @@ -44,7 +44,6 @@ struct l2tp_eth {
>  	struct net_device	*dev;
>  	struct sock		*tunnel_sock;
>  	struct l2tp_session	*session;
> -	struct list_head	list;
>  	atomic_long_t		tx_bytes;
>  	atomic_long_t		tx_packets;
>  	atomic_long_t		tx_dropped;
> @@ -58,17 +57,6 @@ struct l2tp_eth_sess {
>  	struct net_device	*dev;
>  };
>  
> -/* per-net private data for this module */
> -static unsigned int l2tp_eth_net_id;
> -struct l2tp_eth_net {
> -	struct list_head l2tp_eth_dev_list;
> -	spinlock_t l2tp_eth_lock;
> -};
> -
> -static inline struct l2tp_eth_net *l2tp_eth_pernet(struct net *net)
> -{
> -	return net_generic(net, l2tp_eth_net_id);
> -}
>  
>  static int l2tp_eth_dev_init(struct net_device *dev)
>  {
> @@ -84,12 +72,6 @@ static int l2tp_eth_dev_init(struct net_device *dev)
>  
>  static void l2tp_eth_dev_uninit(struct net_device *dev)
>  {
> -	struct l2tp_eth *priv = netdev_priv(dev);
> -	struct l2tp_eth_net *pn = l2tp_eth_pernet(dev_net(dev));
> -
> -	spin_lock(&pn->l2tp_eth_lock);
> -	list_del_init(&priv->list);
> -	spin_unlock(&pn->l2tp_eth_lock);
>  	dev_put(dev);
>  }
>  
> @@ -273,7 +255,6 @@ static int l2tp_eth_create(struct net *net, struct l2tp_tunnel *tunnel,
>  	struct l2tp_eth *priv;
>  	struct l2tp_eth_sess *spriv;
>  	int rc;
> -	struct l2tp_eth_net *pn;
>  
>  	if (cfg->ifname) {
>  		strlcpy(name, cfg->ifname, IFNAMSIZ);
> @@ -305,7 +286,6 @@ static int l2tp_eth_create(struct net *net, struct l2tp_tunnel *tunnel,
>  	priv = netdev_priv(dev);
>  	priv->dev = dev;
>  	priv->session = session;
> -	INIT_LIST_HEAD(&priv->list);
>  
>  	priv->tunnel_sock = tunnel->sock;
>  	session->recv_skb = l2tp_eth_dev_recv;
> @@ -326,10 +306,6 @@ static int l2tp_eth_create(struct net *net, struct l2tp_tunnel *tunnel,
>  	strlcpy(session->ifname, dev->name, IFNAMSIZ);
>  
>  	dev_hold(dev);
> -	pn = l2tp_eth_pernet(dev_net(dev));
> -	spin_lock(&pn->l2tp_eth_lock);
> -	list_add(&priv->list, &pn->l2tp_eth_dev_list);
> -	spin_unlock(&pn->l2tp_eth_lock);
>  
>  	return 0;
>  
> @@ -342,22 +318,6 @@ static int l2tp_eth_create(struct net *net, struct l2tp_tunnel *tunnel,
>  	return rc;
>  }
>  
> -static __net_init int l2tp_eth_init_net(struct net *net)
> -{
> -	struct l2tp_eth_net *pn = net_generic(net, l2tp_eth_net_id);
> -
> -	INIT_LIST_HEAD(&pn->l2tp_eth_dev_list);
> -	spin_lock_init(&pn->l2tp_eth_lock);
> -
> -	return 0;
> -}
> -
> -static struct pernet_operations l2tp_eth_net_ops = {
> -	.init = l2tp_eth_init_net,
> -	.id   = &l2tp_eth_net_id,
> -	.size = sizeof(struct l2tp_eth_net),
> -};
> -
>  
>  static const struct l2tp_nl_cmd_ops l2tp_eth_nl_cmd_ops = {
>  	.session_create	= l2tp_eth_create,
> @@ -371,25 +331,18 @@ static int __init l2tp_eth_init(void)
>  
>  	err = l2tp_nl_register_ops(L2TP_PWTYPE_ETH, &l2tp_eth_nl_cmd_ops);
>  	if (err)
> -		goto out;
> -
> -	err = register_pernet_device(&l2tp_eth_net_ops);
> -	if (err)
> -		goto out_unreg;
> +		goto err;
>  
>  	pr_info("L2TP ethernet pseudowire support (L2TPv3)\n");
>  
>  	return 0;
>  
> -out_unreg:
> -	l2tp_nl_unregister_ops(L2TP_PWTYPE_ETH);
> -out:
> +err:
>  	return err;
>  }
>  
>  static void __exit l2tp_eth_exit(void)
>  {
> -	unregister_pernet_device(&l2tp_eth_net_ops);
>  	l2tp_nl_unregister_ops(L2TP_PWTYPE_ETH);
>  }
>  
> -- 
> 2.14.2
> 

-- 
Tom Parkin
Katalix Systems Ltd
http://www.katalix.com
Catalysts for your Embedded Linux software development

Download attachment "signature.asc" of type "application/pgp-signature" (474 bytes)

Powered by blists - more mailing lists