Re: I found a strange place while reading “net/ipv6/reassembly.c”

[Sabrina Dubroca <sd@queasysnail.net>]

2018-08-15, 04:38:29 +0000, Ttttabcd wrote:
> Hello everyone who develops the kernel.
> 
> At the beginning I was looking for the source author, but his email
> address has expired, so I can only come here to ask questions.
> 
> The problem is in the /net/ipv6/reassembly.c file, the author is
> Pedro Roque.
> 
> I found some strange places when I read the code for this file
> (Linux Kernel version 4.18).
> 
> In the "/net/ipv6/reassembly.c"
> 
> In the function "ip6_frag_queue"
> 
> 	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);
> 		return -1;
> 	}
> 
> Here the length of the payload is judged.

This check is based on the fragment currently being processed, and
only considers the reassembled payload.

> And in the function "ip6_frag_reasm"
> 

Re-adding the comment that comes just above this:

  	/* Unfragmented part is taken from the first segment. */
> 	payload_len = ((head->data - skb_network_header(head)) -
> 		       sizeof(struct ipv6hdr) + fq->q.len -
> 		       sizeof(struct frag_hdr));
> 	if (payload_len > IPV6_MAXPLEN)
> 		goto out_oversize;

This considers the reassembled payload (ie, as above) *and* any
extension headers that might come before it.


> 	......
> 	out_oversize:
> 		net_dbg_ratelimited("ip6_frag_reasm: payload len = %d\n", payload_len);
> 		goto out_fail;
> 
> Here also judges the length of the payload.
> 
> Judged the payload length twice.
> 
> I tested that the code in the label "out_oversize:" does not execute
> at all, because it has been returned in "ip6_frag_queue".

If you try again with a routing header, I think you'll see it trigger.

> Unless I comment out the code that judge the payload length in the
> function "ip6_frag_queue", the code labeled "out_oversize:" can be
> executed.
> 
> So, is this repeated?

-- 
Sabrina