If a packet with bad checksum metadata gets into the ipv4 stack,
skb_checksum_help can corrupt the network header and cause a bunch of
mischief.
This was discovered and reported by Paulos, and has been reporoduced
by others independently since.
We really shouldn't allow such packets in, but as a defence
in depth measure, let's also check before we complete the checksum.
A more complete validation at input is forthcoming, but needs more work.
Assisted-by: LLM
Fixes: f43798c27684 ("tun: Allow GSO using virtio_net_hdr")
Fixes: bfd5f4a3d605 ("packet: Add GSO/csum offload support.")
Reported-by: Paulos Yibelo <[email protected]>
Closes:
https://lore.kernel.org/netdev/[email protected]/
Signed-off-by: Michael S. Tsirkin <[email protected]>
---
Lightly tested.
include/net/ip.h | 7 +++++++
net/bridge/netfilter/nf_conntrack_bridge.c | 10 +++++++---
net/ipv4/ip_output.c | 10 +++++++---
net/netfilter/nfnetlink_queue.c | 7 +++++++
net/netfilter/xt_CHECKSUM.c | 6 +++++-
net/xfrm/xfrm_output.c | 16 +++++++++++-----
6 files changed, 44 insertions(+), 12 deletions(-)
diff --git a/include/net/ip.h b/include/net/ip.h
index 6f602df72ee6..d07c2c573024 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -74,6 +74,13 @@ static inline unsigned int ip_hdrlen(const struct sk_buff
*skb)
return ip_hdr(skb)->ihl * 4;
}
+static inline int ip_check_csum_start(const struct sk_buff *skb)
+{
+ if (unlikely(skb->csum_start < skb->network_header + ip_hdrlen(skb)))
+ return -EINVAL;
+ return 0;
+}
+
struct ipcm_cookie {
struct sockcm_cookie sockc;
__be32 addr;
diff --git a/net/bridge/netfilter/nf_conntrack_bridge.c
b/net/bridge/netfilter/nf_conntrack_bridge.c
index 7ecb8a26bfa3..b5444335b86f 100644
--- a/net/bridge/netfilter/nf_conntrack_bridge.c
+++ b/net/bridge/netfilter/nf_conntrack_bridge.c
@@ -39,9 +39,13 @@ static int nf_br_ip_fragment(struct net *net, struct sock
*sk,
int err = 0;
/* for offloaded checksums cleanup checksum before fragmentation */
- if (skb->ip_summed == CHECKSUM_PARTIAL &&
- (err = skb_checksum_help(skb)))
- goto blackhole;
+ if (skb->ip_summed == CHECKSUM_PARTIAL) {
+ err = ip_check_csum_start(skb);
+ if (!err)
+ err = skb_checksum_help(skb);
+ if (err)
+ goto blackhole;
+ }
iph = ip_hdr(skb);
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 74e095b6b7ca..bea79611617f 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -771,9 +771,13 @@ int ip_do_fragment(struct net *net, struct sock *sk,
struct sk_buff *skb,
int err = 0;
/* for offloaded checksums cleanup checksum before fragmentation */
- if (skb->ip_summed == CHECKSUM_PARTIAL &&
- (err = skb_checksum_help(skb)))
- goto fail;
+ if (skb->ip_summed == CHECKSUM_PARTIAL) {
+ err = ip_check_csum_start(skb);
+ if (!err)
+ err = skb_checksum_help(skb);
+ if (err)
+ goto fail;
+ }
/*
* Point into the IP datagram header.
diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index c727668b0c5b..5177e27174e7 100644
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -40,6 +40,7 @@
#include <linux/udp.h>
#include <net/gre.h>
#include <net/gso.h>
+#include <net/ip.h>
#include <net/sock.h>
#include <net/tcp_states.h>
#include <net/netfilter/nf_queue.h>
@@ -674,6 +675,12 @@ static int nfqnl_put_bridge(struct nf_queue_entry *entry,
struct sk_buff *skb)
static int nf_queue_checksum_help(struct sk_buff *entskb)
{
+ if (entskb->protocol == htons(ETH_P_IP)) {
+ if (!pskb_network_may_pull(entskb, sizeof(struct iphdr)) ||
+ ip_check_csum_start(entskb))
+ return -EINVAL;
+ }
+
if (skb_csum_is_sctp(entskb))
return skb_crc32c_csum_help(entskb);
diff --git a/net/netfilter/xt_CHECKSUM.c b/net/netfilter/xt_CHECKSUM.c
index 9d99f5a3d176..1fb0f8118404 100644
--- a/net/netfilter/xt_CHECKSUM.c
+++ b/net/netfilter/xt_CHECKSUM.c
@@ -15,6 +15,7 @@
#include <linux/netfilter_ipv4/ip_tables.h>
#include <linux/netfilter_ipv6/ip6_tables.h>
+#include <net/ip.h>
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Michael S. Tsirkin <[email protected]>");
@@ -25,8 +26,11 @@ MODULE_ALIAS("ip6t_CHECKSUM");
static unsigned int
checksum_tg(struct sk_buff *skb, const struct xt_action_param *par)
{
- if (skb->ip_summed == CHECKSUM_PARTIAL && !skb_is_gso(skb))
+ if (skb->ip_summed == CHECKSUM_PARTIAL && !skb_is_gso(skb)) {
+ if (xt_family(par) == NFPROTO_IPV4 && ip_check_csum_start(skb))
+ return NF_DROP;
skb_checksum_help(skb);
+ }
return XT_CONTINUE;
}
diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
index e305ba32e356..f1f612cd7b43 100644
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c
@@ -823,16 +823,22 @@ int xfrm_output(struct sock *sk, struct sk_buff *skb)
}
if (skb->ip_summed == CHECKSUM_PARTIAL) {
- err = skb_checksum_help(skb);
- if (err) {
- XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTERROR);
- kfree_skb(skb);
- return err;
+ if (skb->protocol == htons(ETH_P_IP)) {
+ err = ip_check_csum_start(skb);
+ if (err)
+ goto error;
}
+ err = skb_checksum_help(skb);
+ if (err)
+ goto error;
}
out:
return xfrm_output2(net, sk, skb);
+error:
+ XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTERROR);
+ kfree_skb(skb);
+ return err;
}
EXPORT_SYMBOL_GPL(xfrm_output);
--
MST
