ip_do_fragment() and nf_br_ip_fragment() complete a CHECKSUM_PARTIAL skb
before reading the IPv4 header length. ip6_fragment() and br_ip6_fragment()
complete one after parsing the IPv6 header chain. A virtualization
interface can supply a checksum start which, after link-layer removal,
still points inside that parsed network header.
skb_checksum_help() then writes the completed checksum into header bytes
the stack has already consumed. For IPv4, changing iph->ihl after routing
and validation can make fragmentation copy beyond the skb's logical linear
head into transmitted options. A negative checksum-start offset is rejected
by skb_checksum_help(), but only after a WARN_ONCE which can panic a
panic_on_warn system.
Validate the checksum start against the parsed header length before
completing it. For IPv4, read and validate IHL first, retain it, and
reacquire iph after skb_checksum_help() in both implementations. For IPv6,
use the length returned by ip6_find_1stfragopt() in both implementations.
Compare the signed checksum-start offset with the bounded signed header
length so integer promotion cannot bypass either boundary.
Fixes: dbd3393c56a8 ("ipv4: add defensive check for CHECKSUM_PARTIAL skbs in
ip_fragment")
Fixes: 405c92f7a541 ("ipv6: add defensive check for CHECKSUM_PARTIAL skbs in
ip_fragment")
Fixes: 3c171f496ef5 ("netfilter: bridge: add connection tracking system")
Fixes: 764dd163ac92 ("netfilter: nf_conntrack_bridge: add support for IPv6")
Reported-by: Paulos Yibelo <[email protected]>
Link:
https://lore.kernel.org/netdev/[email protected]/
Cc: [email protected]
Signed-off-by: Paulos Yibelo <[email protected]>
---
Changes in v5:
- Compare the checksum-start offset and IPv4 header length as signed
values.
- Add parsed-header checks to the IPv4/IPv6 output and bridge-netfilter
fragmentation paths.
- Drop the prior Acked-by and Reviewed-by tags because the code changed.
Changes in v4:
- State that a TUN device is sufficient and no guest is required, as
noted by Michael S. Tsirkin.
Changes in v3:
- No code changes.
Changes in v2:
- No code changes.
net/bridge/netfilter/nf_conntrack_bridge.c | 21 +++++++++++++++-----
net/ipv4/ip_output.c | 23 ++++++++++++++++------
net/ipv6/ip6_output.c | 12 ++++++++---
net/ipv6/netfilter.c | 12 ++++++++---
4 files changed, 51 insertions(+), 17 deletions(-)
diff --git a/net/bridge/netfilter/nf_conntrack_bridge.c
b/net/bridge/netfilter/nf_conntrack_bridge.c
index 7ecb8a26b..d81ed8692 100644
--- a/net/bridge/netfilter/nf_conntrack_bridge.c
+++ b/net/bridge/netfilter/nf_conntrack_bridge.c
@@ -38,18 +38,29 @@ static int nf_br_ip_fragment(struct net *net, struct sock
*sk,
struct iphdr *iph;
int err = 0;
- /* for offloaded checksums cleanup checksum before fragmentation */
- if (skb->ip_summed == CHECKSUM_PARTIAL &&
- (err = skb_checksum_help(skb)))
+ iph = ip_hdr(skb);
+ hlen = iph->ihl * 4;
+ if (unlikely(hlen < sizeof(*iph) || hlen > skb_headlen(skb))) {
+ err = -EINVAL;
goto blackhole;
+ }
- iph = ip_hdr(skb);
+ /* Complete offloaded checksums only after the validated IP header. */
+ if (skb->ip_summed == CHECKSUM_PARTIAL) {
+ if (unlikely(skb_checksum_start_offset(skb) < (int)hlen)) {
+ err = -EINVAL;
+ goto blackhole;
+ }
+ err = skb_checksum_help(skb);
+ if (err)
+ goto blackhole;
+ iph = ip_hdr(skb);
+ }
/*
* Setup starting values
*/
- hlen = iph->ihl * 4;
frag_max_size -= hlen;
ll_rs = LL_RESERVED_SPACE(skb->dev);
mtu = skb->dev->mtu;
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index a24cc8ee1..fa6a74d20 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -770,16 +770,28 @@ int ip_do_fragment(struct net *net, struct sock *sk,
struct sk_buff *skb,
struct ip_frag_state state;
int err = 0;
- /* for offloaded checksums cleanup checksum before fragmentation */
- if (skb->ip_summed == CHECKSUM_PARTIAL &&
- (err = skb_checksum_help(skb)))
- goto fail;
-
/*
* Point into the IP datagram header.
*/
iph = ip_hdr(skb);
+ hlen = iph->ihl * 4;
+ if (unlikely(hlen < sizeof(*iph) || hlen > skb_headlen(skb))) {
+ err = -EINVAL;
+ goto fail;
+ }
+
+ /* Complete offloaded checksums only after the validated IP header. */
+ if (skb->ip_summed == CHECKSUM_PARTIAL) {
+ if (unlikely(skb_checksum_start_offset(skb) < (int)hlen)) {
+ err = -EINVAL;
+ goto fail;
+ }
+ err = skb_checksum_help(skb);
+ if (err)
+ goto fail;
+ iph = ip_hdr(skb);
+ }
mtu = ip_skb_dst_mtu(sk, skb);
if (IPCB(skb)->frag_max_size && IPCB(skb)->frag_max_size < mtu)
@@ -789,7 +801,6 @@ int ip_do_fragment(struct net *net, struct sock *sk, struct
sk_buff *skb,
* Setup starting values.
*/
- hlen = iph->ihl * 4;
if (mtu < hlen + 8) {
err = -EMSGSIZE;
goto fail;
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 550965058..d157b6ade 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -942,9 +942,15 @@ int ip6_fragment(struct net *net, struct sock *sk, struct
sk_buff *skb,
frag_id = ipv6_select_ident(net, &ipv6_hdr(skb)->daddr,
&ipv6_hdr(skb)->saddr);
- if (skb->ip_summed == CHECKSUM_PARTIAL &&
- (err = skb_checksum_help(skb)))
- goto fail;
+ if (skb->ip_summed == CHECKSUM_PARTIAL) {
+ if (unlikely(skb_checksum_start_offset(skb) < (int)hlen)) {
+ err = -EINVAL;
+ goto fail;
+ }
+ err = skb_checksum_help(skb);
+ if (err)
+ goto fail;
+ }
prevhdr = skb_network_header(skb) + nexthdr_offset;
hroom = LL_RESERVED_SPACE(rt->dst.dev);
diff --git a/net/ipv6/netfilter.c b/net/ipv6/netfilter.c
index a7025ec87..da7ada12f 100644
--- a/net/ipv6/netfilter.c
+++ b/net/ipv6/netfilter.c
@@ -144,9 +144,15 @@ int br_ip6_fragment(struct net *net, struct sock *sk,
struct sk_buff *skb,
frag_id = ipv6_select_ident(net, &ipv6_hdr(skb)->daddr,
&ipv6_hdr(skb)->saddr);
- if (skb->ip_summed == CHECKSUM_PARTIAL &&
- (err = skb_checksum_help(skb)))
- goto blackhole;
+ if (skb->ip_summed == CHECKSUM_PARTIAL) {
+ if (unlikely(skb_checksum_start_offset(skb) < (int)hlen)) {
+ err = -EINVAL;
+ goto blackhole;
+ }
+ err = skb_checksum_help(skb);
+ if (err)
+ goto blackhole;
+ }
prevhdr = skb_network_header(skb) + nexthdr_offset;
hroom = LL_RESERVED_SPACE(skb->dev);
--
2.46.0
