Commit 6bd4f355 authored by Eric Dumazet's avatar Eric Dumazet Committed by David S. Miller
Browse files

ipv6: kill sk_dst_lock 



While testing the np->opt RCU conversion, I found that UDP/IPv6 was
using a mixture of xchg() and sk_dst_lock to protect concurrent changes
to sk->sk_dst_cache, leading to possible corruptions and crashes.

ip6_sk_dst_lookup_flow() uses sk_dst_check() anyway, so the simplest
way to fix the mess is to remove sk_dst_lock completely, as we did for
IPv4.

__ip6_dst_store() and ip6_dst_store() share same implementation.

sk_setup_caps() being called with socket lock being held or not,
we have to use sk_dst_set() instead of __sk_dst_set()

Note that I had to move the "np->dst_cookie = rt6_get_cookie(rt);"
in ip6_dst_store() before the sk_setup_caps(sk, dst) call.

This is because ip6_dst_store() can be called from process context,
without any lock held.

As soon as the dst is installed in sk->sk_dst_cache, dst can be freed
from another cpu doing a concurrent ip6_dst_store()

Doing the dst dereference before doing the install is needed to make
sure no use after free would trigger.

Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
Reported-by: default avatarDmitry Vyukov <dvyukov@google.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent c836a8ba

include/net/ip6_route.h

+4 −13
Original line number Diff line number Diff line
@@ -133,27 +133,18 @@ void rt6_clean_tohost(struct net *net, struct in6_addr *gateway); 
/*

 *	Store a destination cache entry in a socket

 */

static inline void __ip6_dst_store(struct sock *sk, struct dst_entry *dst,

static inline void ip6_dst_store(struct sock *sk, struct dst_entry *dst,

				 const struct in6_addr *daddr,

				 const struct in6_addr *saddr)

{

	struct ipv6_pinfo *np = inet6_sk(sk);

	struct rt6_info *rt = (struct rt6_info *) dst;



	np->dst_cookie = rt6_get_cookie((struct rt6_info *)dst);

	sk_setup_caps(sk, dst);

	np->daddr_cache = daddr;

#ifdef CONFIG_IPV6_SUBTREES

	np->saddr_cache = saddr;

#endif

	np->dst_cookie = rt6_get_cookie(rt);

}



static inline void ip6_dst_store(struct sock *sk, struct dst_entry *dst,

				 struct in6_addr *daddr, struct in6_addr *saddr)

{

	spin_lock(&sk->sk_dst_lock);

	__ip6_dst_store(sk, dst, daddr, saddr);

	spin_unlock(&sk->sk_dst_lock);

}



static inline bool ipv6_unicast_destination(const struct sk_buff *skb)

include/net/sock.h

+1 −2
Original line number Diff line number Diff line
@@ -254,7 +254,6 @@ struct cg_proto; 
  *	@sk_wq: sock wait queue and async head

  *	@sk_rx_dst: receive input route used by early demux

  *	@sk_dst_cache: destination cache

  *	@sk_dst_lock: destination cache lock

  *	@sk_policy: flow policy

  *	@sk_receive_queue: incoming packets

  *	@sk_wmem_alloc: transmit queue bytes committed
@@ -393,7 +392,7 @@ struct sock { 
#endif

	struct dst_entry	*sk_rx_dst;

	struct dst_entry __rcu	*sk_dst_cache;

	spinlock_t		sk_dst_lock;

	/* Note: 32bit hole on 64bit arches */

	atomic_t		sk_wmem_alloc;

	atomic_t		sk_omem_alloc;

	int			sk_sndbuf;

net/core/sock.c

+1 −3
Original line number Diff line number Diff line
@@ -1530,7 +1530,6 @@ struct sock *sk_clone_lock(const struct sock *sk, const gfp_t priority) 
		skb_queue_head_init(&newsk->sk_receive_queue);

		skb_queue_head_init(&newsk->sk_write_queue);



		spin_lock_init(&newsk->sk_dst_lock);

		rwlock_init(&newsk->sk_callback_lock);

		lockdep_set_class_and_name(&newsk->sk_callback_lock,

				af_callback_keys + newsk->sk_family,
@@ -1607,7 +1606,7 @@ void sk_setup_caps(struct sock *sk, struct dst_entry *dst) 
{

	u32 max_segs = 1;



	__sk_dst_set(sk, dst);

	sk_dst_set(sk, dst);

	sk->sk_route_caps = dst->dev->features;

	if (sk->sk_route_caps & NETIF_F_GSO)

		sk->sk_route_caps |= NETIF_F_GSO_SOFTWARE;
@@ -2388,7 +2387,6 @@ void sock_init_data(struct socket *sock, struct sock *sk) 
	} else

		sk->sk_wq	=	NULL;



	spin_lock_init(&sk->sk_dst_lock);

	rwlock_init(&sk->sk_callback_lock);

	lockdep_set_class_and_name(&sk->sk_callback_lock,

			af_callback_keys + sk->sk_family,

net/dccp/ipv6.c

+2 −2
Original line number Diff line number Diff line
@@ -459,7 +459,7 @@ static struct sock *dccp_v6_request_recv_sock(const struct sock *sk, 
	 * comment in that function for the gory details. -acme

	 */



	__ip6_dst_store(newsk, dst, NULL, NULL);

	ip6_dst_store(newsk, dst, NULL, NULL);

	newsk->sk_route_caps = dst->dev->features & ~(NETIF_F_IP_CSUM |

						      NETIF_F_TSO);

	newdp6 = (struct dccp6_sock *)newsk;
@@ -883,7 +883,7 @@ static int dccp_v6_connect(struct sock *sk, struct sockaddr *uaddr, 
	np->saddr = *saddr;

	inet->inet_rcv_saddr = LOOPBACK4_IPV6;



	__ip6_dst_store(sk, dst, NULL, NULL);

	ip6_dst_store(sk, dst, NULL, NULL);



	icsk->icsk_ext_hdr_len = 0;

	if (opt)

net/ipv6/af_inet6.c

+1 −1
Original line number Diff line number Diff line
@@ -673,7 +673,7 @@ int inet6_sk_rebuild_header(struct sock *sk) 
			return PTR_ERR(dst);

		}



		__ip6_dst_store(sk, dst, NULL, NULL);

		ip6_dst_store(sk, dst, NULL, NULL);

	}



	return 0;

CRA Git | Maintained and supported by SUSTech CRA and CCSE