forked from mirrors/linux
		
	ip: fail fast on IP defrag errors
The current behavior of IP defragmentation is inconsistent: - some overlapping/wrong length fragments are dropped without affecting the queue; - most overlapping fragments cause the whole frag queue to be dropped. This patch brings consistency: if a bad fragment is detected, the whole frag queue is dropped. Two major benefits: - fail fast: corrupted frag queues are cleared immediately, instead of by timeout; - testing of overlapping fragments is now much easier: any kind of random fragment length mutation now leads to the frag queue being discarded (IP packet dropped); before this patch, some overlaps were "corrected", with tests not seeing expected packet drops. Note that in one case (see "if (end&7)" conditional) the current behavior is preserved as there are concerns that this could be legitimate padding. Signed-off-by: Peter Oskolkov <posk@google.com> Reviewed-by: Eric Dumazet <edumazet@google.com> Reviewed-by: Willem de Bruijn <willemb@google.com> Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
		
							parent
							
								
									b943f17e06
								
							
						
					
					
						commit
						0ff89efb52
					
				
					 1 changed files with 12 additions and 9 deletions
				
			
		| 
						 | 
					@ -382,7 +382,7 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb)
 | 
				
			||||||
		 */
 | 
							 */
 | 
				
			||||||
		if (end < qp->q.len ||
 | 
							if (end < qp->q.len ||
 | 
				
			||||||
		    ((qp->q.flags & INET_FRAG_LAST_IN) && end != qp->q.len))
 | 
							    ((qp->q.flags & INET_FRAG_LAST_IN) && end != qp->q.len))
 | 
				
			||||||
			goto err;
 | 
								goto discard_qp;
 | 
				
			||||||
		qp->q.flags |= INET_FRAG_LAST_IN;
 | 
							qp->q.flags |= INET_FRAG_LAST_IN;
 | 
				
			||||||
		qp->q.len = end;
 | 
							qp->q.len = end;
 | 
				
			||||||
	} else {
 | 
						} else {
 | 
				
			||||||
| 
						 | 
					@ -394,20 +394,20 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb)
 | 
				
			||||||
		if (end > qp->q.len) {
 | 
							if (end > qp->q.len) {
 | 
				
			||||||
			/* Some bits beyond end -> corruption. */
 | 
								/* Some bits beyond end -> corruption. */
 | 
				
			||||||
			if (qp->q.flags & INET_FRAG_LAST_IN)
 | 
								if (qp->q.flags & INET_FRAG_LAST_IN)
 | 
				
			||||||
				goto err;
 | 
									goto discard_qp;
 | 
				
			||||||
			qp->q.len = end;
 | 
								qp->q.len = end;
 | 
				
			||||||
		}
 | 
							}
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
	if (end == offset)
 | 
						if (end == offset)
 | 
				
			||||||
		goto err;
 | 
							goto discard_qp;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	err = -ENOMEM;
 | 
						err = -ENOMEM;
 | 
				
			||||||
	if (!pskb_pull(skb, skb_network_offset(skb) + ihl))
 | 
						if (!pskb_pull(skb, skb_network_offset(skb) + ihl))
 | 
				
			||||||
		goto err;
 | 
							goto discard_qp;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	err = pskb_trim_rcsum(skb, end - offset);
 | 
						err = pskb_trim_rcsum(skb, end - offset);
 | 
				
			||||||
	if (err)
 | 
						if (err)
 | 
				
			||||||
		goto err;
 | 
							goto discard_qp;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	/* Note : skb->rbnode and skb->dev share the same location. */
 | 
						/* Note : skb->rbnode and skb->dev share the same location. */
 | 
				
			||||||
	dev = skb->dev;
 | 
						dev = skb->dev;
 | 
				
			||||||
