diff mbox series

[stable,4.4,06/11] ipv6: defrag: drop non-last frags smaller than min mtu

Message ID 1548209986-83527-7-git-send-email-maowenan@huawei.com
State Not Applicable
Delegated to: BPF Maintainers
Headers show
Series fix FragmentSmack in stable branch (CVE-2018-5391) | expand

Commit Message

maowenan Jan. 23, 2019, 2:19 a.m. UTC
From: Florian Westphal <fw@strlen.de>

[ Upstream commit 0ed4229b08c13c84a3c301a08defdc9e7f4467e6 ]

don't bother with pathological cases, they only waste cycles.
IPv6 requires a minimum MTU of 1280 so we should never see fragments
smaller than this (except last frag).

v3: don't use awkward "-offset + len"
v2: drop IPv4 part, which added same check w. IPV4_MIN_MTU (68).
    There were concerns that there could be even smaller frags
    generated by intermediate nodes, e.g. on radio networks.

Cc: Peter Oskolkov <posk@google.com>
Cc: Eric Dumazet <edumazet@google.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 net/ipv6/netfilter/nf_conntrack_reasm.c | 4 ++++
 net/ipv6/reassembly.c                   | 4 ++++
 2 files changed, 8 insertions(+)

Comments

Greg KH Jan. 24, 2019, 6:31 p.m. UTC | #1
On Wed, Jan 23, 2019 at 10:19:41AM +0800, Mao Wenan wrote:
> From: Florian Westphal <fw@strlen.de>
> 
> [ Upstream commit 0ed4229b08c13c84a3c301a08defdc9e7f4467e6 ]
> 
> don't bother with pathological cases, they only waste cycles.
> IPv6 requires a minimum MTU of 1280 so we should never see fragments
> smaller than this (except last frag).
> 
> v3: don't use awkward "-offset + len"
> v2: drop IPv4 part, which added same check w. IPV4_MIN_MTU (68).
>     There were concerns that there could be even smaller frags
>     generated by intermediate nodes, e.g. on radio networks.
> 
> Cc: Peter Oskolkov <posk@google.com>
> Cc: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> ---
>  net/ipv6/netfilter/nf_conntrack_reasm.c | 4 ++++
>  net/ipv6/reassembly.c                   | 4 ++++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
> index 9cd8863..c5033a2 100644
> --- a/net/ipv6/netfilter/nf_conntrack_reasm.c
> +++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
> @@ -602,6 +602,10 @@ struct sk_buff *nf_ct_frag6_gather(struct net *net, struct sk_buff *skb, u32 use
>  	hdr = ipv6_hdr(clone);
>  	fhdr = (struct frag_hdr *)skb_transport_header(clone);
>  
> +	if (skb->len - skb_network_offset(skb) < IPV6_MIN_MTU &&
> +	    fhdr->frag_off & htons(IP6_MF))
> +		return -EINVAL;

This backport is incorrect, you should be returning a pointer, right?

How did you test this?  This should have blown up under test :(

I'm going to drop this whole series.  Please fix it up and test it
properly and then resend.

thanks,

greg k-h
maowenan Jan. 25, 2019, 2:24 a.m. UTC | #2
On 2019/1/25 2:31, Greg KH wrote:
> On Wed, Jan 23, 2019 at 10:19:41AM +0800, Mao Wenan wrote:
>> From: Florian Westphal <fw@strlen.de>
>>
>> [ Upstream commit 0ed4229b08c13c84a3c301a08defdc9e7f4467e6 ]
>>
>> don't bother with pathological cases, they only waste cycles.
>> IPv6 requires a minimum MTU of 1280 so we should never see fragments
>> smaller than this (except last frag).
>>
>> v3: don't use awkward "-offset + len"
>> v2: drop IPv4 part, which added same check w. IPV4_MIN_MTU (68).
>>     There were concerns that there could be even smaller frags
>>     generated by intermediate nodes, e.g. on radio networks.
>>
>> Cc: Peter Oskolkov <posk@google.com>
>> Cc: Eric Dumazet <edumazet@google.com>
>> Signed-off-by: Florian Westphal <fw@strlen.de>
>> Signed-off-by: David S. Miller <davem@davemloft.net>
>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
>> ---
>>  net/ipv6/netfilter/nf_conntrack_reasm.c | 4 ++++
>>  net/ipv6/reassembly.c                   | 4 ++++
>>  2 files changed, 8 insertions(+)
>>
>> diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
>> index 9cd8863..c5033a2 100644
>> --- a/net/ipv6/netfilter/nf_conntrack_reasm.c
>> +++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
>> @@ -602,6 +602,10 @@ struct sk_buff *nf_ct_frag6_gather(struct net *net, struct sk_buff *skb, u32 use
>>  	hdr = ipv6_hdr(clone);
>>  	fhdr = (struct frag_hdr *)skb_transport_header(clone);
>>  
>> +	if (skb->len - skb_network_offset(skb) < IPV6_MIN_MTU &&
>> +	    fhdr->frag_off & htons(IP6_MF))
>> +		return -EINVAL;
> 
> This backport is incorrect, you should be returning a pointer, right?

Thank you for correcting me, the return value should be a pointer,
I will fix it and test all of patches again, then resend v2.
sorry for my mistake.
> 
> How did you test this?  This should have blown up under test :(
> 
> I'm going to drop this whole series.  Please fix it up and test it
> properly and then resend.
> 
> thanks,
> 
> greg k-h
> 
> .
>
diff mbox series

Patch

diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
index 9cd8863..c5033a2 100644
--- a/net/ipv6/netfilter/nf_conntrack_reasm.c
+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
@@ -602,6 +602,10 @@  struct sk_buff *nf_ct_frag6_gather(struct net *net, struct sk_buff *skb, u32 use
 	hdr = ipv6_hdr(clone);
 	fhdr = (struct frag_hdr *)skb_transport_header(clone);
 
+	if (skb->len - skb_network_offset(skb) < IPV6_MIN_MTU &&
+	    fhdr->frag_off & htons(IP6_MF))
+		return -EINVAL;
+
 	skb_orphan(skb);
 	fq = fq_find(net, fhdr->identification, user, &hdr->saddr, &hdr->daddr,
 		     skb->dev ? skb->dev->ifindex : 0, ip6_frag_ecn(hdr));
diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
index adc7512..44c7f4c 100644
--- a/net/ipv6/reassembly.c
+++ b/net/ipv6/reassembly.c
@@ -549,6 +549,10 @@  static int ipv6_frag_rcv(struct sk_buff *skb)
 		return 1;
 	}
 
+	if (skb->len - skb_network_offset(skb) < IPV6_MIN_MTU &&
+	    fhdr->frag_off & htons(IP6_MF))
+		goto fail_hdr;
+
 	fq = fq_find(net, fhdr->identification, &hdr->saddr, &hdr->daddr,
 		     skb->dev ? skb->dev->ifindex : 0, ip6_frag_ecn(hdr));
 	if (fq) {