An audit of users of gso_size reveals that an eBPF tc action on a
veth pair can be passed an SCTP GSO skb, which has gso_size of
GSO_BY_FRAGS.

If that action calls bpf_skb_change_proto(), bpf_skb_net_grow()
or bpf_skb_net_shrink(), the gso_size will be unconditionally
incremented or decremented to some nonsense value.

Add helpers that WARN if attempting to change a gso_size of a
GSO_BY_FRAGS skb (and leave the value unchanged).

Signed-off-by: Daniel Axtens <d...@axtens.net>
---
 Documentation/networking/segmentation-offloads.txt | 11 +++++++++--
 include/linux/skbuff.h                             | 16 ++++++++++++++++
 net/core/filter.c                                  |  8 ++++----
 3 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/Documentation/networking/segmentation-offloads.txt 
b/Documentation/networking/segmentation-offloads.txt
index d47480b61ac6..23a8dd91a9ec 100644
--- a/Documentation/networking/segmentation-offloads.txt
+++ b/Documentation/networking/segmentation-offloads.txt
@@ -153,8 +153,15 @@ To signal this, gso_size is set to the special value 
GSO_BY_FRAGS.
 
 Therefore, any code in the core networking stack must be aware of the
 possibility that gso_size will be GSO_BY_FRAGS and handle that case
-appropriately. (For size checks, the skb_gso_validate_*_len family of
-helpers do this automatically.)
+appropriately.
+
+There are a couple of helpers to make this easier:
+
+ - For size checks, the skb_gso_validate_*_len family of helpers correctly
+   considers GSO_BY_FRAGS.
+
+ - For manipulating packets, skb_increase_gso_size and skb_decrease_gso_size
+   will check for GSO_BY_FRAGS and WARN if asked to manipulate these skbs.
 
 This also affects drivers with the NETIF_F_FRAGLIST & NETIF_F_GSO_SCTP bits
 set. Note also that NETIF_F_GSO_SCTP is included in NETIF_F_GSO_SOFTWARE.
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index d8340e6e8814..157a91864e16 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -4047,6 +4047,22 @@ static inline void skb_gso_reset(struct sk_buff *skb)
        skb_shinfo(skb)->gso_type = 0;
 }
 
+static inline void skb_increase_gso_size(struct sk_buff *skb, u16 increment)
+{
+       if (WARN_ON_ONCE(skb_shinfo(skb)->gso_size == GSO_BY_FRAGS))
+               return;
+
+       skb_shinfo(skb)->gso_size += increment;
+}
+
+static inline void skb_decrease_gso_size(struct sk_buff *skb, u16 decrement)
+{
+       if (WARN_ON_ONCE(skb_shinfo(skb)->gso_size == GSO_BY_FRAGS))
+               return;
+
+       skb_shinfo(skb)->gso_size -= decrement;
+}
+
 void __skb_warn_lro_forwarding(const struct sk_buff *skb);
 
 static inline bool skb_warn_if_lro(const struct sk_buff *skb)
diff --git a/net/core/filter.c b/net/core/filter.c
index 0c121adbdbaa..d9684d8a3ac7 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2105,7 +2105,7 @@ static int bpf_skb_proto_4_to_6(struct sk_buff *skb)
                }
 
                /* Due to IPv6 header, MSS needs to be downgraded. */
-               skb_shinfo(skb)->gso_size -= len_diff;
+               skb_decrease_gso_size(skb, len_diff);
                /* Header must be checked, and gso_segs recomputed. */
                skb_shinfo(skb)->gso_type |= SKB_GSO_DODGY;
                skb_shinfo(skb)->gso_segs = 0;
@@ -2141,7 +2141,7 @@ static int bpf_skb_proto_6_to_4(struct sk_buff *skb)
                }
 
                /* Due to IPv4 header, MSS can be upgraded. */
-               skb_shinfo(skb)->gso_size += len_diff;
+               skb_increase_gso_size(skb, len_diff);
                /* Header must be checked, and gso_segs recomputed. */
                skb_shinfo(skb)->gso_type |= SKB_GSO_DODGY;
                skb_shinfo(skb)->gso_segs = 0;
@@ -2253,7 +2253,7 @@ static int bpf_skb_net_grow(struct sk_buff *skb, u32 
len_diff)
 
        if (skb_is_gso(skb)) {
                /* Due to header grow, MSS needs to be downgraded. */
-               skb_shinfo(skb)->gso_size -= len_diff;
+               skb_decrease_gso_size(skb, len_diff);
                /* Header must be checked, and gso_segs recomputed. */
                skb_shinfo(skb)->gso_type |= SKB_GSO_DODGY;
                skb_shinfo(skb)->gso_segs = 0;
@@ -2277,7 +2277,7 @@ static int bpf_skb_net_shrink(struct sk_buff *skb, u32 
len_diff)
 
        if (skb_is_gso(skb)) {
                /* Due to header shrink, MSS can be upgraded. */
-               skb_shinfo(skb)->gso_size += len_diff;
+               skb_increase_gso_size(skb, len_diff);
                /* Header must be checked, and gso_segs recomputed. */
                skb_shinfo(skb)->gso_type |= SKB_GSO_DODGY;
                skb_shinfo(skb)->gso_segs = 0;
-- 
2.14.1

Reply via email to