[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250226171352.258045-1-atenart@kernel.org>
Date: Wed, 26 Feb 2025 18:13:42 +0100
From: Antoine Tenart <atenart@...nel.org>
To: davem@...emloft.net,
kuba@...nel.org,
pabeni@...hat.com,
edumazet@...gle.com
Cc: Antoine Tenart <atenart@...nel.org>,
netdev@...r.kernel.org,
willemdebruijn.kernel@...il.com,
pshelar@....org
Subject: [PATCH net] net: gso: fix ownership in __udp_gso_segment
In __udp_gso_segment the skb destructor is removed before segmenting the
skb but the socket reference is kept as-is. This is an issue if the
original skb is later orphaned as we can hit the following bug:
kernel BUG at ./include/linux/skbuff.h:3312! (skb_orphan)
RIP: 0010:ip_rcv_core+0x8b2/0xca0
Call Trace:
ip_rcv+0xab/0x6e0
__netif_receive_skb_one_core+0x168/0x1b0
process_backlog+0x384/0x1100
__napi_poll.constprop.0+0xa1/0x370
net_rx_action+0x925/0xe50
The above can happen following a sequence of events when using
OpenVSwitch, when an OVS_ACTION_ATTR_USERSPACE action precedes an
OVS_ACTION_ATTR_OUTPUT action:
1. OVS_ACTION_ATTR_USERSPACE is handled (in do_execute_actions): the skb
goes through queue_gso_packets and then __udp_gso_segment, where its
destructor is removed.
2. The segments' data are copied and sent to userspace.
3. OVS_ACTION_ATTR_OUTPUT is handled (in do_execute_actions) and the
same original skb is sent to its path.
4. If it later hits skb_orphan, we hit the bug.
Fix this by also removing the reference to the socket in
__udp_gso_segment.
Fixes: ad405857b174 ("udp: better wmem accounting on gso")
Signed-off-by: Antoine Tenart <atenart@...nel.org>
---
net/ipv4/udp_offload.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index a5be6e4ed326..ecfca59f31f1 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -321,13 +321,17 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
/* clear destructor to avoid skb_segment assigning it to tail */
copy_dtor = gso_skb->destructor == sock_wfree;
- if (copy_dtor)
+ if (copy_dtor) {
gso_skb->destructor = NULL;
+ gso_skb->sk = NULL;
+ }
segs = skb_segment(gso_skb, features);
if (IS_ERR_OR_NULL(segs)) {
- if (copy_dtor)
+ if (copy_dtor) {
gso_skb->destructor = sock_wfree;
+ gso_skb->sk = sk;
+ }
return segs;
}
--
2.48.1
Powered by blists - more mailing lists