All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
To: Antonio Quartulli <antonio@openvpn.net>
Cc: netdev@vger.kernel.org, Sabrina Dubroca <sd@queasysnail.net>,
	"David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Qingfang Deng <dqfext@gmail.com>
Subject: Re: [PATCH net 3/5] ovpn: avoid sleep in atomic context in TCP RX error path
Date: Tue, 3 Jun 2025 08:42:50 +0200	[thread overview]
Message-ID: <aD6ZamCA8eOpMMSu@mev-dev.igk.intel.com> (raw)
In-Reply-To: <20250530101254.24044-4-antonio@openvpn.net>

On Fri, May 30, 2025 at 12:12:52PM +0200, Antonio Quartulli wrote:
> Upon error along the TCP data_ready event path, we have
> the following chain of calls:
> 
> strp_data_ready()
>   ovpn_tcp_rcv()
>     ovpn_peer_del()
>       ovpn_socket_release()
> 
> Since strp_data_ready() may be invoked from softirq context, and
> ovpn_socket_release() may sleep, the above sequence may cause a
> sleep in atomic context like the following:
> 
>     BUG: sleeping function called from invalid context at ./ovpn-backports-ovpn-net-next-main-6.15.0-rc5-20250522/drivers/net/ovpn/socket.c:71
>     in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 25, name: ksoftirqd/3
>     5 locks held by ksoftirqd/3/25:
>      #0: ffffffe000cd0580 (rcu_read_lock){....}-{1:2}, at: netif_receive_skb+0xb8/0x5b0
>      OpenVPN/ovpn-backports#1: ffffffe000cd0580 (rcu_read_lock){....}-{1:2}, at: netif_receive_skb+0xb8/0x5b0
>      OpenVPN/ovpn-backports#2: ffffffe000cd0580 (rcu_read_lock){....}-{1:2}, at: ip_local_deliver_finish+0x66/0x1e0
>      OpenVPN/ovpn-backports#3: ffffffe003ce9818 (slock-AF_INET/1){+.-.}-{2:2}, at: tcp_v4_rcv+0x156e/0x17a0
>      OpenVPN/ovpn-backports#4: ffffffe000cd0580 (rcu_read_lock){....}-{1:2}, at: ovpn_tcp_data_ready+0x0/0x1b0 [ovpn]
>     CPU: 3 PID: 25 Comm: ksoftirqd/3 Not tainted 5.10.104+ #0
>     Call Trace:
>     walk_stackframe+0x0/0x1d0
>     show_stack+0x2e/0x44
>     dump_stack+0xc2/0x102
>     ___might_sleep+0x29c/0x2b0
>     __might_sleep+0x62/0xa0
>     ovpn_socket_release+0x24/0x2d0 [ovpn]
>     unlock_ovpn+0x6e/0x190 [ovpn]
>     ovpn_peer_del+0x13c/0x390 [ovpn]
>     ovpn_tcp_rcv+0x280/0x560 [ovpn]
>     __strp_recv+0x262/0x940
>     strp_recv+0x66/0x80
>     tcp_read_sock+0x122/0x410
>     strp_data_ready+0x156/0x1f0
>     ovpn_tcp_data_ready+0x92/0x1b0 [ovpn]
>     tcp_data_ready+0x6c/0x150
>     tcp_rcv_established+0xb36/0xc50
>     tcp_v4_do_rcv+0x25e/0x380
>     tcp_v4_rcv+0x166a/0x17a0
>     ip_protocol_deliver_rcu+0x8c/0x250
>     ip_local_deliver_finish+0xf8/0x1e0
>     ip_local_deliver+0xc2/0x2d0
>     ip_rcv+0x1f2/0x330
>     __netif_receive_skb+0xfc/0x290
>     netif_receive_skb+0x104/0x5b0
>     br_pass_frame_up+0x190/0x3f0
>     br_handle_frame_finish+0x3e2/0x7a0
>     br_handle_frame+0x750/0xab0
>     __netif_receive_skb_core.constprop.0+0x4c0/0x17f0
>     __netif_receive_skb+0xc6/0x290
>     netif_receive_skb+0x104/0x5b0
>     xgmac_dma_rx+0x962/0xb40
>     __napi_poll.constprop.0+0x5a/0x350
>     net_rx_action+0x1fe/0x4b0
>     __do_softirq+0x1f8/0x85c
>     run_ksoftirqd+0x80/0xd0
>     smpboot_thread_fn+0x1f0/0x3e0
>     kthread+0x1e6/0x210
>     ret_from_kernel_thread+0x8/0xc
> 
> Fix this issue by postponing the ovpn_peer_del() call to
> a scheduled worker, as we already do in ovpn_tcp_send_sock()
> for the very same reason.
> 
> Fixes: 11851cbd60ea ("ovpn: implement TCP transport")
> Reported-by: Qingfang Deng <dqfext@gmail.com>
> Closes: https://github.com/OpenVPN/ovpn-net-next/issues/13
> Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
> ---
>  drivers/net/ovpn/tcp.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ovpn/tcp.c b/drivers/net/ovpn/tcp.c
> index 7e79aad0b043..289f62c5d2c7 100644
> --- a/drivers/net/ovpn/tcp.c
> +++ b/drivers/net/ovpn/tcp.c
> @@ -124,14 +124,18 @@ static void ovpn_tcp_rcv(struct strparser *strp, struct sk_buff *skb)
>  	 * this peer, therefore ovpn_peer_hold() is not expected to fail
>  	 */
>  	if (WARN_ON(!ovpn_peer_hold(peer)))
> -		goto err;
> +		goto err_nopeer;
>  
>  	ovpn_recv(peer, skb);
>  	return;
>  err:
> +	/* take reference for deferred peer deletion. should never fail */
> +	if (WARN_ON(!ovpn_peer_hold(peer)))
> +		goto err_nopeer;
> +	schedule_work(&peer->tcp.defer_del_work);
>  	dev_dstats_rx_dropped(peer->ovpn->dev);
> +err_nopeer:
>  	kfree_skb(skb);
> -	ovpn_peer_del(peer, OVPN_DEL_PEER_REASON_TRANSPORT_ERROR);

Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>

