2024-11-12, 15:03:00 +0100, Antonio Quartulli wrote:
On 12/11/2024 11:56, Sabrina Dubroca wrote:
2024-10-29, 11:47:30 +0100, Antonio Quartulli wrote:
diff --git a/drivers/net/ovpn/io.c b/drivers/net/ovpn/io.c
index 63c140138bf98e5d1df79a2565b666d86513323d..0e8a6f2c76bc7b2ccc287ad1187cf50f033bf261 100644
--- a/drivers/net/ovpn/io.c
+++ b/drivers/net/ovpn/io.c
@@ -135,6 +135,15 @@ void ovpn_decrypt_post(void *data, int ret)
/* keep track of last received authenticated packet for keepalive */
peer->last_recv = ktime_get_real_seconds();
+ if (peer->sock->sock->sk->sk_protocol == IPPROTO_UDP) {
What prevents peer->sock from being replaced and released
concurrently?
Technically nothing.
Userspace currently does not even support updating a peer socket at runtime,
but I wanted ovpn to be flexible enough from the beginning.
Is there a reason to do that? With TCP the peer would have to
reconnect, and I guess fully restart the whole process (become a new
peer with a new ID etc). With UDP, do you need to replace the socket?
One approach might be to go back to peer->sock being unmutable and forget
about this.
OTOH, if we want to keep this flexibility (which I think is nice), I think I
should make peer->sock an RCU pointer and access it accordingly.
You already use kfree_rcu for ovpn_socket, so the only difference
would be the __rcu annotation and helpers? (+ rcu_read_lock/unlock in
a few places)
Adding rcu_read_lock for peer->sock in ovpn_tcp_tx_work looks
painful... (another place that I missed where things could go bad if
the socket was updated in the current implementation, btw)
Maybe save that for later since you don't have a use case for it yet?