queue-6.8/net-mpls-error-out-if-inner-headers-are-not-set.patch

   1 From 025f8ad20f2e3264d11683aa9cbbf0083eefbdcd Mon Sep 17 00:00:00 2001
   2 From: Florian Westphal <fw@strlen.de>
   3 Date: Thu, 22 Feb 2024 15:03:10 +0100
   4 Subject: net: mpls: error out if inner headers are not set
   5
   6 From: Florian Westphal <fw@strlen.de>
   7
   8 commit 025f8ad20f2e3264d11683aa9cbbf0083eefbdcd upstream.
   9
  10 mpls_gso_segment() assumes skb_inner_network_header() returns
  11 a valid result:
  12
  13   mpls_hlen = skb_inner_network_header(skb) - skb_network_header(skb);
  14   if (unlikely(!mpls_hlen || mpls_hlen % MPLS_HLEN))
  15         goto out;
  16   if (unlikely(!pskb_may_pull(skb, mpls_hlen)))
  17
  18 With syzbot reproducer, skb_inner_network_header() yields 0,
  19 skb_network_header() returns 108, so this will
  20 "pskb_may_pull(skb, -108)))" which triggers a newly added
  21 DEBUG_NET_WARN_ON_ONCE() check:
  22
  23 ------------[ cut here ]------------
  24 WARNING: CPU: 0 PID: 5068 at include/linux/skbuff.h:2723 pskb_may_pull_reason include/linux/skbuff.h:2723 [inline]
  25 WARNING: CPU: 0 PID: 5068 at include/linux/skbuff.h:2723 pskb_may_pull include/linux/skbuff.h:2739 [inline]
  26 WARNING: CPU: 0 PID: 5068 at include/linux/skbuff.h:2723 mpls_gso_segment+0x773/0xaa0 net/mpls/mpls_gso.c:34
  27 [..]
  28  skb_mac_gso_segment+0x383/0x740 net/core/gso.c:53
  29  nsh_gso_segment+0x40a/0xad0 net/nsh/nsh.c:108
  30  skb_mac_gso_segment+0x383/0x740 net/core/gso.c:53
  31  __skb_gso_segment+0x324/0x4c0 net/core/gso.c:124
  32  skb_gso_segment include/net/gso.h:83 [inline]
  33  [..]
  34  sch_direct_xmit+0x11a/0x5f0 net/sched/sch_generic.c:327
  35  [..]
  36  packet_sendmsg+0x46a9/0x6130 net/packet/af_packet.c:3113
  37  [..]
  38
  39 First iteration of this patch made mpls_hlen signed and changed
  40 test to error out to "mpls_hlen <= 0 || ..".
  41
  42 Eric Dumazet said:
  43  > I was thinking about adding a debug check in skb_inner_network_header()
  44  > if inner_network_header is zero (that would mean it is not 'set' yet),
  45  > but this would trigger even after your patch.
  46
  47 So add new skb_inner_network_header_was_set() helper and use that.
  48
  49 The syzbot reproducer injects data via packet socket. The skb that gets
  50 allocated and passed down the stack has ->protocol set to NSH (0x894f)
  51 and gso_type set to SKB_GSO_UDP | SKB_GSO_DODGY.
  52
  53 This gets passed to skb_mac_gso_segment(), which sees NSH as ptype to
  54 find a callback for.  nsh_gso_segment() retrieves next type:
  55
  56         proto = tun_p_to_eth_p(nsh_hdr(skb)->np);
  57
  58 ... which is MPLS (TUN_P_MPLS_UC). It updates skb->protocol and then
  59 calls mpls_gso_segment().  Inner offsets are all 0, so mpls_gso_segment()
  60 ends up with a negative header size.
  61
  62 In case more callers rely on silent handling of such large may_pull values
  63 we could also 'legalize' this behaviour, either replacing the debug check
  64 with (len > INT_MAX) test or removing it and instead adding a comment
  65 before existing
  66
  67  if (unlikely(len > skb->len))
  68     return SKB_DROP_REASON_PKT_TOO_SMALL;
  69
  70 test in pskb_may_pull_reason(), saying that this check also implicitly
  71 takes care of callers that miscompute header sizes.
  72
  73 Cc: Simon Horman <horms@kernel.org>
  74 Fixes: 219eee9c0d16 ("net: skbuff: add overflow debug check to pull/push helpers")
  75 Reported-by: syzbot+99d15fcdb0132a1e1a82@syzkaller.appspotmail.com
  76 Closes: https://lore.kernel.org/netdev/00000000000043b1310611e388aa@google.com/raw
  77 Signed-off-by: Florian Westphal <fw@strlen.de>
  78 Link: https://lore.kernel.org/r/20240222140321.14080-1-fw@strlen.de
  79 Signed-off-by: Jakub Kicinski <kuba@kernel.org>
  80 Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  81 ---
  82  include/linux/skbuff.h |    5 +++++
  83  net/mpls/mpls_gso.c    |    3 +++
  84  2 files changed, 8 insertions(+)
  85
  86 --- a/include/linux/skbuff.h
  87 +++ b/include/linux/skbuff.h
  88 @@ -2847,6 +2847,11 @@ static inline void skb_set_inner_network
  89         skb->inner_network_header += offset;
  90  }
  91
  92 +static inline bool skb_inner_network_header_was_set(const struct sk_buff *skb)
  93 +{
  94 +       return skb->inner_network_header > 0;
  95 +}
  96 +
  97  static inline unsigned char *skb_inner_mac_header(const struct sk_buff *skb)
  98  {
  99         return skb->head + skb->inner_mac_header;
 100 --- a/net/mpls/mpls_gso.c
 101 +++ b/net/mpls/mpls_gso.c
 102 @@ -27,6 +27,9 @@ static struct sk_buff *mpls_gso_segment(
 103         __be16 mpls_protocol;
 104         unsigned int mpls_hlen;
 105
 106 +       if (!skb_inner_network_header_was_set(skb))
 107 +               goto out;
 108 +
 109         skb_reset_network_header(skb);
 110         mpls_hlen = skb_inner_network_header(skb) - skb_network_header(skb);
 111         if (unlikely(!mpls_hlen || mpls_hlen % MPLS_HLEN))