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
| ||
|
Message-ID: <CALx6S35aNoSgP5UyA70xE=3h_haWw8NU11ViaqUqG=uGbtzUgQ@mail.gmail.com> Date: Tue, 22 Jan 2019 16:37:45 -0800 From: Tom Herbert <tom@...bertland.com> To: Peter Oskolkov <posk@...gle.com> Cc: David Miller <davem@...emloft.net>, Linux Kernel Network Developers <netdev@...r.kernel.org>, Peter Oskolkov <posk.devel@...il.com>, Eric Dumazet <edumazet@...gle.com>, Florian Westphal <fw@...len.de> Subject: Re: [PATCH net-next 2/4] net: IP6 defrag: use rbtrees for IPv6 defrag On Tue, Jan 22, 2019 at 10:03 AM Peter Oskolkov <posk@...gle.com> wrote: > > Currently, IPv6 defragmentation code drops non-last fragments that > are smaller than 1280 bytes: see > commit 0ed4229b08c1 ("ipv6: defrag: drop non-last frags smaller than min mtu") > Peter, I'm a bit confused on why so much code is required to undo this patch which was a whole three lines of code to begin with. > This behavior is not specified in IPv6 RFCs and appears to break > compatibility with some IPv6 implemenations, as reported here: > https://www.spinics.net/lists/netdev/msg543846.html > I am not yet convinced that the patch should be reverted. While it is not conformant, it is a potential mitigation against DOS attack. For instance, a single fragmented packet could be composed of over 8000 fragments with the minimum fragment size of eight bytes. Receiving packets like that attacks both memory and CPU, and I don't see any purpose to sending tiny fragments other than DOS attack. For practical implemenation, I believe a limit is justified. The 1280 MTU limit in the patch was too austere, I think it should be smaller than that. Also, any such limit should apply to fragments payload length and not length of the packet since a clever attacker could stuff the packet with various other extension headers and still end up sending tiny fragments. I have a patch along these lines with limit configurable by sysctl. I will post shortly. Tom > This patch re-uses common IP defragmentation queueing and reassembly > code in IPv6, removing the 1280 byte restriction. > > Signed-off-by: Peter Oskolkov <posk@...gle.com> > Reported-by: Tom Herbert <tom@...bertland.com> > Cc: Eric Dumazet <edumazet@...gle.com> > Cc: Florian Westphal <fw@...len.de> > --- > include/net/ipv6_frag.h | 11 +- > net/ipv6/reassembly.c | 233 +++++++++++----------------------------- > 2 files changed, 71 insertions(+), 173 deletions(-) > > diff --git a/include/net/ipv6_frag.h b/include/net/ipv6_frag.h > index 6ced1e6899b6..28aa9b30aece 100644 > --- a/include/net/ipv6_frag.h > +++ b/include/net/ipv6_frag.h > @@ -82,8 +82,15 @@ ip6frag_expire_frag_queue(struct net *net, struct frag_queue *fq) > __IP6_INC_STATS(net, __in6_dev_get(dev), IPSTATS_MIB_REASMTIMEOUT); > > /* Don't send error if the first segment did not arrive. */ > - head = fq->q.fragments; > - if (!(fq->q.flags & INET_FRAG_FIRST_IN) || !head) > + if (!(fq->q.flags & INET_FRAG_FIRST_IN)) > + goto out; > + > + /* sk_buff::dev and sk_buff::rbnode are unionized. So we > + * pull the head out of the tree in order to be able to > + * deal with head->dev. > + */ > + head = inet_frag_pull_head(&fq->q); > + if (!head) > goto out; > > head->dev = dev; > diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c > index 36a3d8dc61f5..24264d0a4b85 100644 > --- a/net/ipv6/reassembly.c > +++ b/net/ipv6/reassembly.c > @@ -69,8 +69,8 @@ static u8 ip6_frag_ecn(const struct ipv6hdr *ipv6h) > > static struct inet_frags ip6_frags; > > -static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev, > - struct net_device *dev); > +static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *skb, > + struct sk_buff *prev_tail, struct net_device *dev); > > static void ip6_frag_expire(struct timer_list *t) > { > @@ -111,21 +111,26 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb, > struct frag_hdr *fhdr, int nhoff, > u32 *prob_offset) > { > - struct sk_buff *prev, *next; > - struct net_device *dev; > - int offset, end, fragsize; > struct net *net = dev_net(skb_dst(skb)->dev); > + int offset, end, fragsize; > + struct sk_buff *prev_tail; > + struct net_device *dev; > + int err = -ENOENT; > u8 ecn; > > if (fq->q.flags & INET_FRAG_COMPLETE) > goto err; > > + err = -EINVAL; > offset = ntohs(fhdr->frag_off) & ~0x7; > end = offset + (ntohs(ipv6_hdr(skb)->payload_len) - > ((u8 *)(fhdr + 1) - (u8 *)(ipv6_hdr(skb) + 1))); > > if ((unsigned int)end > IPV6_MAXPLEN) { > *prob_offset = (u8 *)&fhdr->frag_off - skb_network_header(skb); > + /* note that if prob_offset is set, the skb is freed elsewhere, > + * we do not free it here. > + */ > return -1; > } > > @@ -170,62 +175,27 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb, > if (end == offset) > goto discard_fq; > > + err = -ENOMEM; > /* Point into the IP datagram 'data' part. */ > if (!pskb_pull(skb, (u8 *) (fhdr + 1) - skb->data)) > goto discard_fq; > > - if (pskb_trim_rcsum(skb, end - offset)) > + err = pskb_trim_rcsum(skb, end - offset); > + if (err) > goto discard_fq; > > - /* Find out which fragments are in front and at the back of us > - * in the chain of fragments so far. We must know where to put > - * this fragment, right? > - */ > - prev = fq->q.fragments_tail; > - if (!prev || prev->ip_defrag_offset < offset) { > - next = NULL; > - goto found; > - } > - prev = NULL; > - for (next = fq->q.fragments; next != NULL; next = next->next) { > - if (next->ip_defrag_offset >= offset) > - break; /* bingo! */ > - prev = next; > - } > - > -found: > - /* RFC5722, Section 4, amended by Errata ID : 3089 > - * When reassembling an IPv6 datagram, if > - * one or more its constituent fragments is determined to be an > - * overlapping fragment, the entire datagram (and any constituent > - * fragments) MUST be silently discarded. > - */ > - > - /* Check for overlap with preceding fragment. */ > - if (prev && > - (prev->ip_defrag_offset + prev->len) > offset) > - goto discard_fq; > - > - /* Look for overlap with succeeding segment. */ > - if (next && next->ip_defrag_offset < end) > - goto discard_fq; > - > - /* Note : skb->ip_defrag_offset and skb->sk share the same location */ > + /* Note : skb->rbnode and skb->dev share the same location. */ > dev = skb->dev; > - if (dev) > - fq->iif = dev->ifindex; > /* Makes sure compiler wont do silly aliasing games */ > barrier(); > - skb->ip_defrag_offset = offset; > > - /* Insert this fragment in the chain of fragments. */ > - skb->next = next; > - if (!next) > - fq->q.fragments_tail = skb; > - if (prev) > - prev->next = skb; > - else > - fq->q.fragments = skb; > + prev_tail = fq->q.fragments_tail; > + err = inet_frag_queue_insert(&fq->q, skb, offset, end); > + if (err) > + goto insert_error; > + > + if (dev) > + fq->iif = dev->ifindex; > > fq->q.stamp = skb->tstamp; > fq->q.meat += skb->len; > @@ -246,44 +216,48 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb, > > if (fq->q.flags == (INET_FRAG_FIRST_IN | INET_FRAG_LAST_IN) && > fq->q.meat == fq->q.len) { > - int res; > unsigned long orefdst = skb->_skb_refdst; > > skb->_skb_refdst = 0UL; > - res = ip6_frag_reasm(fq, prev, dev); > + err = ip6_frag_reasm(fq, skb, prev_tail, dev); > skb->_skb_refdst = orefdst; > - return res; > + return err; > } > > skb_dst_drop(skb); > - return -1; > + return -EINPROGRESS; > > +insert_error: > + if (err == IPFRAG_DUP) { > + kfree_skb(skb); > + return -EINVAL; > + } > + err = -EINVAL; > + __IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)), > + IPSTATS_MIB_REASM_OVERLAPS); > discard_fq: > inet_frag_kill(&fq->q); > -err: > __IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)), > IPSTATS_MIB_REASMFAILS); > +err: > kfree_skb(skb); > - return -1; > + return err; > } > > /* > * Check if this packet is complete. > - * Returns NULL on failure by any reason, and pointer > - * to current nexthdr field in reassembled frame. > * > * It is called with locked fq, and caller must check that > * queue is eligible for reassembly i.e. it is not COMPLETE, > * the last and the first frames arrived and all the bits are here. > */ > -static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev, > - struct net_device *dev) > +static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *skb, > + struct sk_buff *prev_tail, struct net_device *dev) > { > struct net *net = container_of(fq->q.net, struct net, ipv6.frags); > - struct sk_buff *fp, *head = fq->q.fragments; > - int payload_len, delta; > unsigned int nhoff; > - int sum_truesize; > + void *reasm_data; > + int payload_len; > u8 ecn; > > inet_frag_kill(&fq->q); > @@ -292,121 +266,40 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev, > if (unlikely(ecn == 0xff)) > goto out_fail; > > - /* Make the one we just received the head. */ > - if (prev) { > - head = prev->next; > - fp = skb_clone(head, GFP_ATOMIC); > - > - if (!fp) > - goto out_oom; > - > - fp->next = head->next; > - if (!fp->next) > - fq->q.fragments_tail = fp; > - prev->next = fp; > - > - skb_morph(head, fq->q.fragments); > - head->next = fq->q.fragments->next; > - > - consume_skb(fq->q.fragments); > - fq->q.fragments = head; > - } > - > - WARN_ON(head == NULL); > - WARN_ON(head->ip_defrag_offset != 0); > + reasm_data = inet_frag_reasm_prepare(&fq->q, skb, prev_tail); > + if (!reasm_data) > + goto out_oom; > > - /* Unfragmented part is taken from the first segment. */ > - payload_len = ((head->data - skb_network_header(head)) - > + payload_len = ((skb->data - skb_network_header(skb)) - > sizeof(struct ipv6hdr) + fq->q.len - > sizeof(struct frag_hdr)); > if (payload_len > IPV6_MAXPLEN) > goto out_oversize; > > - delta = - head->truesize; > - > - /* Head of list must not be cloned. */ > - if (skb_unclone(head, GFP_ATOMIC)) > - goto out_oom; > - > - delta += head->truesize; > - if (delta) > - add_frag_mem_limit(fq->q.net, delta); > - > - /* If the first fragment is fragmented itself, we split > - * it to two chunks: the first with data and paged part > - * and the second, holding only fragments. */ > - if (skb_has_frag_list(head)) { > - struct sk_buff *clone; > - int i, plen = 0; > - > - clone = alloc_skb(0, GFP_ATOMIC); > - if (!clone) > - goto out_oom; > - clone->next = head->next; > - head->next = clone; > - skb_shinfo(clone)->frag_list = skb_shinfo(head)->frag_list; > - skb_frag_list_init(head); > - for (i = 0; i < skb_shinfo(head)->nr_frags; i++) > - plen += skb_frag_size(&skb_shinfo(head)->frags[i]); > - clone->len = clone->data_len = head->data_len - plen; > - head->data_len -= clone->len; > - head->len -= clone->len; > - clone->csum = 0; > - clone->ip_summed = head->ip_summed; > - add_frag_mem_limit(fq->q.net, clone->truesize); > - } > - > /* We have to remove fragment header from datagram and to relocate > * header in order to calculate ICV correctly. */ > nhoff = fq->nhoffset; > - skb_network_header(head)[nhoff] = skb_transport_header(head)[0]; > - memmove(head->head + sizeof(struct frag_hdr), head->head, > - (head->data - head->head) - sizeof(struct frag_hdr)); > - if (skb_mac_header_was_set(head)) > - head->mac_header += sizeof(struct frag_hdr); > - head->network_header += sizeof(struct frag_hdr); > - > - skb_reset_transport_header(head); > - skb_push(head, head->data - skb_network_header(head)); > - > - sum_truesize = head->truesize; > - for (fp = head->next; fp;) { > - bool headstolen; > - int delta; > - struct sk_buff *next = fp->next; > - > - sum_truesize += fp->truesize; > - if (head->ip_summed != fp->ip_summed) > - head->ip_summed = CHECKSUM_NONE; > - else if (head->ip_summed == CHECKSUM_COMPLETE) > - head->csum = csum_add(head->csum, fp->csum); > - > - if (skb_try_coalesce(head, fp, &headstolen, &delta)) { > - kfree_skb_partial(fp, headstolen); > - } else { > - fp->sk = NULL; > - if (!skb_shinfo(head)->frag_list) > - skb_shinfo(head)->frag_list = fp; > - head->data_len += fp->len; > - head->len += fp->len; > - head->truesize += fp->truesize; > - } > - fp = next; > - } > - sub_frag_mem_limit(fq->q.net, sum_truesize); > + skb_network_header(skb)[nhoff] = skb_transport_header(skb)[0]; > + memmove(skb->head + sizeof(struct frag_hdr), skb->head, > + (skb->data - skb->head) - sizeof(struct frag_hdr)); > + if (skb_mac_header_was_set(skb)) > + skb->mac_header += sizeof(struct frag_hdr); > + skb->network_header += sizeof(struct frag_hdr); > + > + skb_reset_transport_header(skb); > + > + inet_frag_reasm_finish(&fq->q, skb, reasm_data); > > - skb_mark_not_on_list(head); > - head->dev = dev; > - head->tstamp = fq->q.stamp; > - ipv6_hdr(head)->payload_len = htons(payload_len); > - ipv6_change_dsfield(ipv6_hdr(head), 0xff, ecn); > - IP6CB(head)->nhoff = nhoff; > - IP6CB(head)->flags |= IP6SKB_FRAGMENTED; > - IP6CB(head)->frag_max_size = fq->q.max_size; > + skb->dev = dev; > + ipv6_hdr(skb)->payload_len = htons(payload_len); > + ipv6_change_dsfield(ipv6_hdr(skb), 0xff, ecn); > + IP6CB(skb)->nhoff = nhoff; > + IP6CB(skb)->flags |= IP6SKB_FRAGMENTED; > + IP6CB(skb)->frag_max_size = fq->q.max_size; > > /* Yes, and fold redundant checksum back. 8) */ > - skb_postpush_rcsum(head, skb_network_header(head), > - skb_network_header_len(head)); > + skb_postpush_rcsum(skb, skb_network_header(skb), > + skb_network_header_len(skb)); > > rcu_read_lock(); > __IP6_INC_STATS(net, __in6_dev_get(dev), IPSTATS_MIB_REASMOKS); > @@ -414,6 +307,7 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev, > fq->q.fragments = NULL; > fq->q.rb_fragments = RB_ROOT; > fq->q.fragments_tail = NULL; > + fq->q.last_run_head = NULL; > return 1; > > out_oversize: > @@ -464,10 +358,6 @@ 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; > - > iif = skb->dev ? skb->dev->ifindex : 0; > fq = fq_find(net, fhdr->identification, hdr, iif); > if (fq) { > @@ -485,6 +375,7 @@ static int ipv6_frag_rcv(struct sk_buff *skb) > if (prob_offset) { > __IP6_INC_STATS(net, __in6_dev_get_safely(skb->dev), > IPSTATS_MIB_INHDRERRORS); > + /* icmpv6_param_prob() calls kfree_skb(skb) */ > icmpv6_param_prob(skb, ICMPV6_HDR_FIELD, prob_offset); > } > return ret; > -- > 2.20.1.321.g9e740568ce-goog >
Powered by blists - more mailing lists