]> git.ipfire.org Git - thirdparty/linux.git/commitdiff
net: gso: fix ownership in __udp_gso_segment
authorAntoine Tenart <atenart@kernel.org>
Wed, 26 Feb 2025 17:13:42 +0000 (18:13 +0100)
committerJakub Kicinski <kuba@kernel.org>
Sat, 1 Mar 2025 00:14:36 +0000 (16:14 -0800)
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@kernel.org>
Link: https://patch.msgid.link/20250226171352.258045-1-atenart@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
net/ipv4/udp_offload.c

index a5be6e4ed326fbdc6a9b3889db4da903f7f25d37..ecfca59f31f13ea35bbc1ef6504e58282544e186 100644 (file)
@@ -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;
        }