Re: [PATCH net-next v18 15/25] ovpn: implement multi-peer support
From: Sabrina Dubroca <sd@queasysnail.net>
Date: 2025-02-02 23:00:34
Also in:
linux-kselftest, lkml
2025-01-13, 10:31:34 +0100, Antonio Quartulli wrote:
quoted hunk ↗ jump to hunk
static int ovpn_newlink(struct net *src_net, struct net_device *dev, struct nlattr *tb[], struct nlattr *data[], struct netlink_ext_ack *extack) { struct ovpn_priv *ovpn = netdev_priv(dev); enum ovpn_mode mode = OVPN_MODE_P2P; + int err; if (data && data[IFLA_OVPN_MODE]) { mode = nla_get_u8(data[IFLA_OVPN_MODE]);@@ -136,6 +183,10 @@ static int ovpn_newlink(struct net *src_net, struct net_device *dev, ovpn->mode = mode; spin_lock_init(&ovpn->lock); + err = ovpn_mp_alloc(ovpn);
If register_netdevice fails, ovpn->peers won't get freed in some cases (only if we got past ndo_init). So this should go into ndo_init.
+ if (err < 0) + return err; + /* turn carrier explicitly off after registration, this way state is * clearly defined */
[...]
+static int ovpn_peer_add_mp(struct ovpn_priv *ovpn, struct ovpn_peer *peer)
+{[...]
+ hlist_add_head_rcu(&peer->hash_entry_id,
+ ovpn_get_hash_head(ovpn->peers->by_id, &peer->id,
+ sizeof(peer->id)));
+
+ if (peer->vpn_addrs.ipv4.s_addr != htonl(INADDR_ANY)) {
+ nhead = ovpn_get_hash_head(ovpn->peers->by_vpn_addr,
+ &peer->vpn_addrs.ipv4,
+ sizeof(peer->vpn_addrs.ipv4));
+ hlist_nulls_add_head_rcu(&peer->hash_entry_addr4, nhead);
+ }
+
+ if (!ipv6_addr_any(&peer->vpn_addrs.ipv6)) {
+ nhead = ovpn_get_hash_head(ovpn->peers->by_vpn_addr,
+ &peer->vpn_addrs.ipv6,
+ sizeof(peer->vpn_addrs.ipv6));
+ hlist_nulls_add_head_rcu(&peer->hash_entry_addr6, nhead);
+ }
You can't add hash_entry_addr4 and hash_entry_addr6 to the same
hashtable. ovpn_peer_get_by_vpn_addr{4,6} use those fields as
"member" for hlist_nulls_for_each_entry_rcu, so container_of (in
hlist_nulls_entry) will return a "peer" that's not really a peer
object in memory when we walk past an entry for the wrong address
family:
container_of(peer_v4->hash_entry_addr4, struct ovpn_peer, hash_entry_addr6)
or
container_of(peer_v6->hash_entry_addr6, struct ovpn_peer, hash_entry_addr4)
(probably not visible in testing since we'll never really get 2 peers
(and of different families) into the same bucket, and then also get
them to pass the addr_equal test in ovpn_peer_get_by_vpn_addr{4,6}.
easiest way to try to trigger problems would be making the hashtable
single bucket, and even then...)
--
Sabrina