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...)
--
Sabrina
