Included fixes:

* use rcu_dereference_bh() instead of rcu_access_pointer() where the
   pointer is actually dereferenced
 * ensure TCP global variables are initialized before they can be
   accessed via netlink (e.g. when attaching a TCP socket)
 * actually disable IPv4 redirects on multipeer interfaces (the
   previous attempt was a no-op and did not survive netns moves)
 * hash a floated peer by its transport identity only, consistently
   with the add and lookup paths
 * zero the sockaddr padding before learning a floated endpoint so it
   does not leak into the by_transp_addr hash key
 * ensure the socket is owned by ovpn before dereferencing
   sk_user_data
 * rehash a peer in the by_transp_addr table when its remote endpoint
   is updated via CMD_PEER_SET
 * avoid re-adding to the hashtables a peer that was concurrently
   removed (use-after-free)
 * limit keepalive values to one day to avoid overflowing the
   delayed-work delay on 32-bit systems
 * add the missing rtnl_link_ops->get_size callback so link messages
   account for the nested mode attribute
 -----BEGIN PGP SIGNATURE-----
 
 iJEEABYIADkWIQQr0db7q+Rc7Zog28Fc8QQzwdnOtwUCamsZiRsUgAAAAAAEAA5t
 YW51MiwyLjUrMS4xMiwyLDIACgkQXPEEM8HZzrdjhQD/SJjvWsxHurn7vQJ8VFw9
 wb8Q06TpSjdHkd5xXpQzohoA/joMlAYnVhSlYDcDaF3DCzmCAW6fG/bpPoI4bpFj
 i8UD
 =Oppi
 -----END PGP SIGNATURE-----

Merge tag 'ovpn-net-20260730' of https://github.com/OpenVPN/ovpn-net-next

Antonio Quartulli says:

====================
Included fixes:

* use rcu_dereference_bh() instead of rcu_access_pointer() where the
  pointer is actually dereferenced
* ensure TCP global variables are initialized before they can be
  accessed via netlink (e.g. when attaching a TCP socket)
* actually disable IPv4 redirects on multipeer interfaces (the
  previous attempt was a no-op and did not survive netns moves)
* hash a floated peer by its transport identity only, consistently
  with the add and lookup paths
* zero the sockaddr padding before learning a floated endpoint so it
  does not leak into the by_transp_addr hash key
* ensure the socket is owned by ovpn before dereferencing
  sk_user_data
* rehash a peer in the by_transp_addr table when its remote endpoint
  is updated via CMD_PEER_SET
* avoid re-adding to the hashtables a peer that was concurrently
  removed (use-after-free)
* limit keepalive values to one day to avoid overflowing the
  delayed-work delay on 32-bit systems
* add the missing rtnl_link_ops->get_size callback so link messages
  account for the nested mode attribute

* tag 'ovpn-net-20260730' of https://github.com/OpenVPN/ovpn-net-next:
  ovpn: fix incorrect use of rcu_access_pointer()
  ovpn: ensure TCP vars are initialized first
  ovpn: disable IPv4 redirects on MP interfaces
  ovpn: hash floated peer by transport identity only
  ovpn: zero-initialize sockaddr before learning a floated endpoint
  ovpn: ensure socket is owned by ovpn before deref sk_user_data
  ovpn: rehash peer in by_transp_addr table on CMD_PEER_SET
  ovpn: skip rehash for peers already removed from by_id
  ovpn: limit keepalive values to one day
  ovpn: add missing rtnl_link_ops->get_size callback
====================

Link: https://patch.msgid.link/20260730094624.4102963-1-antonio@openvpn.net
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
Jakub Kicinski 2026-08-04 17:32:36 -07:00
commit e31420642d
7 changed files with 208 additions and 68 deletions

View File

@ -118,12 +118,16 @@ attribute-sets:
doc: >-
The number of seconds after which a keep alive message is sent to the
peer
checks:
max: 86400
-
name: keepalive-timeout
type: u32
doc: >-
The number of seconds from the last activity after which the peer is
assumed dead
checks:
max: 86400
-
name: del-reason
type: u32

