1. 10 Aug, 2024 3 commits
    • Jakub Sitnicki's avatar
      udp: Fall back to software USO if IPv6 extension headers are present · 30b03f2a
      Jakub Sitnicki authored
      In commit 10154dbd ("udp: Allow GSO transmit from devices with no
      checksum offload") we have intentionally allowed UDP GSO packets marked
      CHECKSUM_NONE to pass to the GSO stack, so that they can be segmented and
      checksummed by a software fallback when the egress device lacks these
      features.
      
      What was not taken into consideration is that a CHECKSUM_NONE skb can be
      handed over to the GSO stack also when the egress device advertises the
      tx-udp-segmentation / NETIF_F_GSO_UDP_L4 feature.
      
      This will happen when there are IPv6 extension headers present, which we
      check for in __ip6_append_data(). Syzbot has discovered this scenario,
      producing a warning as below:
      
        ip6tnl0: caps=(0x00000006401d7869, 0x00000006401d7869)
        WARNING: CPU: 0 PID: 5112 at net/core/dev.c:3293 skb_warn_bad_offload+0x166/0x1a0 net/core/dev.c:3291
        Modules linked in:
        CPU: 0 PID: 5112 Comm: syz-executor391 Not tainted 6.10.0-rc7-syzkaller-01603-g80ab5445 #0
        Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 06/07/2024
        RIP: 0010:skb_warn_bad_offload+0x166/0x1a0 net/core/dev.c:3291
        [...]
        Call Trace:
         <TASK>
         __skb_gso_segment+0x3be/0x4c0 net/core/gso.c:127
         skb_gso_segment include/net/gso.h:83 [inline]
         validate_xmit_skb+0x585/0x1120 net/core/dev.c:3661
         __dev_queue_xmit+0x17a4/0x3e90 net/core/dev.c:4415
         neigh_output include/net/neighbour.h:542 [inline]
         ip6_finish_output2+0xffa/0x1680 net/ipv6/ip6_output.c:137
         ip6_finish_output+0x41e/0x810 net/ipv6/ip6_output.c:222
         ip6_send_skb+0x112/0x230 net/ipv6/ip6_output.c:1958
         udp_v6_send_skb+0xbf5/0x1870 net/ipv6/udp.c:1292
         udpv6_sendmsg+0x23b3/0x3270 net/ipv6/udp.c:1588
         sock_sendmsg_nosec net/socket.c:730 [inline]
         __sock_sendmsg+0xef/0x270 net/socket.c:745
         ____sys_sendmsg+0x525/0x7d0 net/socket.c:2585
         ___sys_sendmsg net/socket.c:2639 [inline]
         __sys_sendmmsg+0x3b2/0x740 net/socket.c:2725
         __do_sys_sendmmsg net/socket.c:2754 [inline]
         __se_sys_sendmmsg net/socket.c:2751 [inline]
         __x64_sys_sendmmsg+0xa0/0xb0 net/socket.c:2751
         do_syscall_x64 arch/x86/entry/common.c:52 [inline]
         do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
         entry_SYSCALL_64_after_hwframe+0x77/0x7f
         [...]
         </TASK>
      
      We are hitting the bad offload warning because when an egress device is
      capable of handling segmentation offload requested by
      skb_shinfo(skb)->gso_type, the chain of gso_segment callbacks won't produce
      any segment skbs and return NULL. See the skb_gso_ok() branch in
      {__udp,tcp,sctp}_gso_segment helpers.
      
      To fix it, force a fallback to software USO when processing a packet with
      IPv6 extension headers, since we don't know if these can checksummed by
      all devices which offer USO.
      
      Fixes: 10154dbd ("udp: Allow GSO transmit from devices with no checksum offload")
      Reported-by: syzbot+e15b7e15b8a751a91d9a@syzkaller.appspotmail.com
      Closes: https://lore.kernel.org/all/000000000000e1609a061d5330ce@google.com/Reviewed-by: default avatarWillem de Bruijn <willemb@google.com>
      Signed-off-by: default avatarJakub Sitnicki <jakub@cloudflare.com>
      Link: https://patch.msgid.link/20240808-udp-gso-egress-from-tunnel-v4-2-f5c5b4149ab9@cloudflare.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      30b03f2a
    • Jakub Sitnicki's avatar
      net: Make USO depend on CSUM offload · 2b2bc3ba
      Jakub Sitnicki authored
      UDP segmentation offload inherently depends on checksum offload. It should
      not be possible to disable checksum offload while leaving USO enabled.
      Enforce this dependency in code.
      
      There is a single tx-udp-segmentation feature flag to indicate support for
      both IPv4/6, hence the devices wishing to support USO must offer checksum
      offload for both IP versions.
      
      Fixes: 10154dbd ("udp: Allow GSO transmit from devices with no checksum offload")
      Suggested-by: default avatarWillem de Bruijn <willemdebruijn.kernel@gmail.com>
      Signed-off-by: default avatarJakub Sitnicki <jakub@cloudflare.com>
      Reviewed-by: default avatarWillem de Bruijn <willemb@google.com>
      Link: https://patch.msgid.link/20240808-udp-gso-egress-from-tunnel-v4-1-f5c5b4149ab9@cloudflare.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      2b2bc3ba
    • Eric Dumazet's avatar
      gtp: pull network headers in gtp_dev_xmit() · 3a3be7ff
      Eric Dumazet authored
      syzbot/KMSAN reported use of uninit-value in get_dev_xmit() [1]
      
      We must make sure the IPv4 or Ipv6 header is pulled in skb->head
      before accessing fields in them.
      
      Use pskb_inet_may_pull() to fix this issue.
      
      [1]
      BUG: KMSAN: uninit-value in ipv6_pdp_find drivers/net/gtp.c:220 [inline]
       BUG: KMSAN: uninit-value in gtp_build_skb_ip6 drivers/net/gtp.c:1229 [inline]
       BUG: KMSAN: uninit-value in gtp_dev_xmit+0x1424/0x2540 drivers/net/gtp.c:1281
        ipv6_pdp_find drivers/net/gtp.c:220 [inline]
        gtp_build_skb_ip6 drivers/net/gtp.c:1229 [inline]
        gtp_dev_xmit+0x1424/0x2540 drivers/net/gtp.c:1281
        __netdev_start_xmit include/linux/netdevice.h:4913 [inline]
        netdev_start_xmit include/linux/netdevice.h:4922 [inline]
        xmit_one net/core/dev.c:3580 [inline]
        dev_hard_start_xmit+0x247/0xa20 net/core/dev.c:3596
        __dev_queue_xmit+0x358c/0x5610 net/core/dev.c:4423
        dev_queue_xmit include/linux/netdevice.h:3105 [inline]
        packet_xmit+0x9c/0x6c0 net/packet/af_packet.c:276
        packet_snd net/packet/af_packet.c:3145 [inline]
        packet_sendmsg+0x90e3/0xa3a0 net/packet/af_packet.c:3177
        sock_sendmsg_nosec net/socket.c:730 [inline]
        __sock_sendmsg+0x30f/0x380 net/socket.c:745
        __sys_sendto+0x685/0x830 net/socket.c:2204
        __do_sys_sendto net/socket.c:2216 [inline]
        __se_sys_sendto net/socket.c:2212 [inline]
        __x64_sys_sendto+0x125/0x1d0 net/socket.c:2212
        x64_sys_call+0x3799/0x3c10 arch/x86/include/generated/asm/syscalls_64.h:45
        do_syscall_x64 arch/x86/entry/common.c:52 [inline]
        do_syscall_64+0xcd/0x1e0 arch/x86/entry/common.c:83
       entry_SYSCALL_64_after_hwframe+0x77/0x7f
      
      Uninit was created at:
        slab_post_alloc_hook mm/slub.c:3994 [inline]
        slab_alloc_node mm/slub.c:4037 [inline]
        kmem_cache_alloc_node_noprof+0x6bf/0xb80 mm/slub.c:4080
        kmalloc_reserve+0x13d/0x4a0 net/core/skbuff.c:583
        __alloc_skb+0x363/0x7b0 net/core/skbuff.c:674
        alloc_skb include/linux/skbuff.h:1320 [inline]
        alloc_skb_with_frags+0xc8/0xbf0 net/core/skbuff.c:6526
        sock_alloc_send_pskb+0xa81/0xbf0 net/core/sock.c:2815
        packet_alloc_skb net/packet/af_packet.c:2994 [inline]
        packet_snd net/packet/af_packet.c:3088 [inline]
        packet_sendmsg+0x749c/0xa3a0 net/packet/af_packet.c:3177
        sock_sendmsg_nosec net/socket.c:730 [inline]
        __sock_sendmsg+0x30f/0x380 net/socket.c:745
        __sys_sendto+0x685/0x830 net/socket.c:2204
        __do_sys_sendto net/socket.c:2216 [inline]
        __se_sys_sendto net/socket.c:2212 [inline]
        __x64_sys_sendto+0x125/0x1d0 net/socket.c:2212
        x64_sys_call+0x3799/0x3c10 arch/x86/include/generated/asm/syscalls_64.h:45
        do_syscall_x64 arch/x86/entry/common.c:52 [inline]
        do_syscall_64+0xcd/0x1e0 arch/x86/entry/common.c:83
       entry_SYSCALL_64_after_hwframe+0x77/0x7f
      
      CPU: 0 UID: 0 PID: 7115 Comm: syz.1.515 Not tainted 6.11.0-rc1-syzkaller-00043-g94ede2a3 #0
      Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 06/27/2024
      
      Fixes: 999cb275 ("gtp: add IPv6 support")
      Fixes: 459aa660 ("gtp: add initial driver for datapath of GPRS Tunneling Protocol (GTP-U)")
      Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
      Cc: Harald Welte <laforge@gnumonks.org>
      Reviewed-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
      Link: https://patch.msgid.link/20240808132455.3413916-1-edumazet@google.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      3a3be7ff
  2. 09 Aug, 2024 5 commits
    • Foster Snowhill's avatar
      usbnet: ipheth: fix carrier detection in modes 1 and 4 · 67927a1b
      Foster Snowhill authored
      Apart from the standard "configurations", "interfaces" and "alternate
      interface settings" in USB, iOS devices also have a notion of
      "modes". In different modes, the device exposes a different set of
      available configurations.
      
      Depending on the iOS version, and depending on the current mode, the
      length and contents of the carrier state control message differs:
      
      * 1 byte (seen on iOS 4.2.1, 8.4):
          * 03: carrier off (mode 0)
          * 04: carrier on (mode 0)
      * 3 bytes (seen on iOS 10.3.4, 15.7.6):
          * 03 03 03: carrier off (mode 0)
          * 04 04 03: carrier on (mode 0)
      * 4 bytes (seen on iOS 16.5, 17.6):
          * 03 03 03 00: carrier off (mode 0)
          * 04 03 03 00: carrier off (mode 1)
          * 06 03 03 00: carrier off (mode 4)
          * 04 04 03 04: carrier on (mode 0 and 1)
          * 06 04 03 04: carrier on (mode 4)
      
      Before this change, the driver always used the first byte of the
      response to determine carrier state.
      
      From this larger sample, the first byte seems to indicate the number of
      available USB configurations in the current mode (with the exception of
      the default mode 0), and in some cases (namely mode 1 and 4) does not
      correlate with the carrier state.
      
      Previous logic erroneously counted `04 03 03 00` as "carrier on" and
      `06 04 03 04` as "carrier off" on iOS versions that support mode 1 and
      mode 4 respectively.
      
      Only modes 0, 1 and 4 expose the USB Ethernet interfaces necessary for
      the ipheth driver.
      
      Check the second byte of the control message where possible, and fall
      back to checking the first byte on older iOS versions.
      Signed-off-by: default avatarFoster Snowhill <forst@pen.gy>
      Tested-by: default avatarGeorgi Valkov <gvalkov@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      67927a1b
    • Foster Snowhill's avatar
      usbnet: ipheth: do not stop RX on failing RX callback · 74efed51
      Foster Snowhill authored
      RX callbacks can fail for multiple reasons:
      
      * Payload too short
      * Payload formatted incorrecly (e.g. bad NCM framing)
      * Lack of memory
      
      None of these should cause the driver to seize up.
      
      Make such failures non-critical and continue processing further
      incoming URBs.
      Signed-off-by: default avatarFoster Snowhill <forst@pen.gy>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      74efed51
    • Foster Snowhill's avatar
      usbnet: ipheth: drop RX URBs with no payload · 94d7eeb6
      Foster Snowhill authored
      On iPhone 15 Pro Max one can observe periodic URBs with no payload
      on the "bulk in" (RX) endpoint. These don't seem to do anything
      meaningful. Reproduced on iOS 17.5.1 and 17.6.
      
      This behaviour isn't observed on iPhone 11 on the same iOS version. The
      nature of these zero-length URBs is so far unknown.
      
      Drop RX URBs with no payload.
      Signed-off-by: default avatarFoster Snowhill <forst@pen.gy>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      94d7eeb6
    • Foster Snowhill's avatar
      usbnet: ipheth: remove extraneous rx URB length check · 655b46d7
      Foster Snowhill authored
      Rx URB length was already checked in ipheth_rcvbulk_callback_legacy()
      and ipheth_rcvbulk_callback_ncm(), depending on the current mode.
      The check in ipheth_rcvbulk_callback() was thus mostly a duplicate.
      
      The only place in ipheth_rcvbulk_callback() where we care about the URB
      length is for the initial control frame. These frames are always 4 bytes
      long. This has been checked as far back as iOS 4.2.1 on iPhone 3G.
      
      Remove the extraneous URB length check. For control frames, check for
      the specific 4-byte length instead.
      Signed-off-by: default avatarFoster Snowhill <forst@pen.gy>
      Tested-by: default avatarGeorgi Valkov <gvalkov@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      655b46d7
    • Oliver Neukum's avatar
      usbnet: ipheth: race between ipheth_close and error handling · e5876b08
      Oliver Neukum authored
      ipheth_sndbulk_callback() can submit carrier_work
      as a part of its error handling. That means that
      the driver must make sure that the work is cancelled
      after it has made sure that no more URB can terminate
      with an error condition.
      
      Hence the order of actions in ipheth_close() needs
      to be inverted.
      Signed-off-by: default avatarOliver Neukum <oneukum@suse.com>
      Signed-off-by: default avatarFoster Snowhill <forst@pen.gy>
      Tested-by: default avatarGeorgi Valkov <gvalkov@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      e5876b08
  3. 08 Aug, 2024 32 commits