ACK: [maverick, maverick/ti-omap4, natty/ti-omap4 CVE 1/1] net: ip_expire() must revalidate route

Stefan Bader stefan.bader at canonical.com
Thu Jan 26 14:33:27 UTC 2012


On 26.01.2012 15:14, Andy Whitcroft wrote:
> From: Eric Dumazet <eric.dumazet at gmail.com>
> 
> Commit 4a94445c9a5c (net: Use ip_route_input_noref() in input path)
> added a bug in IP defragmentation handling, in case timeout is fired.
> 
> When a frame is defragmented, we use last skb dst field when building
> final skb. Its dst is valid, since we are in rcu read section.
> 
> But if a timeout occurs, we take first queued fragment to build one ICMP
> TIME EXCEEDED message. Problem is all queued skb have weak dst pointers,
> since we escaped RCU critical section after their queueing. icmp_send()
> might dereference a now freed (and possibly reused) part of memory.
> 
> Calling skb_dst_drop() and ip_route_input_noref() to revalidate route is
> the only possible choice.
> 
> Reported-by: Denys Fedoryshchenko <denys at visp.net.lb>
> Signed-off-by: Eric Dumazet <eric.dumazet at gmail.com>
> Signed-off-by: David S. Miller <davem at davemloft.net>
> 
> (cherry picked from commit 64f3b9e203bd06855072e295557dca1485a2ecba)
> CVE-2011-1927
> BugLink: http://bugs.launchpad.net/bugs/922051
> Signed-off-by: Andy Whitcroft <apw at canonical.com>
> ---
>  net/ipv4/ip_fragment.c |   33 ++++++++++++++++-----------------
>  1 files changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
> index 75347ea..cbc6571 100644
> --- a/net/ipv4/ip_fragment.c
> +++ b/net/ipv4/ip_fragment.c
> @@ -207,31 +207,30 @@ static void ip_expire(unsigned long arg)
>  
>  	if ((qp->q.last_in & INET_FRAG_FIRST_IN) && qp->q.fragments != NULL) {
>  		struct sk_buff *head = qp->q.fragments;
> +		const struct iphdr *iph;
> +		int err;
>  
>  		rcu_read_lock();
>  		head->dev = dev_get_by_index_rcu(net, qp->iif);
>  		if (!head->dev)
>  			goto out_rcu_unlock;
>  
> +		/* skb dst is stale, drop it, and perform route lookup again */
> +		skb_dst_drop(head);
> +		iph = ip_hdr(head);
> +		err = ip_route_input_noref(head, iph->daddr, iph->saddr,
> +					   iph->tos, head->dev);
> +		if (err)
> +			goto out_rcu_unlock;
> +
>  		/*
> -		 * Only search router table for the head fragment,
> -		 * when defraging timeout at PRE_ROUTING HOOK.
> +		 * Only an end host needs to send an ICMP
> +		 * "Fragment Reassembly Timeout" message, per RFC792.
>  		 */
> -		if (qp->user == IP_DEFRAG_CONNTRACK_IN && !skb_dst(head)) {
> -			const struct iphdr *iph = ip_hdr(head);
> -			int err = ip_route_input(head, iph->daddr, iph->saddr,
> -						 iph->tos, head->dev);
> -			if (unlikely(err))
> -				goto out_rcu_unlock;
> -
> -			/*
> -			 * Only an end host needs to send an ICMP
> -			 * "Fragment Reassembly Timeout" message, per RFC792.
> -			 */
> -			if (skb_rtable(head)->rt_type != RTN_LOCAL)
> -				goto out_rcu_unlock;
> +		if (qp->user == IP_DEFRAG_CONNTRACK_IN &&
> +		    skb_rtable(head)->rt_type != RTN_LOCAL)
> +			goto out_rcu_unlock;
>  
> -		}
>  
>  		/* Send an ICMP "Fragment Reassembly Timeout" message. */
>  		icmp_send(head, ICMP_TIME_EXCEEDED, ICMP_EXC_FRAGTIME, 0);

Looks to be doing as claimed and actually is a cherry pick

Acked-by: Stefan Bader <smb at canonical.com>




More information about the kernel-team mailing list