Thread (45 messages) 45 messages, 4 authors, 2024-12-09

Re: [PATCH net-next v12 17/22] ovpn: implement peer add/get/dump/delete via netlink

flat view

From: Paolo Abeni <pabeni@redhat.com>
Date: 2024-12-03 17:46:49
Also in: linux-kselftest, lkml

On 12/2/24 16:07, Antonio Quartulli wrote:
quoted hunk ↗ jump to hunk
+/**
+ * ovpn_nl_peer_modify - modify the peer attributes according to the incoming msg
+ * @peer: the peer to modify
+ * @info: generic netlink info from the user request
+ * @attrs: the attributes from the user request
+ *
+ * Return: a negative error code in case of failure, 0 on success or 1 on
+ *	   success and the VPN IPs have been modified (requires rehashing in MP
+ *	   mode)
+ */
+static int ovpn_nl_peer_modify(struct ovpn_peer *peer, struct genl_info *info,
+			       struct nlattr **attrs)
+{
+	struct sockaddr_storage ss = {};
+	u32 sockfd, interv, timeout;
+	struct socket *sock = NULL;
+	u8 *local_ip = NULL;
+	bool rehash = false;
+	int ret;
+
+	if (attrs[OVPN_A_PEER_SOCKET]) {
+		if (peer->sock) {
+			NL_SET_ERR_MSG_FMT_MOD(info->extack,
+					       "peer socket can't be modified");
+			return -EINVAL;
+		}
+
+		/* lookup the fd in the kernel table and extract the socket
+		 * object
+		 */
+		sockfd = nla_get_u32(attrs[OVPN_A_PEER_SOCKET]);
+		/* sockfd_lookup() increases sock's refcounter */
+		sock = sockfd_lookup(sockfd, &ret);
+		if (!sock) {
+			NL_SET_ERR_MSG_FMT_MOD(info->extack,
+					       "cannot lookup peer socket (fd=%u): %d",
+					       sockfd, ret);
+			return -ENOTSOCK;
+		}
+
+		/* Only when using UDP as transport protocol the remote endpoint
+		 * can be configured so that ovpn knows where to send packets
+		 * to.
+		 *
+		 * In case of TCP, the socket is connected to the peer and ovpn
+		 * will just send bytes over it, without the need to specify a
+		 * destination.
+		 */
+		if (sock->sk->sk_protocol != IPPROTO_UDP &&
+		    (attrs[OVPN_A_PEER_REMOTE_IPV4] ||
+		     attrs[OVPN_A_PEER_REMOTE_IPV6])) {
+			NL_SET_ERR_MSG_FMT_MOD(info->extack,
+					       "unexpected remote IP address for non UDP socket");
+			sockfd_put(sock);
+			return -EINVAL;
+		}
+
+		peer->sock = ovpn_socket_new(sock, peer);
+		if (IS_ERR(peer->sock)) {
+			NL_SET_ERR_MSG_FMT_MOD(info->extack,
+					       "cannot encapsulate socket: %ld",
+					       PTR_ERR(peer->sock));
+			sockfd_put(sock);
+			peer->sock = NULL;
This looks race-prone. If any other CPU can do concurrent read access to
peer->sock it could observe an invalid pointer
Even if such race does not exist, it would be cleaner store
ovpn_socket_new() return value in a local variable and set peer->sock
only on successful creation.

/P
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help