Message ID | 20091028185947.GA12675@hmsreliant.think-freely.org |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
Neil Horman a écrit : > >> I believe we should drop the request, since padding it is not what was expected by user. > > Yeah, I had a feeling. Ok, version 2, this time drop the invalid frame and > report it to user space, instead of expanding it: > > > Augment raw_send_hdrinc to correct for incorrect ip header length values > > A series of oopses was reported to me recently. Apparently when using AF_RAW > sockets to send data to peers that were reachable via ipsec encapsulation, > people could panic or BUG halt their systems. > > I've tracked the problem down to user space sending an invalid ip header over an > AF_RAW socket with IP_HDRINCL set to 1. > > Basically what happens is that userspace sends down an ip frame that includes > only the header (no data), but sets the ip header ihl value to a large number, > one that is larger than the total amount of data passed to the sendmsg call. In > raw_send_hdrincl, we allocate an skb based on the size of the data in the msghdr > that was passed in, but assume the data is all valid. Later during ipsec > encapsulation, xfrm4_tranport_output moves the entire frame back in the skbuff > to provide headroom for the ipsec headers. During this operation, the > skb->transport_header is repointed to a spot computed by > skb->network_header + the ip header length (ihl). Since so little data was > passed in relative to the value of ihl provided by the raw socket, we point > transport header to an unknown location, resulting in various crashes. > > This fix for this is pretty straightforward, simply validate the value of of > iph->ihl when sending over a raw socket. If (iph->ihl*4U) > user data buffer > size, drop the frame and return -EINVAL. I just confirmed this fixes the > reported crashes. > > Signed-off-by: Neil Horman <nhorman@tuxdriver.com> Acked-by: Eric Dumazet <eric.dumazet@gmail.com> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Wed, 28 Oct 2009 22:01:08 +0100 > Neil Horman a écrit : >> >>> I believe we should drop the request, since padding it is not what was expected by user. >> >> Yeah, I had a feeling. Ok, version 2, this time drop the invalid frame and >> report it to user space, instead of expanding it: >> >> >> Augment raw_send_hdrinc to correct for incorrect ip header length values ... >> Signed-off-by: Neil Horman <nhorman@tuxdriver.com> > > Acked-by: Eric Dumazet <eric.dumazet@gmail.com> > Applied to net-2.6, thanks everyone! -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c index 9ef8c08..4b15354 100644 --- a/net/ipv4/raw.c +++ b/net/ipv4/raw.c @@ -351,13 +351,24 @@ static int raw_send_hdrinc(struct sock *sk, void *from, size_t length, skb->ip_summed = CHECKSUM_NONE; skb->transport_header = skb->network_header; - err = memcpy_fromiovecend((void *)iph, from, 0, length); - if (err) - goto error_fault; + err = -EFAULT; + if (memcpy_fromiovecend((void *)iph, from, 0, length)) + goto error_free; - /* We don't modify invalid header */ iphlen = iph->ihl * 4; - if (iphlen >= sizeof(*iph) && iphlen <= length) { + + /* + * We don't want to modify the ip header, but we do need to + * be sure that it won't cause problems later along the network + * stack. Specifically we want to make sure that iph->ihl is a + * sane value. If ihl points beyond the length of the buffer passed + * in, reject the frame as invalid + */ + err = -EINVAL; + if (iphlen > length) + goto error_free; + + if (iphlen >= sizeof(*iph)) { if (!iph->saddr) iph->saddr = rt->rt_src; iph->check = 0; @@ -380,8 +391,7 @@ static int raw_send_hdrinc(struct sock *sk, void *from, size_t length, out: return 0; -error_fault: - err = -EFAULT; +error_free: kfree_skb(skb); error: IP_INC_STATS(net, IPSTATS_MIB_OUTDISCARDS);