netdev - Re: [PATCH net-next v2 04/22] ovpn: add basic interface creation/destruction/management routines

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <48188b78-9238-44cc-ab2f-efdddad90066@openvpn.net>
Date: Tue, 5 Mar 2024 16:51:03 +0100
From: Antonio Quartulli <antonio@...nvpn.net>
To: Andrew Lunn <andrew@...n.ch>
Cc: netdev@...r.kernel.org, Jakub Kicinski <kuba@...nel.org>,
 Sergey Ryazanov <ryazanov.s.a@...il.com>, Paolo Abeni <pabeni@...hat.com>,
 Eric Dumazet <edumazet@...gle.com>
Subject: Re: [PATCH net-next v2 04/22] ovpn: add basic interface
 creation/destruction/management routines



On 04/03/2024 22:33, Andrew Lunn wrote:
>> +int ovpn_struct_init(struct net_device *dev)
>> +{
>> +	struct ovpn_struct *ovpn = netdev_priv(dev);
>> +	int err;
>> +
>> +	memset(ovpn, 0, sizeof(*ovpn));
> 
> Probably not required. When a netdev is created, it should of zeroed
> the priv.

ACK. There is a kvzalloc() involved.

> 
>> +int ovpn_iface_create(const char *name, enum ovpn_mode mode, struct net *net)
>> +{
>> +	struct net_device *dev;
>> +	struct ovpn_struct *ovpn;
>> +	int ret;
>> +
>> +	dev = alloc_netdev(sizeof(struct ovpn_struct), name, NET_NAME_USER, ovpn_setup);
>> +
>> +	dev_net_set(dev, net);
>> +
>> +	ret = ovpn_struct_init(dev);
>> +	if (ret < 0)
>> +		goto err;
>> +
>> +	ovpn = netdev_priv(dev);
>> +	ovpn->mode = mode;
>> +
>> +	rtnl_lock();
>> +
>> +	ret = register_netdevice(dev);
>> +	if (ret < 0) {
>> +		netdev_dbg(dev, "cannot register interface %s: %d\n", dev->name, ret);
>> +		rtnl_unlock();
>> +		goto err;
>> +	}
>> +	rtnl_unlock();
>> +
>> +	return ret;
>> +
>> +err:
>> +	free_netdev(dev);
>> +	return ret;
>> +}
>> +
>> +void ovpn_iface_destruct(struct ovpn_struct *ovpn, bool unregister_netdev)
>> +{
>> +	ASSERT_RTNL();
>> +
>> +	netif_carrier_off(ovpn->dev);
> 
> You often see virtual devices turn their carrier off in there
> probe/create function, because it is unclear what state it is in after
> register_netdevice().

Are you suggesting to turn it off both here and in the create function?
Or should I remove the invocation above?

Regards,

-- 
Antonio Quartulli
OpenVPN Inc.