From e912b1142be8f1e2c71c71001dc992c6e5eb2ec1 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Wed, 8 Jul 2009 19:36:05 +0000 Subject: net: sk_prot_alloc() should not blindly overwrite memory Some sockets use SLAB_DESTROY_BY_RCU, and our RCU code correctness depends on sk->sk_nulls_node.next being always valid. A NULL value is not allowed as it might fault a lockless reader. Current sk_prot_alloc() implementation doesnt respect this hypothesis, calling kmem_cache_alloc() with __GFP_ZERO. Just call memset() around the forbidden field. Signed-off-by: Eric Dumazet Signed-off-by: David S. Miller --- net/core/sock.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) (limited to 'net') diff --git a/net/core/sock.c b/net/core/sock.c index 6354863b1c6..ba5d2116aea 100644 --- a/net/core/sock.c +++ b/net/core/sock.c @@ -939,8 +939,23 @@ static struct sock *sk_prot_alloc(struct proto *prot, gfp_t priority, struct kmem_cache *slab; slab = prot->slab; - if (slab != NULL) - sk = kmem_cache_alloc(slab, priority); + if (slab != NULL) { + sk = kmem_cache_alloc(slab, priority & ~__GFP_ZERO); + if (!sk) + return sk; + if (priority & __GFP_ZERO) { + /* + * caches using SLAB_DESTROY_BY_RCU should let + * sk_node.next un-modified. Special care is taken + * when initializing object to zero. + */ + if (offsetof(struct sock, sk_node.next) != 0) + memset(sk, 0, offsetof(struct sock, sk_node.next)); + memset(&sk->sk_node.pprev, 0, + prot->obj_size - offsetof(struct sock, + sk_node.pprev)); + } + } else sk = kmalloc(prot->obj_size, priority); -- cgit v1.2.3 From e51a67a9c8a2ea5c563f8c2ba6613fe2100ffe67 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Wed, 8 Jul 2009 14:20:42 +0000 Subject: net: ip_push_pending_frames() fix After commit 2b85a34e911bf483c27cfdd124aeb1605145dc80 (net: No more expensive sock_hold()/sock_put() on each tx) we do not take any more references on sk->sk_refcnt on outgoing packets. I forgot to delete two __sock_put() from ip_push_pending_frames() and ip6_push_pending_frames(). Reported-by: Emil S Tantilov Signed-off-by: Eric Dumazet Tested-by: Emil S Tantilov Signed-off-by: David S. Miller --- net/ipv4/ip_output.c | 1 - net/ipv6/ip6_output.c | 1 - 2 files changed, 2 deletions(-) (limited to 'net') diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c index 24702628266..7d082105472 100644 --- a/net/ipv4/ip_output.c +++ b/net/ipv4/ip_output.c @@ -1243,7 +1243,6 @@ int ip_push_pending_frames(struct sock *sk) skb->len += tmp_skb->len; skb->data_len += tmp_skb->len; skb->truesize += tmp_skb->truesize; - __sock_put(tmp_skb->sk); tmp_skb->destructor = NULL; tmp_skb->sk = NULL; } diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c index 7c76e3d1821..87f8419a68f 100644 --- a/net/ipv6/ip6_output.c +++ b/net/ipv6/ip6_output.c @@ -1484,7 +1484,6 @@ int ip6_push_pending_frames(struct sock *sk) skb->len += tmp_skb->len; skb->data_len += tmp_skb->len; skb->truesize += tmp_skb->truesize; - __sock_put(tmp_skb->sk); tmp_skb->destructor = NULL; tmp_skb->sk = NULL; } -- cgit v1.2.3 From f2ba025b2036e52a176cddcf91b15ac2b10c644a Mon Sep 17 00:00:00 2001 From: Sascha Hlusiak Date: Sat, 11 Jul 2009 20:30:52 -0700 Subject: sit: fix regression: do not release skb->dst before xmit The sit module makes use of skb->dst in it's xmit function, so since 93f154b594fe47 ("net: release dst entry in dev_hard_start_xmit()") sit tunnels are broken, because the flag IFF_XMIT_DST_RELEASE is not unset. This patch unsets that flag for sit devices to fix this regression. Signed-off-by: Sascha Hlusiak Acked-by: Eric Dumazet Signed-off-by: David S. Miller --- net/ipv6/sit.c | 1 + 1 file changed, 1 insertion(+) (limited to 'net') diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c index 68e52308e55..98b7327d094 100644 --- a/net/ipv6/sit.c +++ b/net/ipv6/sit.c @@ -1018,6 +1018,7 @@ static void ipip6_tunnel_setup(struct net_device *dev) dev->hard_header_len = LL_MAX_HEADER + sizeof(struct iphdr); dev->mtu = ETH_DATA_LEN - sizeof(struct iphdr); dev->flags = IFF_NOARP; + dev->priv_flags &= ~IFF_XMIT_DST_RELEASE; dev->iflink = 0; dev->addr_len = 4; dev->features |= NETIF_F_NETNS_LOCAL; -- cgit v1.2.3 From ee686ca919193d7c1f87f907e732df5e2f942523 Mon Sep 17 00:00:00 2001 From: Andreas Jaggi Date: Tue, 14 Jul 2009 09:35:59 -0700 Subject: gre: fix ToS/DiffServ inherit bug Fixes two bugs: - ToS/DiffServ inheritance was unintentionally activated when using impair fixed ToS values - ECN bit was lost during ToS/DiffServ inheritance Signed-off-by: Andreas Jaggi Signed-off-by: David S. Miller --- net/ipv4/ip_gre.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'net') diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c index 44e2a3d2359..cb4a0f4bd5e 100644 --- a/net/ipv4/ip_gre.c +++ b/net/ipv4/ip_gre.c @@ -735,10 +735,10 @@ static int ipgre_tunnel_xmit(struct sk_buff *skb, struct net_device *dev) } tos = tiph->tos; - if (tos&1) { + if (tos == 1) { + tos = 0; if (skb->protocol == htons(ETH_P_IP)) tos = old_iph->tos; - tos &= ~1; } { -- cgit v1.2.3 From f7e5cc0c40dff92bad2894153f675c6c542ba2f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lothar=20Wa=C3=9Fmann?= Date: Tue, 14 Jul 2009 23:10:21 +0000 Subject: net/can bugfix: use after free bug in can protocol drivers Fix a use after free bug in can protocol drivers The release functions of the can protocol drivers lack a call to sock_orphan() which leads to referencing freed memory under certain circumstances. This patch fixes a bug reported here: https://lists.berlios.de/pipermail/socketcan-users/2009-July/000985.html Signed-off-by: Lothar Wassmann Acked-by: Oliver Hartkopp Signed-off-by: David S. Miller --- net/can/bcm.c | 3 +++ net/can/raw.c | 3 +++ 2 files changed, 6 insertions(+) (limited to 'net') diff --git a/net/can/bcm.c b/net/can/bcm.c index 95d7f32643a..1d17e41b892 100644 --- a/net/can/bcm.c +++ b/net/can/bcm.c @@ -1469,6 +1469,9 @@ static int bcm_release(struct socket *sock) bo->ifindex = 0; } + sock_orphan(sk); + sock->sk = NULL; + release_sock(sk); sock_put(sk); diff --git a/net/can/raw.c b/net/can/raw.c index 6aa154e806a..3482546e888 100644 --- a/net/can/raw.c +++ b/net/can/raw.c @@ -306,6 +306,9 @@ static int raw_release(struct socket *sock) ro->bound = 0; ro->count = 0; + sock_orphan(sk); + sock->sk = NULL; + release_sock(sk); sock_put(sk); -- cgit v1.2.3 From b13bb2e9933b9dfa25c81d959d847c843481111e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lothar=20Wa=C3=9Fmann?= Date: Tue, 14 Jul 2009 23:12:25 +0000 Subject: net/can: add module alias to can protocol drivers Add appropriate MODULE_ALIAS() to facilitate autoloading of can protocol drivers Signed-off-by: Lothar Wassmann Acked-by: Oliver Hartkopp Signed-off-by: David S. Miller --- net/can/bcm.c | 1 + net/can/raw.c | 1 + 2 files changed, 2 insertions(+) (limited to 'net') diff --git a/net/can/bcm.c b/net/can/bcm.c index 1d17e41b892..72720c71035 100644 --- a/net/can/bcm.c +++ b/net/can/bcm.c @@ -75,6 +75,7 @@ static __initdata const char banner[] = KERN_INFO MODULE_DESCRIPTION("PF_CAN broadcast manager protocol"); MODULE_LICENSE("Dual BSD/GPL"); MODULE_AUTHOR("Oliver Hartkopp "); +MODULE_ALIAS("can-proto-2"); /* easy access to can_frame payload */ static inline u64 GET_U64(const struct can_frame *cp) diff --git a/net/can/raw.c b/net/can/raw.c index 3482546e888..f4cc44548bd 100644 --- a/net/can/raw.c +++ b/net/can/raw.c @@ -62,6 +62,7 @@ static __initdata const char banner[] = MODULE_DESCRIPTION("PF_CAN raw protocol"); MODULE_LICENSE("Dual BSD/GPL"); MODULE_AUTHOR("Urs Thuermann "); +MODULE_ALIAS("can-proto-1"); #define MASK_ALL 0 -- cgit v1.2.3 From aa6a03eb0ae859c1371555ef381de4c96ca1e4e6 Mon Sep 17 00:00:00 2001 From: Patrick McHardy Date: Thu, 16 Jul 2009 14:01:54 +0200 Subject: netfilter: xt_osf: fix nf_log_packet() arguments The first argument is the address family, the second one the hook number. Signed-off-by: Patrick McHardy --- net/netfilter/xt_osf.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'net') diff --git a/net/netfilter/xt_osf.c b/net/netfilter/xt_osf.c index 863e40977a4..0f482e2440b 100644 --- a/net/netfilter/xt_osf.c +++ b/net/netfilter/xt_osf.c @@ -330,7 +330,8 @@ static bool xt_osf_match_packet(const struct sk_buff *skb, fcount++; if (info->flags & XT_OSF_LOG) - nf_log_packet(p->hooknum, 0, skb, p->in, p->out, NULL, + nf_log_packet(p->family, p->hooknum, skb, + p->in, p->out, NULL, "%s [%s:%s] : %pi4:%d -> %pi4:%d hops=%d\n", f->genre, f->version, f->subtype, &ip->saddr, ntohs(tcp->source), @@ -345,7 +346,7 @@ static bool xt_osf_match_packet(const struct sk_buff *skb, rcu_read_unlock(); if (!fcount && (info->flags & XT_OSF_LOG)) - nf_log_packet(p->hooknum, 0, skb, p->in, p->out, NULL, + nf_log_packet(p->family, p->hooknum, skb, p->in, p->out, NULL, "Remote OS is not known: %pi4:%u -> %pi4:%u\n", &ip->saddr, ntohs(tcp->source), &ip->daddr, ntohs(tcp->dest)); -- cgit v1.2.3 From 941297f443f871b8c3372feccf27a8733f6ce9e9 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Thu, 16 Jul 2009 14:03:40 +0200 Subject: netfilter: nf_conntrack: nf_conntrack_alloc() fixes When a slab cache uses SLAB_DESTROY_BY_RCU, we must be careful when allocating objects, since slab allocator could give a freed object still used by lockless readers. In particular, nf_conntrack RCU lookups rely on ct->tuplehash[xxx].hnnode.next being always valid (ie containing a valid 'nulls' value, or a valid pointer to next object in hash chain.) kmem_cache_zalloc() setups object with NULL values, but a NULL value is not valid for ct->tuplehash[xxx].hnnode.next. Fix is to call kmem_cache_alloc() and do the zeroing ourself. As spotted by Patrick, we also need to make sure lookup keys are committed to memory before setting refcount to 1, or a lockless reader could get a reference on the old version of the object. Its key re-check could then pass the barrier. Signed-off-by: Eric Dumazet Signed-off-by: Patrick McHardy --- net/netfilter/nf_conntrack_core.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) (limited to 'net') diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c index 7508f11c5b3..b5869b9574b 100644 --- a/net/netfilter/nf_conntrack_core.c +++ b/net/netfilter/nf_conntrack_core.c @@ -561,23 +561,38 @@ struct nf_conn *nf_conntrack_alloc(struct net *net, } } - ct = kmem_cache_zalloc(nf_conntrack_cachep, gfp); + /* + * Do not use kmem_cache_zalloc(), as this cache uses + * SLAB_DESTROY_BY_RCU. + */ + ct = kmem_cache_alloc(nf_conntrack_cachep, gfp); if (ct == NULL) { pr_debug("nf_conntrack_alloc: Can't alloc conntrack.\n"); atomic_dec(&net->ct.count); return ERR_PTR(-ENOMEM); } - + /* + * Let ct->tuplehash[IP_CT_DIR_ORIGINAL].hnnode.next + * and ct->tuplehash[IP_CT_DIR_REPLY].hnnode.next unchanged. + */ + memset(&ct->tuplehash[IP_CT_DIR_MAX], 0, + sizeof(*ct) - offsetof(struct nf_conn, tuplehash[IP_CT_DIR_MAX])); spin_lock_init(&ct->lock); - atomic_set(&ct->ct_general.use, 1); ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple = *orig; + ct->tuplehash[IP_CT_DIR_ORIGINAL].hnnode.pprev = NULL; ct->tuplehash[IP_CT_DIR_REPLY].tuple = *repl; + ct->tuplehash[IP_CT_DIR_REPLY].hnnode.pprev = NULL; /* Don't set timer yet: wait for confirmation */ setup_timer(&ct->timeout, death_by_timeout, (unsigned long)ct); #ifdef CONFIG_NET_NS ct->ct_net = net; #endif + /* + * changes to lookup keys must be done before setting refcnt to 1 + */ + smp_wmb(); + atomic_set(&ct->ct_general.use, 1); return ct; } EXPORT_SYMBOL_GPL(nf_conntrack_alloc); -- cgit v1.2.3 From 4dc6dc7162c08b9965163c9ab3f9375d4adff2c7 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Wed, 15 Jul 2009 23:13:10 +0000 Subject: net: sock_copy() fixes Commit e912b1142be8f1e2c71c71001dc992c6e5eb2ec1 (net: sk_prot_alloc() should not blindly overwrite memory) took care of not zeroing whole new socket at allocation time. sock_copy() is another spot where we should be very careful. We should not set refcnt to a non null value, until we are sure other fields are correctly setup, or a lockless reader could catch this socket by mistake, while not fully (re)initialized. This patch puts sk_node & sk_refcnt to the very beginning of struct sock to ease sock_copy() & sk_prot_alloc() job. We add appropriate smp_wmb() before sk_refcnt initializations to match our RCU requirements (changes to sock keys should be committed to memory before sk_refcnt setting) Signed-off-by: Eric Dumazet Signed-off-by: David S. Miller --- net/core/sock.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) (limited to 'net') diff --git a/net/core/sock.c b/net/core/sock.c index ba5d2116aea..d9eec153d53 100644 --- a/net/core/sock.c +++ b/net/core/sock.c @@ -919,13 +919,19 @@ static inline void sock_lock_init(struct sock *sk) af_family_keys + sk->sk_family); } +/* + * Copy all fields from osk to nsk but nsk->sk_refcnt must not change yet, + * even temporarly, because of RCU lookups. sk_node should also be left as is. + */ static void sock_copy(struct sock *nsk, const struct sock *osk) { #ifdef CONFIG_SECURITY_NETWORK void *sptr = nsk->sk_security; #endif - - memcpy(nsk, osk, osk->sk_prot->obj_size); + BUILD_BUG_ON(offsetof(struct sock, sk_copy_start) != + sizeof(osk->sk_node) + sizeof(osk->sk_refcnt)); + memcpy(&nsk->sk_copy_start, &osk->sk_copy_start, + osk->sk_prot->obj_size - offsetof(struct sock, sk_copy_start)); #ifdef CONFIG_SECURITY_NETWORK nsk->sk_security = sptr; security_sk_clone(osk, nsk); @@ -1140,6 +1146,11 @@ struct sock *sk_clone(const struct sock *sk, const gfp_t priority) newsk->sk_err = 0; newsk->sk_priority = 0; + /* + * Before updating sk_refcnt, we must commit prior changes to memory + * (Documentation/RCU/rculist_nulls.txt for details) + */ + smp_wmb(); atomic_set(&newsk->sk_refcnt, 2); /* @@ -1855,6 +1866,11 @@ void sock_init_data(struct socket *sock, struct sock *sk) sk->sk_stamp = ktime_set(-1L, 0); + /* + * Before updating sk_refcnt, we must commit prior changes to memory + * (Documentation/RCU/rculist_nulls.txt for details) + */ + smp_wmb(); atomic_set(&sk->sk_refcnt, 1); atomic_set(&sk->sk_wmem_alloc, 1); atomic_set(&sk->sk_drops, 0); -- cgit v1.2.3