netdev - [PATCH 5/6] net: add and use helpers when adjusting gso

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20180227130407.22498-6-dja@axtens.net>
Date:   Wed, 28 Feb 2018 00:04:06 +1100
From:   Daniel Axtens <dja@...ens.net>
To:     netdev@...r.kernel.org
Cc:     Daniel Axtens <dja@...ens.net>
Subject: [PATCH 5/6] net: add and use helpers when adjusting gso_size

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 <dja@...ens.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