| 
						 | 
					@ -423,6 +423,7 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb)
 | 
				
			||||||
	 * We do the same here for IPv4 (and increment an snmp counter).
 | 
						 * We do the same here for IPv4 (and increment an snmp counter).
 | 
				
			||||||
	 */
 | 
						 */
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
						err = -EINVAL;
 | 
				
			||||||
	/* Find out where to put this fragment.  */
 | 
						/* Find out where to put this fragment.  */
 | 
				
			||||||
	prev_tail = qp->q.fragments_tail;
 | 
						prev_tail = qp->q.fragments_tail;
 | 
				
			||||||
	if (!prev_tail)
 | 
						if (!prev_tail)
 | 
				
			||||||
| 
						 | 
					@ -431,7 +432,7 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb)
 | 
				
			||||||
		/* This is the common case: skb goes to the end. */
 | 
							/* This is the common case: skb goes to the end. */
 | 
				
			||||||
		/* Detect and discard overlaps. */
 | 
							/* Detect and discard overlaps. */
 | 
				
			||||||
		if (offset < prev_tail->ip_defrag_offset + prev_tail->len)
 | 
							if (offset < prev_tail->ip_defrag_offset + prev_tail->len)
 | 
				
			||||||
			goto discard_qp;
 | 
								goto overlap;
 | 
				
			||||||
		if (offset == prev_tail->ip_defrag_offset + prev_tail->len)
 | 
							if (offset == prev_tail->ip_defrag_offset + prev_tail->len)
 | 
				
			||||||
			ip4_frag_append_to_last_run(&qp->q, skb);
 | 
								ip4_frag_append_to_last_run(&qp->q, skb);
 | 
				
			||||||
		else
 | 
							else
 | 
				
			||||||
| 
						 | 
					@ -450,7 +451,7 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb)
 | 
				
			||||||
						FRAG_CB(skb1)->frag_run_len)
 | 
											FRAG_CB(skb1)->frag_run_len)
 | 
				
			||||||
				rbn = &parent->rb_right;
 | 
									rbn = &parent->rb_right;
 | 
				
			||||||
			else /* Found an overlap with skb1. */
 | 
								else /* Found an overlap with skb1. */
 | 
				
			||||||
				goto discard_qp;
 | 
									goto overlap;
 | 
				
			||||||
		} while (*rbn);
 | 
							} while (*rbn);
 | 
				
			||||||
		/* Here we have parent properly set, and rbn pointing to
 | 
							/* Here we have parent properly set, and rbn pointing to
 | 
				
			||||||
		 * one of its NULL left/right children. Insert skb.
 | 
							 * one of its NULL left/right children. Insert skb.
 | 
				
			||||||
| 
						 | 
					@ -487,16 +488,18 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb)
 | 
				
			||||||
		skb->_skb_refdst = 0UL;
 | 
							skb->_skb_refdst = 0UL;
 | 
				
			||||||
		err = ip_frag_reasm(qp, skb, prev_tail, dev);
 | 
							err = ip_frag_reasm(qp, skb, prev_tail, dev);
 | 
				
			||||||
		skb->_skb_refdst = orefdst;
 | 
							skb->_skb_refdst = orefdst;
 | 
				
			||||||
 | 
							if (err)
 | 
				
			||||||
 | 
								inet_frag_kill(&qp->q);
 | 
				
			||||||
		return err;
 | 
							return err;
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	skb_dst_drop(skb);
 | 
						skb_dst_drop(skb);
 | 
				
			||||||
	return -EINPROGRESS;
 | 
						return -EINPROGRESS;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					overlap:
 | 
				
			||||||
 | 
						__IP_INC_STATS(net, IPSTATS_MIB_REASM_OVERLAPS);
 | 
				
			||||||
discard_qp:
 | 
					discard_qp:
 | 
				
			||||||
	inet_frag_kill(&qp->q);
 | 
						inet_frag_kill(&qp->q);
 | 
				
			||||||
	err = -EINVAL;
 | 
					 | 
				
			||||||
	__IP_INC_STATS(net, IPSTATS_MIB_REASM_OVERLAPS);
 | 
					 | 
				
			||||||
err:
 | 
					err:
 | 
				
			||||||
	kfree_skb(skb);
 | 
						kfree_skb(skb);
 | 
				
			||||||
	return err;
 | 
						return err;
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
		Reference in a new issue