2025-01-13, 10:31:34 +0100, Antonio Quartulli wrote:
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...)