View File

@ -35,25 +35,11 @@ static void ovpn_priv_free(struct net_device *net)
static int ovpn_mp_alloc(struct ovpn_priv *ovpn)
{
struct in_device *dev_v4;
int i;
if (ovpn->mode != OVPN_MODE_MP)
return 0;
dev_v4 = __in_dev_get_rtnl(ovpn->dev);
if (dev_v4) {
/* disable redirects as Linux gets confused by ovpn
* handling same-LAN routing.
* This happens because a multipeer interface is used as
* relay point between hosts in the same subnet, while
* in a classic LAN this would not be needed because the
* two hosts would be able to talk directly.
*/
IN_DEV_CONF_SET(dev_v4, SEND_REDIRECTS, false);
IPV4_DEVCONF_ALL(dev_net(ovpn->dev), SEND_REDIRECTS) = false;
}
/* the peer container is fairly large, therefore we allocate it only in
* MP mode
*/
@ -97,9 +83,38 @@ static void ovpn_net_uninit(struct net_device *dev)
gro_cells_destroy(&ovpn->gro_cells);
}
static int ovpn_net_open(struct net_device *dev)
{
struct ovpn_priv *ovpn = netdev_priv(dev);
struct in_device *dev_v4;
/* the IPv4 in_device (and thus its config) is recreated whenever the
* interface is moved to a new netns, so redirects must be disabled on
* every bring-up rather than once at creation time, otherwise the
* setting is silently lost after such a move
*/
if (ovpn->mode == OVPN_MODE_MP) {
dev_v4 = __in_dev_get_rtnl(dev);
if (dev_v4) {
/* disable redirects as Linux gets confused by ovpn
* handling same-LAN routing.
* This happens because a multipeer interface is used as
* relay point between hosts in the same subnet, while
* in a classic LAN this would not be needed because the
* two hosts would be able to talk directly.
*/
IN_DEV_CONF_SET(dev_v4, SEND_REDIRECTS, false);
IPV4_DEVCONF_ALL(dev_net(dev), SEND_REDIRECTS) = false;
}
}
return 0;
}
static const struct net_device_ops ovpn_netdev_ops = {
.ndo_init = ovpn_net_init,
.ndo_uninit = ovpn_net_uninit,
.ndo_open = ovpn_net_open,
.ndo_start_xmit = ovpn_net_xmit,
};
@ -183,6 +198,7 @@ static int ovpn_newlink(struct net_device *dev,
struct ovpn_priv *ovpn = netdev_priv(dev);
struct nlattr **data = params->data;
enum ovpn_mode mode = OVPN_MODE_P2P;
int ret;
if (data && data[IFLA_OVPN_MODE]) {
mode = nla_get_u8(data[IFLA_OVPN_MODE]);
@ -207,7 +223,17 @@ static int ovpn_newlink(struct net_device *dev,
else
netif_carrier_off(dev);
return register_netdevice(dev);
ret = register_netdevice(dev);
if (ret < 0)
return ret;
return 0;
}
static size_t ovpn_get_size(const struct net_device *dev)
{
/* IFLA_OVPN_MODE */
return nla_total_size(sizeof(u8));
}
static int ovpn_fill_info(struct sk_buff *skb, const struct net_device *dev)
@ -228,13 +254,17 @@ static struct rtnl_link_ops ovpn_link_ops = {
.policy = ovpn_policy,
.maxtype = IFLA_OVPN_MAX,
.newlink = ovpn_newlink,
.get_size = ovpn_get_size,
.fill_info = ovpn_fill_info,
};
static int __init ovpn_init(void)
{
int err = rtnl_link_register(&ovpn_link_ops);
int err;
ovpn_tcp_init();
err = rtnl_link_register(&ovpn_link_ops);
if (err) {
pr_err("ovpn: can't register rtnl link ops: %d\n", err);
return err;
@ -246,8 +276,6 @@ static int __init ovpn_init(void)
goto unreg_rtnl;
}
ovpn_tcp_init();
return 0;
unreg_rtnl:

View File

@ -16,6 +16,14 @@ static const struct netlink_range_validation ovpn_a_peer_id_range = {
.max = 16777215ULL,
};
static const struct netlink_range_validation ovpn_a_peer_keepalive_interval_range = {
.max = 86400ULL,
};
static const struct netlink_range_validation ovpn_a_peer_keepalive_timeout_range = {
.max = 86400ULL,
};
static const struct netlink_range_validation ovpn_a_peer_tx_id_range = {
.max = 16777215ULL,
};
@ -68,8 +76,8 @@ const struct nla_policy ovpn_peer_nl_policy[OVPN_A_PEER_TX_ID + 1] = {
[OVPN_A_PEER_LOCAL_IPV4] = { .type = NLA_BE32, },
[OVPN_A_PEER_LOCAL_IPV6] = NLA_POLICY_EXACT_LEN(16),
[OVPN_A_PEER_LOCAL_PORT] = NLA_POLICY_MIN(NLA_BE16, 1),
[OVPN_A_PEER_KEEPALIVE_INTERVAL] = { .type = NLA_U32, },
[OVPN_A_PEER_KEEPALIVE_TIMEOUT] = { .type = NLA_U32, },
[OVPN_A_PEER_KEEPALIVE_INTERVAL] = NLA_POLICY_FULL_RANGE(NLA_U32, &ovpn_a_peer_keepalive_interval_range),
[OVPN_A_PEER_KEEPALIVE_TIMEOUT] = NLA_POLICY_FULL_RANGE(NLA_U32, &ovpn_a_peer_keepalive_timeout_range),
[OVPN_A_PEER_DEL_REASON] = NLA_POLICY_MAX(NLA_U32, 4),
[OVPN_A_PEER_VPN_RX_BYTES] = { .type = NLA_UINT, },
[OVPN_A_PEER_VPN_TX_BYTES] = { .type = NLA_UINT, },
@ -97,8 +105,8 @@ const struct nla_policy ovpn_peer_new_input_nl_policy[OVPN_A_PEER_TX_ID + 1] = {
[OVPN_A_PEER_VPN_IPV6] = NLA_POLICY_EXACT_LEN(16),
[OVPN_A_PEER_LOCAL_IPV4] = { .type = NLA_BE32, },
[OVPN_A_PEER_LOCAL_IPV6] = NLA_POLICY_EXACT_LEN(16),
[OVPN_A_PEER_KEEPALIVE_INTERVAL] = { .type = NLA_U32, },
[OVPN_A_PEER_KEEPALIVE_TIMEOUT] = { .type = NLA_U32, },
[OVPN_A_PEER_KEEPALIVE_INTERVAL] = NLA_POLICY_FULL_RANGE(NLA_U32, &ovpn_a_peer_keepalive_interval_range),
[OVPN_A_PEER_KEEPALIVE_TIMEOUT] = NLA_POLICY_FULL_RANGE(NLA_U32, &ovpn_a_peer_keepalive_timeout_range),
[OVPN_A_PEER_TX_ID] = NLA_POLICY_FULL_RANGE(NLA_U32, &ovpn_a_peer_tx_id_range),
};
@ -112,8 +120,8 @@ const struct nla_policy ovpn_peer_set_input_nl_policy[OVPN_A_PEER_TX_ID + 1] = {
[OVPN_A_PEER_VPN_IPV6] = NLA_POLICY_EXACT_LEN(16),
[OVPN_A_PEER_LOCAL_IPV4] = { .type = NLA_BE32, },
[OVPN_A_PEER_LOCAL_IPV6] = NLA_POLICY_EXACT_LEN(16),
[OVPN_A_PEER_KEEPALIVE_INTERVAL] = { .type = NLA_U32, },
[OVPN_A_PEER_KEEPALIVE_TIMEOUT] = { .type = NLA_U32, },
[OVPN_A_PEER_KEEPALIVE_INTERVAL] = NLA_POLICY_FULL_RANGE(NLA_U32, &ovpn_a_peer_keepalive_interval_range),
[OVPN_A_PEER_KEEPALIVE_TIMEOUT] = NLA_POLICY_FULL_RANGE(NLA_U32, &ovpn_a_peer_keepalive_timeout_range),
[OVPN_A_PEER_TX_ID] = NLA_POLICY_FULL_RANGE(NLA_U32, &ovpn_a_peer_tx_id_range),
};

View File

@ -534,6 +534,12 @@ int ovpn_nl_peer_set_doit(struct sk_buff *skb, struct genl_info *info)
*/
if (ret > 0)
ovpn_peer_hash_vpn_ip(peer);
/* if the remote endpoint was updated, the by_transp_addr hash bucket
* also needs to be refreshed, otherwise incoming packets from the new
* remote address would fail the lockless lookup
*/
if (attrs[OVPN_A_PEER_REMOTE_IPV4] || attrs[OVPN_A_PEER_REMOTE_IPV6])
ovpn_peer_hash_transp_addr(peer);
spin_unlock_bh(&ovpn->lock);
ovpn_peer_put(peer);

View File

@ -189,6 +189,9 @@ int ovpn_peer_reset_sockaddr(struct ovpn_peer *peer,
&(*__tbl1)[ovpn_get_hash_slot(*__tbl1, _key, _key_len)];\
})
static void __ovpn_peer_hash_transp_addr(struct ovpn_peer *peer,
const struct ovpn_bind *bind);
/**
* ovpn_peer_endpoints_update - update remote or local endpoint for peer
* @peer: peer to update the remote endpoint for
@ -196,7 +199,6 @@ int ovpn_peer_reset_sockaddr(struct ovpn_peer *peer,
*/
void ovpn_peer_endpoints_update(struct ovpn_peer *peer, struct sk_buff *skb)
{
struct hlist_nulls_head *nhead;
struct sockaddr_storage ss;
struct sockaddr_in6 *sa6;
bool reset_cache = false;
@ -220,9 +222,16 @@ void ovpn_peer_endpoints_update(struct ovpn_peer *peer, struct sk_buff *skb)
*/
local_ip = &ip_hdr(skb)->daddr;
sa = (struct sockaddr_in *)&ss;
sa->sin_family = AF_INET;
sa->sin_addr.s_addr = ip_hdr(skb)->saddr;
sa->sin_port = udp_hdr(skb)->source;
/* use a designated initializer so the sin_zero padding
* is zeroed (it ends up in the by_transp_addr hash key)
* without memset-ing the whole sockaddr_storage on the
* RX fast path
*/
*sa = (struct sockaddr_in) {
.sin_family = AF_INET,
.sin_addr.s_addr = ip_hdr(skb)->saddr,
.sin_port = udp_hdr(skb)->source,
};
salen = sizeof(*sa);
reset_cache = true;
break;
@ -248,11 +257,19 @@ void ovpn_peer_endpoints_update(struct ovpn_peer *peer, struct sk_buff *skb)
*/
local_ip = &ipv6_hdr(skb)->daddr;
sa6 = (struct sockaddr_in6 *)&ss;
sa6->sin6_family = AF_INET6;
sa6->sin6_addr = ipv6_hdr(skb)->saddr;
sa6->sin6_port = udp_hdr(skb)->source;
sa6->sin6_scope_id = ipv6_iface_scope_id(&ipv6_hdr(skb)->saddr,
skb->skb_iif);
/* use a designated initializer so the sin6_flowinfo
* padding is zeroed (it ends up in the by_transp_addr
* hash key) without memset-ing the whole
* sockaddr_storage on the RX fast path
*/
*sa6 = (struct sockaddr_in6) {
.sin6_family = AF_INET6,
.sin6_addr = ipv6_hdr(skb)->saddr,
.sin6_port = udp_hdr(skb)->source,
.sin6_scope_id =
ipv6_iface_scope_id(&ipv6_hdr(skb)->saddr,
skb->skb_iif),
};
salen = sizeof(*sa6);
reset_cache = true;
break;
@ -295,42 +312,25 @@ void ovpn_peer_endpoints_update(struct ovpn_peer *peer, struct sk_buff *skb)
ovpn_nl_peer_float_notify(peer, &ss);
/* rehashing is required only in MP mode as P2P has one peer
* only and thus there is no hashtable
* only and thus there is no hashtable.
*
* This function may be invoked concurrently, so re-read peer->bind
* under the proper locks and rehash against its current value.
*/
if (peer->ovpn->mode == OVPN_MODE_MP) {
spin_lock_bh(&peer->ovpn->lock);
spin_lock_bh(&peer->lock);
bind = rcu_dereference_protected(peer->bind,
lockdep_is_held(&peer->lock));
if (unlikely(!bind)) {
spin_unlock_bh(&peer->lock);
spin_unlock_bh(&peer->ovpn->lock);
return;
}
if (peer->ovpn->mode != OVPN_MODE_MP)
return;
/* This function may be invoked concurrently, therefore another
* float may have happened in parallel: perform rehashing
* using the peer->bind->remote directly as key
*/
switch (bind->remote.in4.sin_family) {
case AF_INET:
salen = sizeof(*sa);
break;
case AF_INET6:
salen = sizeof(*sa6);
break;
}
/* remove old hashing */
hlist_nulls_del_init_rcu(&peer->hash_entry_transp_addr);
/* re-add with new transport address */
nhead = ovpn_get_hash_head(peer->ovpn->peers->by_transp_addr,
&bind->remote, salen);
hlist_nulls_add_head_rcu(&peer->hash_entry_transp_addr, nhead);
spin_unlock_bh(&peer->lock);
spin_unlock_bh(&peer->ovpn->lock);
}
/* This function may be invoked concurrently, therefore another
* float may have happened in parallel: re-acquire the locks and
* rehash using the peer->bind->remote directly as key
*/
spin_lock_bh(&peer->ovpn->lock);
spin_lock_bh(&peer->lock);
bind = rcu_dereference_protected(peer->bind,
lockdep_is_held(&peer->lock));
__ovpn_peer_hash_transp_addr(peer, bind);
spin_unlock_bh(&peer->lock);
spin_unlock_bh(&peer->ovpn->lock);
return;
unlock:
spin_unlock_bh(&peer->lock);
@ -896,6 +896,83 @@ bool ovpn_peer_check_by_src(struct ovpn_priv *ovpn, struct sk_buff *skb,
return match;
}
/* Move @peer to the by_transp_addr bucket matching its current bind.
*
* Caller must hold both peer->ovpn->lock and peer->lock, and must have
* already dereferenced a valid (non-NULL) peer->bind, passed in as @bind.
*/
static void __ovpn_peer_hash_transp_addr(struct ovpn_peer *peer,
const struct ovpn_bind *bind)
{
struct sockaddr_storage sa = {};
struct hlist_nulls_head *nhead;
struct sockaddr_in6 *sa6;
struct sockaddr_in *sa4;
size_t salen;
lockdep_assert_held(&peer->ovpn->lock);
lockdep_assert_held(&peer->lock);
if (WARN_ON_ONCE(!bind))
return;
/* peer may have been concurrently removed between the caller's
* initial lookup and our acquisition of ovpn->lock; skip the
* rehash so we don't re-insert a removed peer
*/
if (unlikely(hlist_unhashed(&peer->hash_entry_id)))
return;
/* Build the hash key from the transport identity only
* (family/address/port), matching ovpn_peer_add_mp() and the lookup
* in ovpn_peer_get_by_transp_addr(). Hashing bind->remote directly
* would fold in sin6_scope_id (set on the float path but never by the
* lookup), scattering the peer into a bucket lookups cannot reach.
*/
switch (bind->remote.in4.sin_family) {
case AF_INET:
sa4 = (struct sockaddr_in *)&sa;
sa4->sin_family = AF_INET;
sa4->sin_addr.s_addr = bind->remote.in4.sin_addr.s_addr;
sa4->sin_port = bind->remote.in4.sin_port;
salen = sizeof(*sa4);
break;
case AF_INET6:
sa6 = (struct sockaddr_in6 *)&sa;
sa6->sin6_family = AF_INET6;
sa6->sin6_addr = bind->remote.in6.sin6_addr;
sa6->sin6_port = bind->remote.in6.sin6_port;
salen = sizeof(*sa6);
break;
default:
return;
}
/* remove old hashing (no-op if entry is not currently linked) */
hlist_nulls_del_init_rcu(&peer->hash_entry_transp_addr);
/* re-add with current transport address */
nhead = ovpn_get_hash_head(peer->ovpn->peers->by_transp_addr, &sa,
salen);
hlist_nulls_add_head_rcu(&peer->hash_entry_transp_addr, nhead);
}
void ovpn_peer_hash_transp_addr(struct ovpn_peer *peer)
{
struct ovpn_bind *bind;
lockdep_assert_held(&peer->ovpn->lock);
/* rehashing makes sense only in multipeer mode */
if (peer->ovpn->mode != OVPN_MODE_MP)
return;
spin_lock_bh(&peer->lock);
bind = rcu_dereference_protected(peer->bind,
lockdep_is_held(&peer->lock));
__ovpn_peer_hash_transp_addr(peer, bind);
spin_unlock_bh(&peer->lock);
}
void ovpn_peer_hash_vpn_ip(struct ovpn_peer *peer)
{
struct hlist_nulls_head *nhead;
@ -906,6 +983,13 @@ void ovpn_peer_hash_vpn_ip(struct ovpn_peer *peer)
if (peer->ovpn->mode != OVPN_MODE_MP)
return;
/* peer may have been concurrently removed between the caller's
* initial lookup and our acquisition of ovpn->lock; skip the
* rehash so we don't re-insert a removed peer
*/
if (hlist_unhashed(&peer->hash_entry_id))
return;
if (peer->vpn_addrs.ipv4.s_addr != htonl(INADDR_ANY)) {
/* remove potential old hashing */
hlist_nulls_del_init_rcu(&peer->hash_entry_addr4);
@ -1165,7 +1249,7 @@ static void ovpn_peer_release_p2p(struct ovpn_priv *ovpn, struct sock *sk,
}
if (sk) {
ovpn_sock = rcu_access_pointer(peer->sock);
ovpn_sock = rcu_dereference_bh(peer->sock);
if (!ovpn_sock || ovpn_sock->sk != sk) {
spin_unlock_bh(&ovpn->lock);
return;

View File

@ -150,6 +150,7 @@ struct ovpn_peer *ovpn_peer_get_by_id(struct ovpn_priv *ovpn, u32 peer_id);
struct ovpn_peer *ovpn_peer_get_by_dst(struct ovpn_priv *ovpn,
struct sk_buff *skb);
void ovpn_peer_hash_vpn_ip(struct ovpn_peer *peer);
void ovpn_peer_hash_transp_addr(struct ovpn_peer *peer);
bool ovpn_peer_check_by_src(struct ovpn_priv *ovpn, struct sk_buff *skb,
struct ovpn_peer *peer);

View File

@ -162,6 +162,15 @@ struct ovpn_socket *ovpn_socket_new(struct socket *sock, struct ovpn_peer *peer)
rcu_read_lock();
ovpn_sock = rcu_dereference_sk_user_data(sk);
if (ovpn_sock) {
/* something else filled the sk_user_data without
* setting the encap_type. Reject the socket.
*/
if (!type) {
ovpn_sock = ERR_PTR(-EBUSY);
rcu_read_unlock();
goto sock_release;
}
/* socket owned by another ovpn instance, we can't use it */
if (ovpn_sock->ovpn != peer->ovpn) {
ovpn_sock = ERR_PTR(-EBUSY);