>  }
>  
>  static int ovpn_tcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
> -- 
> 2.49.0

  reply	other threads:[~2025-06-03  6:43 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-30 10:12 [PATCH net 0/5] pull request: fixes for ovpn 2025-05-30 Antonio Quartulli
2025-05-30 10:12 ` [PATCH net 1/5] ovpn: properly deconfigure UDP-tunnel Antonio Quartulli
2025-06-03  6:30   ` Michal Swiatkowski
2025-06-03  9:02   ` Paolo Abeni
2025-06-03  9:08     ` Antonio Quartulli
2025-06-03  9:58       ` Paolo Abeni
2025-05-30 10:12 ` [PATCH net 2/5] ovpn: ensure sk is still valid during cleanup Antonio Quartulli
2025-06-03  6:40   ` Michal Swiatkowski
2025-05-30 10:12 ` [PATCH net 3/5] ovpn: avoid sleep in atomic context in TCP RX error path Antonio Quartulli
2025-06-03  6:42   ` Michal Swiatkowski [this message]
2025-05-30 10:12 ` [PATCH net 4/5] selftest/net/ovpn: fix TCP socket creation Antonio Quartulli
2025-05-30 10:12 ` [PATCH net 5/5] selftest/net/ovpn: fix missing file Antonio Quartulli
  -- strict thread matches above, loose matches on Subject: below --
2025-06-03 11:11 [PATCH net 0/5] pull request: fixes for ovpn 2025-06-03 Antonio Quartulli
2025-06-03 11:11 ` [PATCH net 3/5] ovpn: avoid sleep in atomic context in TCP RX error path Antonio Quartulli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aD6ZamCA8eOpMMSu@mev-dev.igk.intel.com \
    --to=michal.swiatkowski@linux.intel.com \
    --cc=antonio@openvpn.net \
    --cc=davem@davemloft.net \
    --cc=dqfext@gmail.com \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sd@queasysnail.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.