* [PATCH net 1/2] vxlan: avoid using stale vxlan socket.
@ 2016-10-27 18:51 Pravin B Shelar
2016-10-27 18:51 ` [PATCH net 2/2] geneve: avoid using stale geneve socket Pravin B Shelar
2016-10-27 23:02 ` [PATCH net 1/2] vxlan: avoid using stale vxlan socket Stephen Hemminger
0 siblings, 2 replies; 6+ messages in thread
From: Pravin B Shelar @ 2016-10-27 18:51 UTC (permalink / raw)
To: netdev; +Cc: Pravin B Shelar
When vxlan device is closed vxlan socket is freed. This
operation can race with vxlan-xmit function which
dereferences vxlan socket. Following patch uses RCU
mechanism to avoid this situation.
Signed-off-by: Pravin B Shelar <pshelar@ovn.org>
---
drivers/net/vxlan.c | 80 +++++++++++++++++++++++++++++++++--------------------
include/net/vxlan.h | 4 +--
2 files changed, 52 insertions(+), 32 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index c1639a3..0bc1dcc 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -943,17 +943,20 @@ static bool vxlan_snoop(struct net_device *dev,
static bool vxlan_group_used(struct vxlan_net *vn, struct vxlan_dev *dev)
{
struct vxlan_dev *vxlan;
+ struct vxlan_sock *sock4;
+ struct vxlan_sock *sock6 = NULL;
unsigned short family = dev->default_dst.remote_ip.sa.sa_family;
+ sock4 = rtnl_dereference(dev->vn4_sock);
+
/* The vxlan_sock is only used by dev, leaving group has
* no effect on other vxlan devices.
*/
- if (family == AF_INET && dev->vn4_sock &&
- atomic_read(&dev->vn4_sock->refcnt) == 1)
+ if (family == AF_INET && sock4 && atomic_read(&sock4->refcnt) == 1)
return false;
#if IS_ENABLED(CONFIG_IPV6)
- if (family == AF_INET6 && dev->vn6_sock &&
- atomic_read(&dev->vn6_sock->refcnt) == 1)
+ sock6 = rtnl_dereference(dev->vn6_sock);
+ if (family == AF_INET6 && sock6 && atomic_read(&sock6->refcnt) == 1)
return false;
#endif
@@ -961,10 +964,12 @@ static bool vxlan_group_used(struct vxlan_net *vn, struct vxlan_dev *dev)
if (!netif_running(vxlan->dev) || vxlan == dev)
continue;
- if (family == AF_INET && vxlan->vn4_sock != dev->vn4_sock)
+ if (family == AF_INET &&
+ rtnl_dereference(vxlan->vn4_sock) != sock4)
continue;
#if IS_ENABLED(CONFIG_IPV6)
- if (family == AF_INET6 && vxlan->vn6_sock != dev->vn6_sock)
+ if (family == AF_INET6 &&
+ rtnl_dereference(vxlan->vn6_sock) != sock6)
continue;
#endif
@@ -1005,22 +1010,25 @@ static bool __vxlan_sock_release_prep(struct vxlan_sock *vs)
static void vxlan_sock_release(struct vxlan_dev *vxlan)
{
- bool ipv4 = __vxlan_sock_release_prep(vxlan->vn4_sock);
+ struct vxlan_sock *sock4 = rtnl_dereference(vxlan->vn4_sock);
#if IS_ENABLED(CONFIG_IPV6)
- bool ipv6 = __vxlan_sock_release_prep(vxlan->vn6_sock);
+ struct vxlan_sock *sock6 = rtnl_dereference(vxlan->vn6_sock);
+
+ rcu_assign_pointer(vxlan->vn6_sock, NULL);
#endif
+ rcu_assign_pointer(vxlan->vn4_sock, NULL);
synchronize_net();
- if (ipv4) {
- udp_tunnel_sock_release(vxlan->vn4_sock->sock);
- kfree(vxlan->vn4_sock);
+ if (__vxlan_sock_release_prep(sock4)) {
+ udp_tunnel_sock_release(sock4->sock);
+ kfree(sock4);
}
#if IS_ENABLED(CONFIG_IPV6)
- if (ipv6) {
- udp_tunnel_sock_release(vxlan->vn6_sock->sock);
- kfree(vxlan->vn6_sock);
+ if (__vxlan_sock_release_prep(sock6)) {
+ udp_tunnel_sock_release(sock6->sock);
+ kfree(sock6);
}
#endif
}
@@ -1036,18 +1044,21 @@ static int vxlan_igmp_join(struct vxlan_dev *vxlan)
int ret = -EINVAL;
if (ip->sa.sa_family == AF_INET) {
+ struct vxlan_sock *sock4 = rtnl_dereference(vxlan->vn4_sock);
struct ip_mreqn mreq = {
.imr_multiaddr.s_addr = ip->sin.sin_addr.s_addr,
.imr_ifindex = ifindex,
};
- sk = vxlan->vn4_sock->sock->sk;
+ sk = sock4->sock->sk;
lock_sock(sk);
ret = ip_mc_join_group(sk, &mreq);
release_sock(sk);
#if IS_ENABLED(CONFIG_IPV6)
} else {
- sk = vxlan->vn6_sock->sock->sk;
+ struct vxlan_sock *sock6 = rtnl_dereference(vxlan->vn6_sock);
+
+ sk = sock6->sock->sk;
lock_sock(sk);
ret = ipv6_stub->ipv6_sock_mc_join(sk, ifindex,
&ip->sin6.sin6_addr);
@@ -1067,18 +1078,21 @@ static int vxlan_igmp_leave(struct vxlan_dev *vxlan)
int ret = -EINVAL;
if (ip->sa.sa_family == AF_INET) {
+ struct vxlan_sock *sock4 = rtnl_dereference(vxlan->vn4_sock);
struct ip_mreqn mreq = {
.imr_multiaddr.s_addr = ip->sin.sin_addr.s_addr,
.imr_ifindex = ifindex,
};
- sk = vxlan->vn4_sock->sock->sk;
+ sk = sock4->sock->sk;
lock_sock(sk);
ret = ip_mc_leave_group(sk, &mreq);
release_sock(sk);
#if IS_ENABLED(CONFIG_IPV6)
} else {
- sk = vxlan->vn6_sock->sock->sk;
+ struct vxlan_sock *sock6 = rtnl_dereference(vxlan->vn6_sock);
+
+ sk = sock6->sock->sk;
lock_sock(sk);
ret = ipv6_stub->ipv6_sock_mc_drop(sk, ifindex,
&ip->sin6.sin6_addr);
@@ -1828,11 +1842,15 @@ static struct dst_entry *vxlan6_get_route(struct vxlan_dev *vxlan,
struct dst_cache *dst_cache,
const struct ip_tunnel_info *info)
{
+ struct vxlan_sock *sock6 = rcu_dereference(vxlan->vn6_sock);
bool use_cache = ip_tunnel_dst_cache_usable(skb, info);
struct dst_entry *ndst;
struct flowi6 fl6;
int err;
+ if (!sock6)
+ return ERR_PTR(-EIO);
+
if (tos && !info)
use_cache = false;
if (use_cache) {
@@ -1850,7 +1868,7 @@ static struct dst_entry *vxlan6_get_route(struct vxlan_dev *vxlan,
fl6.flowi6_proto = IPPROTO_UDP;
err = ipv6_stub->ipv6_dst_lookup(vxlan->net,
- vxlan->vn6_sock->sock->sk,
+ sock6->sock->sk,
&ndst, &fl6);
if (err < 0)
return ERR_PTR(err);
@@ -1995,9 +2013,11 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
}
if (dst->sa.sa_family == AF_INET) {
- if (!vxlan->vn4_sock)
+ struct vxlan_sock *sock4 = rcu_dereference(vxlan->vn4_sock);
+
+ if (!sock4)
goto drop;
- sk = vxlan->vn4_sock->sock->sk;
+ sk = sock4->sock->sk;
rt = vxlan_get_route(vxlan, skb,
rdst ? rdst->remote_ifindex : 0, tos,
@@ -2050,12 +2070,13 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
src_port, dst_port, xnet, !udp_sum);
#if IS_ENABLED(CONFIG_IPV6)
} else {
+ struct vxlan_sock *sock6 = rcu_dereference(vxlan->vn6_sock);
struct dst_entry *ndst;
u32 rt6i_flags;
- if (!vxlan->vn6_sock)
+ if (!sock6)
goto drop;
- sk = vxlan->vn6_sock->sock->sk;
+ sk = sock6->sock->sk;
ndst = vxlan6_get_route(vxlan, skb,
rdst ? rdst->remote_ifindex : 0, tos,
@@ -2415,9 +2436,10 @@ static int vxlan_fill_metadata_dst(struct net_device *dev, struct sk_buff *skb)
dport = info->key.tp_dst ? : vxlan->cfg.dst_port;
if (ip_tunnel_info_af(info) == AF_INET) {
+ struct vxlan_sock *sock4 = rcu_dereference(vxlan->vn4_sock);
struct rtable *rt;
- if (!vxlan->vn4_sock)
+ if (!sock4)
return -EINVAL;
rt = vxlan_get_route(vxlan, skb, 0, info->key.tos,
info->key.u.ipv4.dst,
@@ -2429,8 +2451,6 @@ static int vxlan_fill_metadata_dst(struct net_device *dev, struct sk_buff *skb)
#if IS_ENABLED(CONFIG_IPV6)
struct dst_entry *ndst;
- if (!vxlan->vn6_sock)
- return -EINVAL;
ndst = vxlan6_get_route(vxlan, skb, 0, info->key.tos,
info->key.label, &info->key.u.ipv6.dst,
&info->key.u.ipv6.src, NULL, info);
@@ -2740,10 +2760,10 @@ static int __vxlan_sock_add(struct vxlan_dev *vxlan, bool ipv6)
return PTR_ERR(vs);
#if IS_ENABLED(CONFIG_IPV6)
if (ipv6)
- vxlan->vn6_sock = vs;
+ rcu_assign_pointer(vxlan->vn6_sock, vs);
else
#endif
- vxlan->vn4_sock = vs;
+ rcu_assign_pointer(vxlan->vn4_sock, vs);
vxlan_vs_add_dev(vs, vxlan);
return 0;
}
@@ -2754,9 +2774,9 @@ static int vxlan_sock_add(struct vxlan_dev *vxlan)
bool metadata = vxlan->flags & VXLAN_F_COLLECT_METADATA;
int ret = 0;
- vxlan->vn4_sock = NULL;
+ rcu_assign_pointer(vxlan->vn4_sock, NULL);
#if IS_ENABLED(CONFIG_IPV6)
- vxlan->vn6_sock = NULL;
+ rcu_assign_pointer(vxlan->vn6_sock, NULL);
if (ipv6 || metadata)
ret = __vxlan_sock_add(vxlan, true);
#endif
diff --git a/include/net/vxlan.h b/include/net/vxlan.h
index 0255613..308adc4 100644
--- a/include/net/vxlan.h
+++ b/include/net/vxlan.h
@@ -225,9 +225,9 @@ struct vxlan_config {
struct vxlan_dev {
struct hlist_node hlist; /* vni hash table */
struct list_head next; /* vxlan's per namespace list */
- struct vxlan_sock *vn4_sock; /* listening socket for IPv4 */
+ struct vxlan_sock __rcu *vn4_sock; /* listening socket for IPv4 */
#if IS_ENABLED(CONFIG_IPV6)
- struct vxlan_sock *vn6_sock; /* listening socket for IPv6 */
+ struct vxlan_sock __rcu *vn6_sock; /* listening socket for IPv6 */
#endif
struct net_device *dev;
struct net *net; /* netns for packet i/o */
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net 2/2] geneve: avoid using stale geneve socket.
2016-10-27 18:51 [PATCH net 1/2] vxlan: avoid using stale vxlan socket Pravin B Shelar
@ 2016-10-27 18:51 ` Pravin B Shelar
2016-10-28 14:27 ` John W. Linville
2016-10-27 23:02 ` [PATCH net 1/2] vxlan: avoid using stale vxlan socket Stephen Hemminger
1 sibling, 1 reply; 6+ messages in thread
From: Pravin B Shelar @ 2016-10-27 18:51 UTC (permalink / raw)
To: netdev; +Cc: Pravin B Shelar
This patch is similar to earlier vxlan patch.
Geneve device close operation frees geneve socket. This
operation can race with geneve-xmit function which
dereferences geneve socket. Following patch uses RCU
mechanism to avoid this situation.
Signed-off-by: Pravin B Shelar <pshelar@ovn.org>
---
drivers/net/geneve.c | 45 ++++++++++++++++++++++++++++++++++-----------
1 file changed, 34 insertions(+), 11 deletions(-)
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index 16af1ce..42edd7b 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -58,9 +58,9 @@ struct geneve_dev {
struct hlist_node hlist; /* vni hash table */
struct net *net; /* netns for packet i/o */
struct net_device *dev; /* netdev for geneve tunnel */
- struct geneve_sock *sock4; /* IPv4 socket used for geneve tunnel */
+ struct geneve_sock __rcu *sock4; /* IPv4 socket used for geneve tunnel */
#if IS_ENABLED(CONFIG_IPV6)
- struct geneve_sock *sock6; /* IPv6 socket used for geneve tunnel */
+ struct geneve_sock __rcu *sock6; /* IPv6 socket used for geneve tunnel */
#endif
u8 vni[3]; /* virtual network ID for tunnel */
u8 ttl; /* TTL override */
@@ -543,9 +543,19 @@ static void __geneve_sock_release(struct geneve_sock *gs)
static void geneve_sock_release(struct geneve_dev *geneve)
{
- __geneve_sock_release(geneve->sock4);
+ struct geneve_sock *gs4 = rtnl_dereference(geneve->sock4);
#if IS_ENABLED(CONFIG_IPV6)
- __geneve_sock_release(geneve->sock6);
+ struct geneve_sock *gs6 = rtnl_dereference(geneve->sock6);
+
+ rcu_assign_pointer(geneve->sock6, NULL);
+#endif
+
+ rcu_assign_pointer(geneve->sock4, NULL);
+ synchronize_net();
+
+ __geneve_sock_release(gs4);
+#if IS_ENABLED(CONFIG_IPV6)
+ __geneve_sock_release(gs6);
#endif
}
@@ -586,10 +596,10 @@ static int geneve_sock_add(struct geneve_dev *geneve, bool ipv6)
gs->flags = geneve->flags;
#if IS_ENABLED(CONFIG_IPV6)
if (ipv6)
- geneve->sock6 = gs;
+ rcu_assign_pointer(geneve->sock6, gs);
else
#endif
- geneve->sock4 = gs;
+ rcu_assign_pointer(geneve->sock4, gs);
hash = geneve_net_vni_hash(geneve->vni);
hlist_add_head_rcu(&geneve->hlist, &gs->vni_list[hash]);
@@ -603,9 +613,7 @@ static int geneve_open(struct net_device *dev)
bool metadata = geneve->collect_md;
int ret = 0;
- geneve->sock4 = NULL;
#if IS_ENABLED(CONFIG_IPV6)
- geneve->sock6 = NULL;
if (ipv6 || metadata)
ret = geneve_sock_add(geneve, true);
#endif
@@ -720,6 +728,9 @@ static struct rtable *geneve_get_v4_rt(struct sk_buff *skb,
struct rtable *rt = NULL;
__u8 tos;
+ if (!rcu_dereference(geneve->sock4))
+ return ERR_PTR(-EIO);
+
memset(fl4, 0, sizeof(*fl4));
fl4->flowi4_mark = skb->mark;
fl4->flowi4_proto = IPPROTO_UDP;
@@ -772,11 +783,15 @@ static struct dst_entry *geneve_get_v6_dst(struct sk_buff *skb,
{
bool use_cache = ip_tunnel_dst_cache_usable(skb, info);
struct geneve_dev *geneve = netdev_priv(dev);
- struct geneve_sock *gs6 = geneve->sock6;
struct dst_entry *dst = NULL;
struct dst_cache *dst_cache;
+ struct geneve_sock *gs6;
__u8 prio;
+ gs6 = rcu_dereference(geneve->sock6);
+ if (!gs6)
+ return ERR_PTR(-EIO);
+
memset(fl6, 0, sizeof(*fl6));
fl6->flowi6_mark = skb->mark;
fl6->flowi6_proto = IPPROTO_UDP;
@@ -842,7 +857,7 @@ static netdev_tx_t geneve_xmit_skb(struct sk_buff *skb, struct net_device *dev,
struct ip_tunnel_info *info)
{
struct geneve_dev *geneve = netdev_priv(dev);
- struct geneve_sock *gs4 = geneve->sock4;
+ struct geneve_sock *gs4;
struct rtable *rt = NULL;
const struct iphdr *iip; /* interior IP header */
int err = -EINVAL;
@@ -853,6 +868,10 @@ static netdev_tx_t geneve_xmit_skb(struct sk_buff *skb, struct net_device *dev,
bool xnet = !net_eq(geneve->net, dev_net(geneve->dev));
u32 flags = geneve->flags;
+ gs4 = rcu_dereference(geneve->sock4);
+ if (!gs4)
+ goto tx_error;
+
if (geneve->collect_md) {
if (unlikely(!info || !(info->mode & IP_TUNNEL_INFO_TX))) {
netdev_dbg(dev, "no tunnel metadata\n");
@@ -932,9 +951,9 @@ static netdev_tx_t geneve6_xmit_skb(struct sk_buff *skb, struct net_device *dev,
struct ip_tunnel_info *info)
{
struct geneve_dev *geneve = netdev_priv(dev);
- struct geneve_sock *gs6 = geneve->sock6;
struct dst_entry *dst = NULL;
const struct iphdr *iip; /* interior IP header */
+ struct geneve_sock *gs6;
int err = -EINVAL;
struct flowi6 fl6;
__u8 prio, ttl;
@@ -943,6 +962,10 @@ static netdev_tx_t geneve6_xmit_skb(struct sk_buff *skb, struct net_device *dev,
bool xnet = !net_eq(geneve->net, dev_net(geneve->dev));
u32 flags = geneve->flags;
+ gs6 = rcu_dereference(geneve->sock6);
+ if (!gs6)
+ goto tx_error;
+
if (geneve->collect_md) {
if (unlikely(!info || !(info->mode & IP_TUNNEL_INFO_TX))) {
netdev_dbg(dev, "no tunnel metadata\n");
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/2] vxlan: avoid using stale vxlan socket.
2016-10-27 18:51 [PATCH net 1/2] vxlan: avoid using stale vxlan socket Pravin B Shelar
2016-10-27 18:51 ` [PATCH net 2/2] geneve: avoid using stale geneve socket Pravin B Shelar
@ 2016-10-27 23:02 ` Stephen Hemminger
2016-10-28 16:59 ` Pravin Shelar
1 sibling, 1 reply; 6+ messages in thread
From: Stephen Hemminger @ 2016-10-27 23:02 UTC (permalink / raw)
To: Pravin B Shelar; +Cc: netdev
On Thu, 27 Oct 2016 11:51:55 -0700
Pravin B Shelar <pshelar@ovn.org> wrote:
> - vxlan->vn4_sock = NULL;
> + rcu_assign_pointer(vxlan->vn4_sock, NULL);
> #if IS_ENABLED(CONFIG_IPV6)
> - vxlan->vn6_sock = NULL;
> + rcu_assign_pointer(vxlan->vn6_sock, NULL);
> if (ipv6 || metadata)
User RCU_INIT_POINTER when setting pointer to NULL.
It avoids unnecessary barrier.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 2/2] geneve: avoid using stale geneve socket.
2016-10-27 18:51 ` [PATCH net 2/2] geneve: avoid using stale geneve socket Pravin B Shelar
@ 2016-10-28 14:27 ` John W. Linville
2016-10-28 17:01 ` Pravin Shelar
0 siblings, 1 reply; 6+ messages in thread
From: John W. Linville @ 2016-10-28 14:27 UTC (permalink / raw)
To: Pravin B Shelar; +Cc: netdev
On Thu, Oct 27, 2016 at 11:51:56AM -0700, Pravin B Shelar wrote:
> This patch is similar to earlier vxlan patch.
> Geneve device close operation frees geneve socket. This
> operation can race with geneve-xmit function which
> dereferences geneve socket. Following patch uses RCU
> mechanism to avoid this situation.
>
> Signed-off-by: Pravin B Shelar <pshelar@ovn.org>
LGTM, although I reckon that Stephen's comment about RCU_INIT_POINTER
applies to this patch as well.
Either way...
Acked-by: John W. Linville <linville@tuxdriver.com>
> ---
> drivers/net/geneve.c | 45 ++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 34 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> index 16af1ce..42edd7b 100644
> --- a/drivers/net/geneve.c
> +++ b/drivers/net/geneve.c
> @@ -58,9 +58,9 @@ struct geneve_dev {
> struct hlist_node hlist; /* vni hash table */
> struct net *net; /* netns for packet i/o */
> struct net_device *dev; /* netdev for geneve tunnel */
> - struct geneve_sock *sock4; /* IPv4 socket used for geneve tunnel */
> + struct geneve_sock __rcu *sock4; /* IPv4 socket used for geneve tunnel */
> #if IS_ENABLED(CONFIG_IPV6)
> - struct geneve_sock *sock6; /* IPv6 socket used for geneve tunnel */
> + struct geneve_sock __rcu *sock6; /* IPv6 socket used for geneve tunnel */
> #endif
> u8 vni[3]; /* virtual network ID for tunnel */
> u8 ttl; /* TTL override */
> @@ -543,9 +543,19 @@ static void __geneve_sock_release(struct geneve_sock *gs)
>
> static void geneve_sock_release(struct geneve_dev *geneve)
> {
> - __geneve_sock_release(geneve->sock4);
> + struct geneve_sock *gs4 = rtnl_dereference(geneve->sock4);
> #if IS_ENABLED(CONFIG_IPV6)
> - __geneve_sock_release(geneve->sock6);
> + struct geneve_sock *gs6 = rtnl_dereference(geneve->sock6);
> +
> + rcu_assign_pointer(geneve->sock6, NULL);
> +#endif
> +
> + rcu_assign_pointer(geneve->sock4, NULL);
> + synchronize_net();
> +
> + __geneve_sock_release(gs4);
> +#if IS_ENABLED(CONFIG_IPV6)
> + __geneve_sock_release(gs6);
> #endif
> }
>
> @@ -586,10 +596,10 @@ static int geneve_sock_add(struct geneve_dev *geneve, bool ipv6)
> gs->flags = geneve->flags;
> #if IS_ENABLED(CONFIG_IPV6)
> if (ipv6)
> - geneve->sock6 = gs;
> + rcu_assign_pointer(geneve->sock6, gs);
> else
> #endif
> - geneve->sock4 = gs;
> + rcu_assign_pointer(geneve->sock4, gs);
>
> hash = geneve_net_vni_hash(geneve->vni);
> hlist_add_head_rcu(&geneve->hlist, &gs->vni_list[hash]);
> @@ -603,9 +613,7 @@ static int geneve_open(struct net_device *dev)
> bool metadata = geneve->collect_md;
> int ret = 0;
>
> - geneve->sock4 = NULL;
> #if IS_ENABLED(CONFIG_IPV6)
> - geneve->sock6 = NULL;
> if (ipv6 || metadata)
> ret = geneve_sock_add(geneve, true);
> #endif
> @@ -720,6 +728,9 @@ static struct rtable *geneve_get_v4_rt(struct sk_buff *skb,
> struct rtable *rt = NULL;
> __u8 tos;
>
> + if (!rcu_dereference(geneve->sock4))
> + return ERR_PTR(-EIO);
> +
> memset(fl4, 0, sizeof(*fl4));
> fl4->flowi4_mark = skb->mark;
> fl4->flowi4_proto = IPPROTO_UDP;
> @@ -772,11 +783,15 @@ static struct dst_entry *geneve_get_v6_dst(struct sk_buff *skb,
> {
> bool use_cache = ip_tunnel_dst_cache_usable(skb, info);
> struct geneve_dev *geneve = netdev_priv(dev);
> - struct geneve_sock *gs6 = geneve->sock6;
> struct dst_entry *dst = NULL;
> struct dst_cache *dst_cache;
> + struct geneve_sock *gs6;
> __u8 prio;
>
> + gs6 = rcu_dereference(geneve->sock6);
> + if (!gs6)
> + return ERR_PTR(-EIO);
> +
> memset(fl6, 0, sizeof(*fl6));
> fl6->flowi6_mark = skb->mark;
> fl6->flowi6_proto = IPPROTO_UDP;
> @@ -842,7 +857,7 @@ static netdev_tx_t geneve_xmit_skb(struct sk_buff *skb, struct net_device *dev,
> struct ip_tunnel_info *info)
> {
> struct geneve_dev *geneve = netdev_priv(dev);
> - struct geneve_sock *gs4 = geneve->sock4;
> + struct geneve_sock *gs4;
> struct rtable *rt = NULL;
> const struct iphdr *iip; /* interior IP header */
> int err = -EINVAL;
> @@ -853,6 +868,10 @@ static netdev_tx_t geneve_xmit_skb(struct sk_buff *skb, struct net_device *dev,
> bool xnet = !net_eq(geneve->net, dev_net(geneve->dev));
> u32 flags = geneve->flags;
>
> + gs4 = rcu_dereference(geneve->sock4);
> + if (!gs4)
> + goto tx_error;
> +
> if (geneve->collect_md) {
> if (unlikely(!info || !(info->mode & IP_TUNNEL_INFO_TX))) {
> netdev_dbg(dev, "no tunnel metadata\n");
> @@ -932,9 +951,9 @@ static netdev_tx_t geneve6_xmit_skb(struct sk_buff *skb, struct net_device *dev,
> struct ip_tunnel_info *info)
> {
> struct geneve_dev *geneve = netdev_priv(dev);
> - struct geneve_sock *gs6 = geneve->sock6;
> struct dst_entry *dst = NULL;
> const struct iphdr *iip; /* interior IP header */
> + struct geneve_sock *gs6;
> int err = -EINVAL;
> struct flowi6 fl6;
> __u8 prio, ttl;
> @@ -943,6 +962,10 @@ static netdev_tx_t geneve6_xmit_skb(struct sk_buff *skb, struct net_device *dev,
> bool xnet = !net_eq(geneve->net, dev_net(geneve->dev));
> u32 flags = geneve->flags;
>
> + gs6 = rcu_dereference(geneve->sock6);
> + if (!gs6)
> + goto tx_error;
> +
> if (geneve->collect_md) {
> if (unlikely(!info || !(info->mode & IP_TUNNEL_INFO_TX))) {
> netdev_dbg(dev, "no tunnel metadata\n");
> --
> 1.9.1
>
>
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/2] vxlan: avoid using stale vxlan socket.
2016-10-27 23:02 ` [PATCH net 1/2] vxlan: avoid using stale vxlan socket Stephen Hemminger
@ 2016-10-28 16:59 ` Pravin Shelar
0 siblings, 0 replies; 6+ messages in thread
From: Pravin Shelar @ 2016-10-28 16:59 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Linux Kernel Network Developers
On Thu, Oct 27, 2016 at 4:02 PM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> On Thu, 27 Oct 2016 11:51:55 -0700
> Pravin B Shelar <pshelar@ovn.org> wrote:
>
>> - vxlan->vn4_sock = NULL;
>> + rcu_assign_pointer(vxlan->vn4_sock, NULL);
>> #if IS_ENABLED(CONFIG_IPV6)
>> - vxlan->vn6_sock = NULL;
>> + rcu_assign_pointer(vxlan->vn6_sock, NULL);
>> if (ipv6 || metadata)
>
> User RCU_INIT_POINTER when setting pointer to NULL.
> It avoids unnecessary barrier.
OK. I have posted v2 patch to use RCU_INIT_POINTER().
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 2/2] geneve: avoid using stale geneve socket.
2016-10-28 14:27 ` John W. Linville
@ 2016-10-28 17:01 ` Pravin Shelar
0 siblings, 0 replies; 6+ messages in thread
From: Pravin Shelar @ 2016-10-28 17:01 UTC (permalink / raw)
To: John W. Linville; +Cc: Linux Kernel Network Developers
On Fri, Oct 28, 2016 at 7:27 AM, John W. Linville
<linville@tuxdriver.com> wrote:
> On Thu, Oct 27, 2016 at 11:51:56AM -0700, Pravin B Shelar wrote:
>> This patch is similar to earlier vxlan patch.
>> Geneve device close operation frees geneve socket. This
>> operation can race with geneve-xmit function which
>> dereferences geneve socket. Following patch uses RCU
>> mechanism to avoid this situation.
>>
>> Signed-off-by: Pravin B Shelar <pshelar@ovn.org>
>
> LGTM, although I reckon that Stephen's comment about RCU_INIT_POINTER
> applies to this patch as well.
>
Geneve code is bit different and it is using rcu_assign_pointer()
while destroying the object. so we need the barrier.
> Either way...
>
> Acked-by: John W. Linville <linville@tuxdriver.com>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-10-28 17:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-27 18:51 [PATCH net 1/2] vxlan: avoid using stale vxlan socket Pravin B Shelar
2016-10-27 18:51 ` [PATCH net 2/2] geneve: avoid using stale geneve socket Pravin B Shelar
2016-10-28 14:27 ` John W. Linville
2016-10-28 17:01 ` Pravin Shelar
2016-10-27 23:02 ` [PATCH net 1/2] vxlan: avoid using stale vxlan socket Stephen Hemminger
2016-10-28 16:59 ` Pravin Shelar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).