netdev - Re: [PATCH v2 10/12] l2tp: Add L2TP ethernet pseudowire support

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <4BB0989B.1040504@katalix.com>
Date:	Mon, 29 Mar 2010 13:10:03 +0100
From:	James Chapman <jchapman@...alix.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
CC:	netdev@...r.kernel.org
Subject: Re: [PATCH v2 10/12] l2tp: Add L2TP ethernet pseudowire support

Eric Dumazet wrote:
> Le lundi 29 mars 2010 à 10:57 +0100, James Chapman a écrit :
> 
>> +static struct l2tp_session *l2tp_eth_session_find_by_ifname(struct net *net, char *ifname)
>> +{
>> +	struct net_device *dev;
>> +
>> +	dev = dev_get_by_name(net, ifname);
>> +	if (dev) {
>> +		struct l2tp_eth *priv = netdev_priv(dev);
>> +		return priv->session;
>> +	}
>> +
>> +	return NULL;
>> +}
>> +
> 
> Not clear to me if  caller of l2tp_eth_session_find_by_ifname()
> is able to do dev_put(dev);
> 
> l2tp_eth_create() ->

Good catch. If a device name were specified by the caller when creating
the new session (usually the default name is used), a dev_put() is
needed, since the dev_get_by_name() call is only used to test if a
device with that name already exists. I'll roll a fix into the next
patch version.

>> +static int l2tp_eth_create(struct net *net, u32 tunnel_id, u32 session_id, u32 peer_session_id, struct l2tp_session_cfg *cfg)
>> +{
>> +	struct net_device *dev;
>> +	char name[IFNAMSIZ];
>> +	struct l2tp_tunnel *tunnel;
>> +	struct l2tp_session *session;
>> +	struct l2tp_eth *priv;
>> +	struct l2tp_eth_sess *spriv;
>> +	int rc;
>> +	struct l2tp_eth_net *pn;
>> +
>> +	tunnel = l2tp_tunnel_find(net, tunnel_id);
>> +	if (!tunnel) {
>> +		rc = -ENODEV;
>> +		goto out;
>> +	}
>> +
>> +	session = l2tp_session_find(net, tunnel, session_id);
>> +	if (session) {
>> +		rc = -EEXIST;
>> +		goto out;
>> +	}
>> +
>> +	if (cfg->ifname) {
>> +		session = l2tp_eth_session_find_by_ifname(net, cfg->ifname);
>> +		if (session) {
>> +			rc = -EEXIST;
>> +			goto out;
>> +		}
>> +		strlcpy(name, cfg->ifname, IFNAMSIZ);
>> +	} else
>> +		strcpy(name, L2TP_ETH_DEV_NAME);
>> +
>> +	session = l2tp_session_create(sizeof(*spriv), tunnel, session_id,
>> +				      peer_session_id, cfg);
>> +	if (!session) {
>> +		rc = -ENOMEM;
>> +		goto out;
>> +	}
>> +
>> +	dev = alloc_netdev(sizeof(*priv), name, l2tp_eth_dev_setup);
>> +	if (!dev) {
>> +		rc = -ENOMEM;
>> +		goto out_del_session;
>> +	}
>> +
>> +	dev_net_set(dev, net);
>> +	if (session->mtu == 0)
>> +		session->mtu = dev->mtu - session->hdr_len;
>> +	dev->mtu = session->mtu;
>> +	dev->needed_headroom += session->hdr_len;
>> +
>> +	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;
>> +	session->session_close = l2tp_eth_delete;
>> +#ifdef CONFIG_PROC_FS
>> +	session->show = l2tp_eth_show;
>> +#endif
>> +
>> +	spriv = l2tp_session_priv(session);
>> +	spriv->dev = dev;
>> +
>> +	rc = register_netdev(dev);
>> +	if (rc < 0)
>> +		goto out_del_dev;
>> +
>> +	/* Must be done after register_netdev() */
>> +	strlcpy(session->ifname, dev->name, IFNAMSIZ);
>> +
>> +	dev_hold(dev);
>> +	pn = l2tp_eth_pernet(dev_net(dev));
>> +	write_lock(&pn->l2tp_eth_lock);
>> +	list_add(&priv->list, &pn->l2tp_eth_dev_list);
>> +	write_unlock(&pn->l2tp_eth_lock);
>> +
>> +	return 0;
>> +
>> +out_del_dev:
>> +	free_netdev(dev);
>> +out_del_session:
>> +	l2tp_session_delete(session);
> 
> 	I cant see the dev_put() here
> 
> 
>> +out:
>> +	return rc;
>> +}
> 
> 
> 


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

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html