2025-02-27, 02:21:37 +0100, Antonio Quartulli wrote:
Moreover export tcp_release_cb by means of EXPORT_SYMBOL instead of EXPORT_IPV6_MOD, so that other modules can use it, even if IPV6 is not compiled in.
Is that really needed? You're saving tcp.sk_cb.prot, so you could just call peer->tcp.sk_cb.prot->release_cb? (with a bit of care since it's called after peer_put)
[I don't know what the maintainers' preference is wrt "re-exporting" symbols that got moved to EXPORT_IPV6_MOD]
[...]
+static void ovpn_tcp_send_sock(struct ovpn_peer *peer, struct sock *sk) +{
- struct sk_buff *skb = peer->tcp.out_msg.skb;
- if (!skb)
return;- if (peer->tcp.tx_in_progress)
return;- peer->tcp.tx_in_progress = true;
- do {
int ret = skb_send_sock_locked(sk, skb,peer->tcp.out_msg.offset,peer->tcp.out_msg.len);if (unlikely(ret < 0)) {if (ret == -EAGAIN)goto out;net_warn_ratelimited("%s: TCP error to peer %u: %d\n",netdev_name(peer->ovpn->dev),peer->id, ret);/* in case of TCP error we can't recover the VPN* stream therefore we abort the connection*/ovpn_peer_del(peer,OVPN_DEL_PEER_REASON_TRANSPORT_ERROR);
I don't think this works:
ovpn_peer_del -> unlock_ovpn -> ovpn_socket_release -> might_sleep
but we can get to ovpn_tcp_send_sock in a few contexts that are not allowed to sleep:
ovpn_tcp_send_skb -> ovpn_tcp_send_sock_skb -> ovpn_tcp_send_sock __sk_flush_backlog -> release_cb = ovpn_tcp_release -> ovpn_tcp_send_sock_skb release_sock -> release_cb = ovpn_tcp_release -> ovpn_tcp_send_sock_skb
(I checked all other paths leading to unlock_ovpn/ovpn_socket_release, this is the only one I could find that is not allowed to sleep. So it would likely be easier to push this peer_del (or even just the handling of release_list) into some other sleepable context than trying to reshuffle all the other paths)